Patchwork [C++,Path] PR 38958

login
register
mail settings
Submitter Paolo Carlini
Date June 12, 2013, 3:58 p.m.
Message ID <51B89A91.2010204@oracle.com>
Download mbox | patch
Permalink /patch/250819/
State New
Headers show

Comments

Paolo Carlini - June 12, 2013, 3:58 p.m.
Hi,

this bug is a sort of follow up to 10416, which I fixed some time ago 
and was about avoiding -Wunused warnings for class types with 
destructors with side-effects.

In this issue reporter notes that we don't handle in the same way 
references, thus, considering the testcase, we do not warn for:

     Lock lock = AcquireLock();

and we do for:

     const Lock& lock = AcquireLock();

whereas the destructor is involved in both cases in a similar way, etc.

Thus I changed the code in poplevel to see through references for 
-Wunused-variable. Tested x86_64-linux.

Thanks,
Paolo.

////////////////////
/cp
2013-06-12  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/38958
	* decl.c (poplevel): For -Wunused-variable see through references.

/testsuite
2013-06-12  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/38958
	* g++.dg/warn/Wunused-var-20.C: New.
Jason Merrill - June 12, 2013, 4:40 p.m.
On 06/12/2013 11:58 AM, Paolo Carlini wrote:
> -		     && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
> +		     && TREE_CODE (type) != REFERENCE_TYPE

This change is wrong; we specifically want to suppress the 
unused-but-set warning for reference variables.  Drop it and the patch 
is OK.

Jason
Paolo Carlini - June 12, 2013, 5:29 p.m.
Hi,

Jason Merrill <jason@redhat.com> ha scritto:

>On 06/12/2013 11:58 AM, Paolo Carlini wrote:
>> -		     && TREE_CODE (TREE_TYPE (decl)) != REFERENCE_TYPE
>> +		     && TREE_CODE (type) != REFERENCE_TYPE
>
>This change is wrong; we specifically want to suppress the
>unused-but-set warning for reference variables.  Drop it and the patch
>is OK.

I understand the general issue, but I'm not sure we can't use 'type' here: we don't even consider emitting the unused-but-set warning when TREE_USED (decl) is false and in this case I don't call non_reference at the outset. I think ;)

Paolo
Paolo Carlini - June 12, 2013, 5:37 p.m.
Humpf,

>I understand the general issue, but I'm not sure we can't use 'type'
>here: we don't even consider emitting the unused-but-set warning when
>TREE_USED (decl) is false and in this case I don't call non_reference
>at the outset.

I meant *only* in this case I call non_reference, you see my point.

Rewording: as the one line comment says, I only call non_reference at the outset when I know that either we'll end up producing the normal unused-variable warning or nothing at all.

Paolo
Jason Merrill - June 12, 2013, 5:46 p.m.
On 06/12/2013 01:37 PM, Paolo Carlini wrote:
> Rewording: as the one line comment says, I only call non_reference at the outset when I know that either we'll end up producing the normal unused-variable warning or nothing at all.

Oh, I see.  But that's a rather subtle difference; better to have 'type' 
mean something consistent and leave the unused-but-set code checking 
TREE_TYPE (decl).

Jason

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 200012)
+++ cp/decl.c	(working copy)
@@ -622,18 +622,22 @@  poplevel (int keep, int reverse, int functionbody)
 	   push_local_binding where the list of decls returned by
 	   getdecls is built.  */
 	decl = TREE_CODE (d) == TREE_LIST ? TREE_VALUE (d) : d;
+	tree type = TREE_TYPE (decl);
+	if (VAR_P (decl) && ! TREE_USED (decl))
+	  // For -Wunused-variable see through references (PR 38958).
+	  type = non_reference (type);
 	if (VAR_P (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))))
+	    && type != error_mark_node
+	    && (!CLASS_TYPE_P (type)
+		|| !TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type)))
 	  {
 	    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
+		     && TREE_CODE (type) != REFERENCE_TYPE
 		     && errorcount == unused_but_set_errorcount)
 	      {
 		warning (OPT_Wunused_but_set_variable,
Index: testsuite/g++.dg/warn/Wunused-var-20.C
===================================================================
--- testsuite/g++.dg/warn/Wunused-var-20.C	(revision 0)
+++ testsuite/g++.dg/warn/Wunused-var-20.C	(working copy)
@@ -0,0 +1,19 @@ 
+// PR c++/38958
+// { dg-options "-Wunused" }
+
+volatile int g;
+
+struct Lock
+{
+  ~Lock() { g = 0; }
+};
+
+Lock AcquireLock() { return Lock(); }
+
+int main()
+{
+  const Lock& lock = AcquireLock();
+  g = 1;
+  g = 2;
+  g = 3;
+}