Message ID | 1442009555-13690-1-git-send-email-mjw@redhat.com |
---|---|
State | New |
Headers | show |
On 09/12/2015 12:12 AM, Mark Wielaard wrote: > 12 years ago it was decided that -Wunused-variable shouldn't warn about > static const variables because some code used const static char rcsid[] > strings which were never used but wanted in the code anyway. But as the > bug points out this hides some real bugs. These days the usage of rcsids > is not very popular anymore. So this patch changes the default to warn > about unused static const variables with -Wunused-variable. And it adds > a new option -Wno-unused-const-variable to turn this warning off. New > testcases are included to test the new warning with -Wunused-variable > and suppressing it with -Wno-unused-const-variable or unused attribute. > PR c/28901 > * gcc.dg/unused-4.c: Adjust warning for static const. > * gcc.dg/unused-variable-1.c: New test. > * gcc.dg/unused-variable-2.c: Likewise. Should these go into c-c++-common? Otherwise I'm ok with the patch, please wait a few days to see if there are objections to this change then commit. Bernd
On Sat, Sep 12, 2015 at 12:29:05AM +0200, Bernd Schmidt wrote: > On 09/12/2015 12:12 AM, Mark Wielaard wrote: > >12 years ago it was decided that -Wunused-variable shouldn't warn about > >static const variables because some code used const static char rcsid[] > >strings which were never used but wanted in the code anyway. But as the > >bug points out this hides some real bugs. These days the usage of rcsids > >is not very popular anymore. So this patch changes the default to warn > >about unused static const variables with -Wunused-variable. And it adds > >a new option -Wno-unused-const-variable to turn this warning off. New > >testcases are included to test the new warning with -Wunused-variable > >and suppressing it with -Wno-unused-const-variable or unused attribute. > > > PR c/28901 > > * gcc.dg/unused-4.c: Adjust warning for static const. > > * gcc.dg/unused-variable-1.c: New test. > > * gcc.dg/unused-variable-2.c: Likewise. > > Should these go into c-c++-common? No. It is C only. But I realize that isn't really clear from my patch nor from the documentation I wrote for it. Since in C++ a const isn't by default file scoped and const variables always need to be initialized they are used differently than in C. Where in C you would use a #define in C++ you would use a const. So the cxx_warn_unused_global_decl () lang hook explicitly says to not warn about them. Although I think that is correct, I now think it is confusing you cannot enable the warning for C++ if you really want to. It should be off by default for C++, but on by default for C when -Wunused-variable is enabled. But it would actually be nice to be able to use it too for C++ if the user really wants to instead of having the warning suppression for C++ hardcoded. > Otherwise I'm ok with the patch, please > wait a few days to see if there are objections to this change then commit. I'll rewrite my patch a little, add some C++ testcases, and update the documentation. Then we can discuss again. Thanks, Mark
diff --git a/gcc/common.opt b/gcc/common.opt index 94d1d88..ae2fe77 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -735,6 +735,10 @@ Wunused-variable Common Var(warn_unused_variable) Warning EnabledBy(Wunused) Warn when a variable is unused +Wunused-const-variable +Common Var(warn_unused_const_variable) Warning EnabledBy(Wunused-variable) +Warn when a const variable is unused + Wcoverage-mismatch Common Var(warn_coverage_mismatch) Init(1) Warning Warn in case profiles in -fprofile-use do not match diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 518d689..211b9e1 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -290,6 +290,7 @@ Objective-C and Objective-C++ Dialects}. -Wunsuffixed-float-constants -Wunused -Wunused-function @gol -Wunused-label -Wunused-local-typedefs -Wunused-parameter @gol -Wno-unused-result -Wunused-value @gol -Wunused-variable @gol +-Wunused-const-variable @gol -Wunused-but-set-parameter -Wunused-but-set-variable @gol -Wuseless-cast -Wvariadic-macros -Wvector-operation-performance @gol -Wvla -Wvolatile-register-var -Wwrite-strings @gol @@ -4143,13 +4144,22 @@ its return value. The default is @option{-Wunused-result}. @item -Wunused-variable @opindex Wunused-variable @opindex Wno-unused-variable -Warn whenever a local variable or non-constant static variable is unused -aside from its declaration. +Warn whenever a local or static variable is unused aside from its +declaration. This option implies @option{-Wunused-const-variable}. This warning is enabled by @option{-Wall}. To suppress this warning use the @code{unused} attribute (@pxref{Variable Attributes}). +@item -Wunused-const-variable +@opindex Wunused-const-variable +@opindex Wno-unused-const-variable +Warn whenever a constant static variable is unused aside from its declaration. +This warning is enabled by @option{-Wunused-variable}. + +To suppress this warning use the @code{unused} attribute +(@pxref{Variable Attributes}). + @item -Wunused-value @opindex Wunused-value @opindex Wno-unused-value diff --git a/gcc/testsuite/gcc.dg/unused-4.c b/gcc/testsuite/gcc.dg/unused-4.c index 99e845f..5323600 100644 --- a/gcc/testsuite/gcc.dg/unused-4.c +++ b/gcc/testsuite/gcc.dg/unused-4.c @@ -1,6 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-Wunused -O3" } */ -static const int i = 0; +static const int i = 0; /* { dg-warning "defined but not used" } */ static void f() { } /* { dg-warning "defined but not used" } */ static inline void g() { } diff --git a/gcc/testsuite/gcc.dg/unused-variable-1.c b/gcc/testsuite/gcc.dg/unused-variable-1.c new file mode 100644 index 0000000..cb86c3bc --- /dev/null +++ b/gcc/testsuite/gcc.dg/unused-variable-1.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-Wunused-variable" } */ + +static int a = 0; /* { dg-warning "defined but not used" } */ +static const int b = 0; /* { dg-warning "defined but not used" } */ +static int c __attribute__ ((unused)) = 0; +static const char rcsid[] __attribute__ ((unused)) = "version-string"; diff --git a/gcc/testsuite/gcc.dg/unused-variable-2.c b/gcc/testsuite/gcc.dg/unused-variable-2.c new file mode 100644 index 0000000..0496466 --- /dev/null +++ b/gcc/testsuite/gcc.dg/unused-variable-2.c @@ -0,0 +1,7 @@ +/* { dg-do compile } */ +/* { dg-options "-Wunused-variable -Wno-unused-const-variable" } */ + +static int a = 0; /* { dg-warning "defined but not used" } */ +static const int b = 0; +static int c __attribute__ ((unused)) = 0; +static const char rcsid[] = "version-string"; diff --git a/gcc/toplev.c b/gcc/toplev.c index 926224a..95e4c52 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -497,10 +497,9 @@ check_global_declaration (tree decl) /* Warn about static fns or vars defined but not used. */ if (((warn_unused_function && TREE_CODE (decl) == FUNCTION_DECL) - /* We don't warn about "static const" variables because the - "rcs_id" idiom uses that construction. */ - || (warn_unused_variable - && TREE_CODE (decl) == VAR_DECL && ! TREE_READONLY (decl))) + || (((warn_unused_variable && ! TREE_READONLY (decl)) + || (warn_unused_const_variable && TREE_READONLY (decl))) + && TREE_CODE (decl) == VAR_DECL)) && ! DECL_IN_SYSTEM_HEADER (decl) && ! snode->referred_to_p (/*include_self=*/false) /* This TREE_USED check is needed in addition to referred_to_p @@ -527,7 +526,9 @@ check_global_declaration (tree decl) warning_at (DECL_SOURCE_LOCATION (decl), (TREE_CODE (decl) == FUNCTION_DECL) ? OPT_Wunused_function - : OPT_Wunused_variable, + : (TREE_READONLY (decl) + ? OPT_Wunused_const_variable + : OPT_Wunused_variable), "%qD defined but not used", decl); }