| 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); }
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. gcc/ChangeLog PR c/28901 * common.opt (Wunused-const-variable): New option. * toplev.c (check_global_declaration): Check and use warn_unused_const_variable. * doc/invoke.texi (Warning Options): Add -Wunused-const-variable. (-Wunused-variable): Remove non-constant. Implies -Wunused-const-variable. (-Wunused-const-variable): New. gcc/testsuite/ChangeLog 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. Tested on x86_64-pc-linux-gnu, no regressions. ---