[PATCH] PR28901 -Wunused-variable ignores unused const initialised variables

Mark Wielaard mjw@redhat.com
Sun Sep 13 18:50:00 GMT 2015


On Sun, Sep 13, 2015 at 02:50:53PM +0200, Manuel López-Ibáñez wrote:
> On 13/09/15 13:40, Mark Wielaard wrote:
> >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.
> 
> >+  /* -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 ());
> >+
> 
> Can't you use LangEnabledBy() in c.opt to implement this?

Yes, I can. See simplified patch attached.
I wasn't aware there was already such a nice framework for this.

Thanks,

Mark
-------------- next part --------------
commit 97505bd0e4ac15d86c2a302cfebc5f1a4fc2c2e8
Author: Mark Wielaard <mjw@redhat.com>
Date:   Fri Sep 11 23:54:15 2015 +0200

    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
           * 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. For C implies
           -Wunused-const-variable.
           (-Wunused-const-variable): New.
    
    gcc/c-family/ChangeLog
    
           PR c/28901
           * c.opt (Wunused-variable): Option from common.opt.
           (Wunused-const-variable): New option.
    
    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.opt b/gcc/c-family/c.opt
index d519d7a..47ba070 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -912,6 +912,14 @@ Wunused-result
 C ObjC C++ ObjC++ Var(warn_unused_result) Init(1) Warning
 Warn if a caller of a function, marked with attribute warn_unused_result, does not use its return value
 
+Wunused-variable
+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)
+Warn when a const variable is unused
+
 Wvariadic-macros
 C ObjC C++ ObjC++ CPP(warn_variadic_macros) CppReason(CPP_W_VARIADIC_MACROS) Var(cpp_warn_variadic_macros) Init(0) Warning LangEnabledBy(C ObjC C++ ObjC++,Wpedantic || Wtraditional)
 Warn about using variadic macros
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..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);
 }
 


More information about the Gcc-patches mailing list