| Message ID | 1456018952-25270-1-git-send-email-mjw@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/20/2016 06:42 PM, Mark Wielaard wrote: > There is some controversy about enabling -Wunused-const-variable for all > unused static const variables because some feel there are too many errors > exposed in header files. Create two levels for -Wunused-const-variable. > One level to only check for unused static const variables in the main > compilation file. Which is enabled by -Wunused-variable. And a second > level that also checks for unused static const variables in included > header files. Which must be explicitly enabled. > > gcc/ChangeLog > > PR c/28901 > * cgraphunit.c (check_global_declaration): Check level of > warn_unused_const_variable and main_input_filename. > * doc/invoke.texi (Warning Options): Add -Wunused-const-variable=. > (-Wunused-variable): For C implies -Wunused-const-variable=1. > (-Wunused-const-variable): Explain levels 1 and 2. > > gcc/c-family/ChangeLog > > PR c/28901 > * c.opt (Wunused-const-variable): Turn into Alias for... > (Wunused-const-variable=): New option. > > gcc/testsuite/ChangeLog > > PR c/28901 > * gcc.dg/unused-variable-3.c: New test. Note that given the discussion in the BZ, I'm going to consider this a regression and thus eligible for the trunk. > diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > index 0a745f0..27a073a 100644 > --- a/gcc/cgraphunit.c > +++ b/gcc/cgraphunit.c > @@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode) > (TREE_CODE (decl) == FUNCTION_DECL) > ? OPT_Wunused_function > : (TREE_READONLY (decl) > - ? OPT_Wunused_const_variable > + ? OPT_Wunused_const_variable_ Typo here? If that's not a typo, then just say so and this is approved. If it is a typo, the patch is approved with the typo fixed appropriately. jeff
On Mon, Feb 22, 2016 at 11:57:56AM -0700, Jeff Law wrote: > >diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > >index 0a745f0..27a073a 100644 > >--- a/gcc/cgraphunit.c > >+++ b/gcc/cgraphunit.c > >@@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode) > > (TREE_CODE (decl) == FUNCTION_DECL) > > ? OPT_Wunused_function > > : (TREE_READONLY (decl) > >- ? OPT_Wunused_const_variable > >+ ? OPT_Wunused_const_variable_ > Typo here? The _ stands for = from the option name, as -Wunused-const-variable is an alias to -Wunused-const-variable=2, I think we want the trailing underscore. Jakub
On 02/22/2016 12:00 PM, Jakub Jelinek wrote: > On Mon, Feb 22, 2016 at 11:57:56AM -0700, Jeff Law wrote: >>> diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c >>> index 0a745f0..27a073a 100644 >>> --- a/gcc/cgraphunit.c >>> +++ b/gcc/cgraphunit.c >>> @@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode) >>> (TREE_CODE (decl) == FUNCTION_DECL) >>> ? OPT_Wunused_function >>> : (TREE_READONLY (decl) >>> - ? OPT_Wunused_const_variable >>> + ? OPT_Wunused_const_variable_ >> Typo here? > > The _ stands for = from the option name, as -Wunused-const-variable > is an alias to -Wunused-const-variable=2, I think we want the trailing > underscore. Looked funny. If that's standard practice, then it's fine with me. jeff
On Mon, Feb 22, 2016 at 11:57:56AM -0700, Jeff Law wrote: > On 02/20/2016 06:42 PM, Mark Wielaard wrote: > Note that given the discussion in the BZ, I'm going to consider this a > regression and thus eligible for the trunk. Thanks. Unfortunately new warnings always seem to make some people unhappy (even when others are happy and see them as useful). Hopefully this compromise makes it so that nobody sees this warning as regression. > >diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c > >index 0a745f0..27a073a 100644 > >--- a/gcc/cgraphunit.c > >+++ b/gcc/cgraphunit.c > >@@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode) > > (TREE_CODE (decl) == FUNCTION_DECL) > > ? OPT_Wunused_function > > : (TREE_READONLY (decl) > >- ? OPT_Wunused_const_variable > >+ ? OPT_Wunused_const_variable_ > Typo here? > > If that's not a typo, then just say so and this is approved. As Jakub already explained that was deliberate. It is how a warning option that can take a level is represented. Pushed. Thanks, Mark
On Mon, Feb 22, 2016 at 2:43 PM, Mark Wielaard <mjw@redhat.com> wrote: > On Mon, Feb 22, 2016 at 11:57:56AM -0700, Jeff Law wrote: >> On 02/20/2016 06:42 PM, Mark Wielaard wrote: >> Note that given the discussion in the BZ, I'm going to consider this a >> regression and thus eligible for the trunk. > > Thanks. Unfortunately new warnings always seem to make some people > unhappy (even when others are happy and see them as useful). Hopefully > this compromise makes it so that nobody sees this warning as regression. > >> >diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c >> >index 0a745f0..27a073a 100644 >> >--- a/gcc/cgraphunit.c >> >+++ b/gcc/cgraphunit.c >> >@@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode) >> > (TREE_CODE (decl) == FUNCTION_DECL) >> > ? OPT_Wunused_function >> > : (TREE_READONLY (decl) >> >- ? OPT_Wunused_const_variable >> >+ ? OPT_Wunused_const_variable_ >> Typo here? >> >> If that's not a typo, then just say so and this is approved. > > As Jakub already explained that was deliberate. It is how a warning > option that can take a level is represented. Pushed. > It caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69911
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 638e9c2..7c5f6c7 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -949,7 +949,11 @@ C ObjC C++ ObjC++ LangEnabledBy(C ObjC C++ ObjC++,Wunused) ; documented in common.opt Wunused-const-variable -C ObjC C++ ObjC++ Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable) +C ObjC C++ ObjC++ Warning Alias(Wunused-const-variable=, 2, 0) +Warn when a const variable is unused. + +Wunused-const-variable= +C ObjC C++ ObjC++ Joined RejectNegative UInteger Var(warn_unused_const_variable) Warning LangEnabledBy(C ObjC,Wunused-variable, 1, 0) Warn when a const variable is unused. Wvariadic-macros diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c index 0a745f0..27a073a 100644 --- a/gcc/cgraphunit.c +++ b/gcc/cgraphunit.c @@ -942,7 +942,10 @@ check_global_declaration (symtab_node *snode) /* Warn about static fns or vars defined but not used. */ if (((warn_unused_function && TREE_CODE (decl) == FUNCTION_DECL) || (((warn_unused_variable && ! TREE_READONLY (decl)) - || (warn_unused_const_variable && TREE_READONLY (decl))) + || (warn_unused_const_variable > 0 && TREE_READONLY (decl) + && (warn_unused_const_variable == 2 + || filename_cmp (main_input_filename, + DECL_SOURCE_FILE (decl)) == 0))) && TREE_CODE (decl) == VAR_DECL)) && ! DECL_IN_SYSTEM_HEADER (decl) && ! snode->referred_to_p (/*include_self=*/false) @@ -971,7 +974,7 @@ check_global_declaration (symtab_node *snode) (TREE_CODE (decl) == FUNCTION_DECL) ? OPT_Wunused_function : (TREE_READONLY (decl) - ? OPT_Wunused_const_variable + ? OPT_Wunused_const_variable_ : OPT_Wunused_variable), "%qD defined but not used", decl); } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index c1ab788..490df93 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -303,7 +303,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-const-variable -Wunused-const-variable=@var{n} @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 @@ -4231,23 +4231,39 @@ its return value. The default is @option{-Wunused-result}. @opindex Wunused-variable @opindex Wno-unused-variable Warn whenever a local or static variable is unused aside from its -declaration. This option implies @option{-Wunused-const-variable} for C, +declaration. This option implies @option{-Wunused-const-variable=1} for C, but not for C++. This warning is enabled by @option{-Wall}. To suppress this warning use the @code{unused} attribute (@pxref{Variable Attributes}). @item -Wunused-const-variable +@itemx -Wunused-const-variable=@var{n} @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} for C, but not for C++. -In C++ this is normally not an error since const variables take the place of -@code{#define}s in C++. +@option{-Wunused-const-variable=1} is enabled by @option{-Wunused-variable} +for C, but not for C++. In C this declares variable storage, but in C++ this +is not an error since const variables take the place of @code{#define}s. To suppress this warning use the @code{unused} attribute (@pxref{Variable Attributes}). +@table @gcctabopt +@item -Wunused-const-variable=1 +This is the warning level that is enabled by @option{-Wunused-variable} for +C. It warns only about unused static const variables defined in the main +compilation unit, but not about static const variables declared in any +header included. + +@item -Wunused-const-variable=2 +This warning level also warns for unused constant static variables in +headers (excluding system headers). This is the warning level of +@option{-Wunused-const-variable} and must be explicitly requested since +in C++ this isn't an error and in C it might be harder to clean up all +headers included. +@end table + @item -Wunused-value @opindex Wunused-value @opindex Wno-unused-value diff --git a/gcc/testsuite/gcc.dg/unused-variable-3.c b/gcc/testsuite/gcc.dg/unused-variable-3.c new file mode 100644 index 0000000..6aca958 --- /dev/null +++ b/gcc/testsuite/gcc.dg/unused-variable-3.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-Wunused-variable" } */ + +static const int cmain = 42; /* { dg-warning "defined but not used" } */ + +/* Don't warn for unused static consts in headers, + unless -Wunused-const-variable=2. */ +#line 1 "header.h" +static const int cheader = 42;