On Sun, Sep 13, 2015 at 12:00:51AM +0200, Mark Wielaard wrote:
> I'll rewrite my patch a little, add some C++ testcases, and update the
> documentation. Then we can discuss again.

Slightly adjusted patch attached. Now it is explicit that the warning is
enabled by -Wunused-variable for C, but not C++. There are testcases for
both C and C++ to check the defaults. And the hardcoded override is
removed for C++, so the user could enable it if they want.

Regtested on x86_64-pc-linux-gnu only new PASSes.

>From 609e4495b6d63a0212e9964064f47f7b26c69b7e Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@redhat.com>
Date: Fri, 11 Sep 2015 23:54:15 +0200
Subject: [PATCH] PR28901 -Wunused-variable ignores unused const initialised
 variables in C

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 in C with -Wunused-variable. And it
adds a new option -Wno-unused-const-variable to turn this warning off.
For C++ this new warning is off by default, since const variables can be
used as #defines in C++. New testcases for the new defaults in C and C++
are included testing the new warning and suppressing it with an unused
attribute or using -Wno-unused-const-variable.

gcc/ChangeLog

       PR c/28901
       * common.opt (Wunused-const-variable): New option.
       * toplev.c (check_global_declaration): Check and use
       warn_unused_const_variable_set.
       * doc/invoke.texi (Warning Options): Add -Wunused-const-variable.
       (-Wunused-variable): Remove non-constant. For C mplies
       -Wunused-const-variable.
       (-Wunused-const-variable): New.

gcc/c-family/ChangeLog

       PR c/28901
       * c-opts.c (c_common_post_options): warn_unused_const_variable_set
       is set when not explicitly set for C when warn_unused_variable is
       set.

gcc/cp/ChangeLog

       PR c/28901
       * cp-objcp-common.c (cxx_warn_unused_global_decl): Remove hard-coded
       VAR_P TREE_READONLY override.

gcc/testsuite/ChangeLog

       PR c/28901
       * g++.dg/warn/unused-variable-1.C: New test.
       * g++.dg/warn/unused-variable-2.C: Likewise.
       * 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.
---
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 3239a85..f8a5aa1 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -862,6 +862,12 @@ c_common_post_options (const char **pfilename)
     warn_shift_negative_value = (extra_warnings
                                 && (cxx_dialect >= cxx11 || flag_isoc99));
 
+  /* -Wunused-variable implies -Wunused-const-variable for C, but not
+     for C++, because const variables take the place of #defines in C++.  */
+  if (warn_unused_const_variable_set == -1)
+    warn_unused_const_variable_set = (warn_unused_variable
+                                     && ! c_dialect_cxx ());
+
   /* Declone C++ 'structors if -Os.  */
   if (flag_declone_ctor_dtor == -1)
     flag_declone_ctor_dtor = optimize_size;
diff --git a/gcc/common.opt b/gcc/common.opt
index 94d1d88..c85f1b9 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_set) Init(-1) Warning
+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/cp/cp-objcp-common.c b/gcc/cp/cp-objcp-common.c
index 2cab89c..808defd 100644
--- a/gcc/cp/cp-objcp-common.c
+++ b/gcc/cp/cp-objcp-common.c
@@ -62,10 +62,6 @@ cxx_warn_unused_global_decl (const_tree decl)
   if (DECL_IN_SYSTEM_HEADER (decl))
     return false;
 
-  /* Const variables take the place of #defines in C++.  */
-  if (VAR_P (decl) && TREE_READONLY (decl))
-    return false;
-
   return true;
 }
 
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 518d689..7b5e44e 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,9 +4144,20 @@ 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.
-This warning is enabled by @option{-Wall}.
+Warn whenever a local or static variable is unused aside from its
+declaration. This option implies @option{-Wunused-const-variable} 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
+@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++.
 
 To suppress this warning use the @code{unused} attribute
 (@pxref{Variable Attributes}).
diff --git a/gcc/testsuite/g++.dg/warn/unused-variable-1.C 
b/gcc/testsuite/g++.dg/warn/unused-variable-1.C
new file mode 100644
index 0000000..cf531c0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/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;          /* Unlike C, this doesn't cause a warning in 
C++. */
+static int c __attribute__ ((unused)) = 0;
+static const char rcsid[] = "version-string"; /* Likewise. */
diff --git a/gcc/testsuite/g++.dg/warn/unused-variable-2.C 
b/gcc/testsuite/g++.dg/warn/unused-variable-2.C
new file mode 100644
index 0000000..b608fbc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/unused-variable-2.C
@@ -0,0 +1,7 @@
+/* { dg-do compile } */
+/* { dg-options "-Wunused-variable -Wunused-const-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-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..83cafed 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_set > 0 && 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);
 }
 

Reply via email to