Skip to content
This repository has been archived by the owner on Jan 2, 2021. It is now read-only.

missing-prototypes #312

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

NaruFGT
Copy link

@NaruFGT NaruFGT commented Jan 3, 2020

Contributor @strager had recommended enabling missing-prototypes, which proved useful for identifying unused code. In addition to missing-prototypes in C code, this will enable -Wall and -Werror.

Contributor @strager had recommended enabling missing-prototypes, which proved us eful for identifying unused code. In addition to missing-prototypes, this will en able -Wall and -Werror for C code.
@NaruFGT NaruFGT marked this pull request as ready for review January 3, 2020 10:09
CMakeLists.txt Outdated Show resolved Hide resolved
add_compile_options() will need CMake generator expressions to distinguish between invocations of C and C++ compilers. This commit did not solve the issue and may cause build errors if the missing-prototypes option is passed to C++ compilers.

This reverts commit 45a1357.
Contributor @strager had recommended enabling missing-prototypes, which proved useful for identifying unused code.
I have added warning options -Wmissing-declarations and for the C compiler -Wmissing-prototypes.
Until these warnings are taken care of, I've disabled errors for these warnings.
Copy link
Contributor

@sas sas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should add these checks and then lower them to warning status unless we actually fix the issues in the code. The value provided by these warnings is low because it will just result in more noise during the build.

@NaruFGT
Copy link
Author

NaruFGT commented Feb 10, 2020

If we aren't going to address them then perhaps just disabling this warning entirely on GCC and Clang builds would be appropriate?

@sas
Copy link
Contributor

sas commented Feb 10, 2020

Disabling them completely would be fine by me. It's up to you; if you want we can disable them, but if you prefer fixing them and landing the fixes along with the changes that enable the warnings everywhere, then that works too.

Forgive me if I asked you this question before but are these warnings enabled by default on certain versions of gcc/clang? If I remember correctly, they are disabled by default and the user needs to enabled them.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants