diff mbox

[C++] Fix -Wunused-but-set-parameter bug with references (PR c++/47783)

Message ID 20110217195435.GG30899@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 17, 2011, 7:54 p.m. UTC 
Hi!

On the following testcase we incorrectly warn that r argument
is set but not used.  For normal references (instead of
references inside of aggregates) we don't warn because we never
consider REFERENCE_TYPE parameters or variables for these warnings.

The following patch fixes it, but am not sure if you want to call there
mark_exp_read, mark_rvalue_use or something else or if you prefer to
do it somewhere else.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2011-02-17  Jakub Jelinek  <jakub@redhat.com>

	PR c++/47783
	* cvt.c (convert_from_reference): Call mark_exp_read.

	* g++.dg/warn/Wunused-parm-4.C: New test.


	Jakub

Comments

Jason Merrill Feb. 17, 2011, 8:29 p.m. UTC | #1 
On 02/17/2011 02:54 PM, Jakub Jelinek wrote:
> On the following testcase we incorrectly warn that r argument
> is set but not used.  For normal references (instead of
> references inside of aggregates) we don't warn because we never
> consider REFERENCE_TYPE parameters or variables for these warnings.
>
> 	* cvt.c (convert_from_reference): Call mark_exp_read.

> +foo (R r, int&s)
> +{
> +  r.i = 7;

Are we giving the warning because of this assignment?  That's wrong, 
this assignment does not modify r.

Jason
Jakub Jelinek Feb. 17, 2011, 8:37 p.m. UTC | #2 
On Thu, Feb 17, 2011 at 03:29:03PM -0500, Jason Merrill wrote:
> On 02/17/2011 02:54 PM, Jakub Jelinek wrote:
> >On the following testcase we incorrectly warn that r argument
> >is set but not used.  For normal references (instead of
> >references inside of aggregates) we don't warn because we never
> >consider REFERENCE_TYPE parameters or variables for these warnings.
> >
> >	* cvt.c (convert_from_reference): Call mark_exp_read.
> 
> >+foo (R r, int&s)
> >+{
> >+  r.i = 7;
> 
> Are we giving the warning because of this assignment?  That's wrong,
> this assignment does not modify r.

The warning works by complaining about all variables that are TREE_USED,
but haven't been marked DECL_READ_P via mark_exp_read.  In this case
the parameter has been marked TREE_USED because of the r.i use in
the function, but nothing calls mark_exp_read on it (which is what the
patch fixes).

	Jakub
diff mbox

Patch

--- gcc/cp/cvt.c.jj	2011-02-15 15:42:18.000000000 +0100
+++ gcc/cp/cvt.c	2011-02-17 18:01:50.000000000 +0100
@@ -512,6 +512,7 @@  convert_from_reference (tree val)
       tree t = TREE_TYPE (TREE_TYPE (val));
       tree ref = build1 (INDIRECT_REF, t, val);
 
+      mark_exp_read (val);
        /* We *must* set TREE_READONLY when dereferencing a pointer to const,
 	  so that we get the proper error message if the result is used
 	  to assign to.  Also, &* is supposed to be a no-op.  */
--- gcc/testsuite/g++.dg/warn/Wunused-parm-4.C.jj	2011-02-17 18:11:28.000000000 +0100
+++ gcc/testsuite/g++.dg/warn/Wunused-parm-4.C	2011-02-17 18:11:09.000000000 +0100
@@ -0,0 +1,24 @@ 
+// PR c++/47783
+// { dg-do compile }
+// { dg-options "-Wunused -W" }
+
+struct R
+{
+  int &i;
+};
+
+void
+foo (R r, int &s)
+{
+  r.i = 7;
+  s = 8;
+}
+
+int
+bar ()
+{
+  int x = 1, y = 1;
+  R r = { x };
+  foo (r, y);
+  return x + y;
+}