[C++ PATCH] Don't allow taking the address of an rvalue reference (PR c++/35153)

[C++ PATCH] Don't allow taking the address of an rvalue reference (PR c++/35153)

[Doug Gregor]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

This tiny little patch makes sure that we don't try to take the
address of an rvalue. Before C++0x rvalue references, we had all of
the cases covered; now we can end up with a function call that returns
an rvalue reference. In this case, we'll wrap an INDIRECT_REF around
the call... and not diagnose when we've taken the address of that
rvalue. I think this bug is technically a regression, because we used
to reject the invalid code in the test case but now we accept it. This
patch fixes that problem.

Tested i686-pc-linux-gnu; okay for 4.3?

  - Doug

2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/35153
	* typeck.c (build_x_unary_op): Don't allow us to take the address
	of an rvalue that's hiding being an INDIRECT_REF. We can only end
	up triggering that error when we have some kind of rvalue
	reference.

2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/35153
	* g++.dg/cpp0x/rref-35153.C: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: address-of-rref.patch --]
[-- Type: text/x-patch; name=address-of-rref.patch, Size: 867 bytes --]

Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 132347)
+++ cp/typeck.c	(working copy)
@@ -4040,6 +4040,10 @@ build_x_unary_op (enum tree_code code, t
 	}
       else if (TREE_CODE (xarg) == TARGET_EXPR)
 	warning (0, "taking address of temporary");
+      else if (TREE_CODE (xarg) == INDIRECT_REF
+	       && real_lvalue_p (xarg) == clk_none)
+	error ("cannot take the address of rvalue %<%E%>", xarg);
+
       exp = build_unary_op (ADDR_EXPR, xarg, 0);
     }
 
Index: testsuite/g++.dg/cpp0x/rref-35153.C
===================================================================
--- testsuite/g++.dg/cpp0x/rref-35153.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/rref-35153.C	(revision 0)
@@ -0,0 +1,3 @@
+// { dg-options "-std=c++0x" }
+int && f();
+void g() { &(f()); } // { dg-error "address of rvalue" }

Re: [C++ PATCH] Don't allow taking the address of an rvalue reference (PR c++/35153)

[Mark Mitchell]

Doug Gregor wrote:

> 2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>
> 
> 	PR c++/35153
> 	* typeck.c (build_x_unary_op): Don't allow us to take the address
> 	of an rvalue that's hiding being an INDIRECT_REF. We can only end
> 	up triggering that error when we have some kind of rvalue
> 	reference.
> 
> 2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>
> 
> 	PR c++/35153
> 	* g++.dg/cpp0x/rref-35153.C: New.

Why not put this in build_unary_op?  That's where we do most of the 
other checks -- though, of course, OFFSET_REF and TARGET_EXPR are in 
build_x_unary_op, for reasons I don't really understand.

Here's what I think the division of labor *should* be:

1. build_x_unary_op is where user-written "& ..." goes, and, therefore 
checks for overloaded operators.

2. build_unary_op is where user-written "& ..." goes if there's no 
overloaded operator, or where compiler-generated address-generation goes 
if normal user-level semantics are required.

3. build_address is where other compiler-generated address-generation goes.

At present, though, some compiler-generated code goes through 
build_unary_op that probably shouldn't.

So, I think your check should go in build_unary_op -- unless for some 
reason it can't, like if there's a compiler-generated need to take the 
address of the rvalue reference, and we're going through build_unary_op 
instead of build_address.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [C++ PATCH] Don't allow taking the address of an rvalue reference (PR c++/35153)

[Doug Gregor]

On Feb 18, 2008 8:07 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> Doug Gregor wrote:
>
> > 2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>
> >
> >       PR c++/35153
> >       * typeck.c (build_x_unary_op): Don't allow us to take the address
> >       of an rvalue that's hiding being an INDIRECT_REF. We can only end
> >       up triggering that error when we have some kind of rvalue
> >       reference.
> >
> > 2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>
> >
> >       PR c++/35153
> >       * g++.dg/cpp0x/rref-35153.C: New.
>
> Why not put this in build_unary_op?  That's where we do most of the
> other checks -- though, of course, OFFSET_REF and TARGET_EXPR are in
> build_x_unary_op, for reasons I don't really understand.

I tried putting it in build_unary_op the first time, and..

> So, I think your check should go in build_unary_op -- unless for some
> reason it can't, like if there's a compiler-generated need to take the
> address of the rvalue reference,

Bingo. This is exactly what happened---compiler-generated code was
going through build_unary_op, and triggering this error when it was
okay to take the address of the result.

> and we're going through build_unary_op
> instead of build_address.

I didn't think of that. build_address doesn't do the
ADDR_EXPR/INDIRECT_REF simplification, which some of the current
callers of build_unary_op(ADDR_EXPR...) might rely on. So, I can try
to change the compiler-generated calls to build_unary_op(ADDR_EXPR...)
into build_address, and move the address-of-rvalue check into
build_unary_op, if you'd still prefer that implementation.

  - Doug

Re: [C++ PATCH] Don't allow taking the address of an rvalue reference (PR c++/35153)

[Mark Mitchell]

Doug Gregor wrote:

>> So, I think your check should go in build_unary_op -- unless for some
>> reason it can't, like if there's a compiler-generated need to take the
>> address of the rvalue reference,
> 
> Bingo. This is exactly what happened---compiler-generated code was
> going through build_unary_op, and triggering this error when it was
> okay to take the address of the result.

OK.

>> and we're going through build_unary_op
>> instead of build_address.
> 
> I didn't think of that. build_address doesn't do the
> ADDR_EXPR/INDIRECT_REF simplification, which some of the current
> callers of build_unary_op(ADDR_EXPR...) might rely on. So, I can try
> to change the compiler-generated calls to build_unary_op(ADDR_EXPR...)
> into build_address, and move the address-of-rvalue check into
> build_unary_op, if you'd still prefer that implementation.

Yes, I think that's the Right Answer.  If anything in the front end 
depends on that simplification for compiler-generated code, it seems 
busted on its face to me -- and one certainly hopes that the middle-end 
is plenty smart enough to work this out these days.

In general, the front end is going to be simpler and more reliable if we 
clearly differentiate between compiler-generated implicit stuff (the 
sort of thing that would be generated during IL lowering) and user-level 
stuff.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713