Patchwork PR c++/29028 - Missed unused warning on using declaration

login
register
mail settings
Submitter Dodji Seketeli
Date Sept. 21, 2012, 12:58 p.m.
Message ID <m3obkzilcc.fsf@redhat.com>
Download mbox | patch
Permalink /patch/185729/
State New
Headers show

Comments

Dodji Seketeli - Sept. 21, 2012, 12:58 p.m.
Hello,

In the example of the patch, g++ fails to warn that the variable N::i
(introduced via a using declaration) is unused.

This is because as we want to emit the warning in poplevel, when we
walk the local bindings returned by getdecls, we forget that a
VAR_DECL introduced by a using declaration is represented by a
TREE_LIST which TREE_VALUE is the VAR_DECL, and we wrongly look for a
bare VAR_DECL.

Fixed thus and tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* decl.c (poplevel<warn_unused*>): Do not forget that some local
	bindings are represented by a TREE_LIST.

gcc/testsuite/

	* g++.dg/warn/Wunused-var-18.C: New test.
---
 gcc/cp/decl.c                              |   46 ++++++++++++++++------------
 gcc/testsuite/g++.dg/warn/Wunused-var-18.C |   14 ++++++++
 2 files changed, 40 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Wunused-var-18.C
Gabriel Dos Reis - Sept. 21, 2012, 2:57 p.m.
On Fri, Sep 21, 2012 at 7:58 AM, Dodji Seketeli <dodji@redhat.com> wrote:

> +    for (tree d = getdecls (); d; d = TREE_CHAIN (d))
> +      {
> +       /* There are cases where D itself is a TREE_LIST.  See in
> +          push_local_binding where the list of decls returned by
> +          getdecls is built.  */
> +       decl = TREE_CODE (d) == TREE_LIST ? TREE_VALUE (d) : d;
> +       if (TREE_CODE (decl) == VAR_DECL
> +           && (! TREE_USED (decl) || !DECL_READ_P (decl))
> +           && ! DECL_IN_SYSTEM_HEADER (decl)
> +           && DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl)
> +           && TREE_TYPE (decl) != error_mark_node
> +           && (!CLASS_TYPE_P (TREE_TYPE (decl))
> +               || !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl))))
> +         {
> +           if (! TREE_USED (decl))
> +             warning (OPT_Wunused_variable, "unused variable %q+D", decl);
> +           else if (DECL_CONTEXT (decl) == current_function_decl
> +                    && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
> +                    && errorcount == unused_but_set_errorcount)
> +             {
> +               warning (OPT_Wunused_but_set_variable,
> +                        "variable %q+D set but not used", decl);
> +               unused_but_set_errorcount = errorcount;
> +             }
> +         }
> +      }
>

this is block of code that qualifies to be a function in its own.
the condition is quite impressive...

-- Gaby
Jason Merrill - Sept. 24, 2012, 7:35 p.m.
OK.

Jason

Patch

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 6b5b986..dc54904 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -617,26 +617,32 @@  poplevel (int keep, int reverse, int functionbody)
   /* Before we remove the declarations first check for unused variables.  */
   if ((warn_unused_variable || warn_unused_but_set_variable)
       && !processing_template_decl)
-    for (decl = getdecls (); decl; decl = TREE_CHAIN (decl))
-      if (TREE_CODE (decl) == VAR_DECL
-	  && (! TREE_USED (decl) || !DECL_READ_P (decl))
-	  && ! DECL_IN_SYSTEM_HEADER (decl)
-	  && DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl)
-	  && TREE_TYPE (decl) != error_mark_node
-	  && (!CLASS_TYPE_P (TREE_TYPE (decl))
-	      || !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl))))
-	{
-	  if (! TREE_USED (decl))
-	    warning (OPT_Wunused_variable, "unused variable %q+D", decl);
-	  else if (DECL_CONTEXT (decl) == current_function_decl
-		   && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
-		   && errorcount == unused_but_set_errorcount)
-	    {
-	      warning (OPT_Wunused_but_set_variable,
-		       "variable %q+D set but not used", decl); 
-	      unused_but_set_errorcount = errorcount;
-	    }
-	}
+    for (tree d = getdecls (); d; d = TREE_CHAIN (d))
+      {
+	/* There are cases where D itself is a TREE_LIST.  See in
+	   push_local_binding where the list of decls returned by
+	   getdecls is built.  */
+	decl = TREE_CODE (d) == TREE_LIST ? TREE_VALUE (d) : d;
+	if (TREE_CODE (decl) == VAR_DECL
+	    && (! TREE_USED (decl) || !DECL_READ_P (decl))
+	    && ! DECL_IN_SYSTEM_HEADER (decl)
+	    && DECL_NAME (decl) && ! DECL_ARTIFICIAL (decl)
+	    && TREE_TYPE (decl) != error_mark_node
+	    && (!CLASS_TYPE_P (TREE_TYPE (decl))
+		|| !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (TREE_TYPE (decl))))
+	  {
+	    if (! TREE_USED (decl))
+	      warning (OPT_Wunused_variable, "unused variable %q+D", decl);
+	    else if (DECL_CONTEXT (decl) == current_function_decl
+		     && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
+		     && errorcount == unused_but_set_errorcount)
+	      {
+		warning (OPT_Wunused_but_set_variable,
+			 "variable %q+D set but not used", decl);
+		unused_but_set_errorcount = errorcount;
+	      }
+	  }
+      }
 
   /* Remove declarations for all the DECLs in this level.  */
   for (link = decls; link; link = TREE_CHAIN (link))
diff --git a/gcc/testsuite/g++.dg/warn/Wunused-var-18.C b/gcc/testsuite/g++.dg/warn/Wunused-var-18.C
new file mode 100644
index 0000000..0339663
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wunused-var-18.C
@@ -0,0 +1,14 @@ 
+// Origin: PR c++/29028
+// { dg-options "-Wunused" }
+// { dg-do compile }
+
+namespace N
+{
+    int i; // { dg-warning "unused variable" }
+}
+
+void
+f ()
+{
+    using N::i;
+}