diff mbox

C++ PR 54197: lifetime of reference not properly extended

Message ID CAFOgFcTDQBwaA1Mq+9yy5FzAj5tHCuqfV3E+bKSFqR6gZogpog@mail.gmail.com
State New
Headers show

Commit Message

Ollie Wild Aug. 13, 2012, 9:01 p.m. UTC
On Mon, Aug 13, 2012 at 3:50 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> The formatting doesn't match GCC coding conventions in several ways.
> You don't have spaces before (, and ( shouldn't be at the end of line if
> possible.

Updated patch attached.

Ollie
commit d023097c555a6f7cb84685fd7befedb550889d2c
Author: Ollie Wild <aaw@google.com>
Date:   Mon Aug 13 15:36:24 2012 -0500

    2012-08-13  Ollie Wild  <aaw@google.com>
    
    	PR c++/54197
    	* gcc/cp/call.c (extend_ref_init_temps_1): Handle COMPOUND_EXPR trees.
    	* gcc/testsuite/g++.dg/init/lifetime3.C: New test.

Comments

Ollie Wild Aug. 15, 2012, 2:52 p.m. UTC | #1
(Adding other C++ maintainers in case someone else wants to have a stab.)

Ping?

Ollie


On Mon, Aug 13, 2012 at 4:01 PM, Ollie Wild <aaw@google.com> wrote:
>
> On Mon, Aug 13, 2012 at 3:50 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > The formatting doesn't match GCC coding conventions in several ways.
> > You don't have spaces before (, and ( shouldn't be at the end of line if
> > possible.
>
> Updated patch attached.
>
> Ollie
Diego Novillo Aug. 16, 2012, 5:12 p.m. UTC | #2
On Wed, Aug 15, 2012 at 10:52 AM, Ollie Wild <aaw@google.com> wrote:
> (Adding other C++ maintainers in case someone else wants to have a stab.)
>
> Ping?
>
> Ollie

I wonder if it wouldn't make more sense to iterate until we find the
rightmost element in a compound_expr chain, but I don't think they are
neither common nor long enough to matter.

It looks like a good fix, but I would rather have a C++ maintainer
give final approval for gcc-4_7-branch and trunk.  In the meantime,
should we install it in google/gcc-4_7?  I can deal with any merge
conflicts, in case the patch needs more work for the upstream
branches.


Thanks.  Diego.
Ollie Wild Aug. 16, 2012, 6:20 p.m. UTC | #3
On Thu, Aug 16, 2012 at 12:12 PM, Diego Novillo <dnovillo@google.com> wrote:
>
> I wonder if it wouldn't make more sense to iterate until we find the
> rightmost element in a compound_expr chain, but I don't think they are
> neither common nor long enough to matter.

Yeah, that was my thinking.  I can certainly do whatever the maintainers want.

> It looks like a good fix, but I would rather have a C++ maintainer
> give final approval for gcc-4_7-branch and trunk.  In the meantime,
> should we install it in google/gcc-4_7?  I can deal with any merge
> conflicts, in case the patch needs more work for the upstream
> branches.

I'll go ahead and submit to google/gcc-4_7, then.

Thanks,
Ollie
Gabriel Dos Reis Aug. 16, 2012, 7:13 p.m. UTC | #4
On Wed, Aug 15, 2012 at 9:52 AM, Ollie Wild <aaw@google.com> wrote:
> (Adding other C++ maintainers in case someone else wants to have a stab.)
>
> Ping?

I consider Jason to be the expert on this; so let see what he says.

-- Gaby


>
> Ollie
>
>
> On Mon, Aug 13, 2012 at 4:01 PM, Ollie Wild <aaw@google.com> wrote:
>>
>> On Mon, Aug 13, 2012 at 3:50 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>> >
>> > The formatting doesn't match GCC coding conventions in several ways.
>> > You don't have spaces before (, and ( shouldn't be at the end of line if
>> > possible.
>>
>> Updated patch attached.
>>
>> Ollie
Ollie Wild Aug. 20, 2012, 2:58 p.m. UTC | #5
On Thu, Aug 16, 2012 at 2:13 PM, Gabriel Dos Reis
<gdr@integrable-solutions.net> wrote:
>
> On Wed, Aug 15, 2012 at 9:52 AM, Ollie Wild <aaw@google.com> wrote:
> > (Adding other C++ maintainers in case someone else wants to have a
> > stab.)
> >
> > Ping?
>
> I consider Jason to be the expert on this; so let see what he says.
>
> -- Gaby

Jason, any idea when you can look at this?

The patch is about as short as they come, so it shouldn't take long to review.

Thanks,
Ollie
Ollie Wild Aug. 28, 2012, 2:41 p.m. UTC | #6
On Mon, Aug 20, 2012 at 9:58 AM, Ollie Wild <aaw@google.com> wrote:
>
> Jason, any idea when you can look at this?
>
> The patch is about as short as they come, so it shouldn't take long to
> review.

Ping?
Jason Merrill Aug. 31, 2012, 3:37 p.m. UTC | #7
OK, sorry for the delay.

Jason
Ollie Wild Aug. 31, 2012, 5:19 p.m. UTC | #8
On Fri, Aug 31, 2012 at 10:37 AM, Jason Merrill <jason@redhat.com> wrote:
> OK, sorry for the delay.

No worries.  Thanks.

Submitted to trunk and gcc-4_7-branch as r190834 and r190839.

Ollie
diff mbox

Patch

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 5345f2b..f3a73af 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8924,6 +8924,12 @@  extend_ref_init_temps_1 (tree decl, tree init, VEC(tree,gc) **cleanups)
   tree sub = init;
   tree *p;
   STRIP_NOPS (sub);
+  if (TREE_CODE (sub) == COMPOUND_EXPR)
+    {
+      TREE_OPERAND (sub, 1)
+        = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 1), cleanups);
+      return init;
+    }
   if (TREE_CODE (sub) != ADDR_EXPR)
     return init;
   /* Deal with binding to a subobject.  */
diff --git a/gcc/testsuite/g++.dg/init/lifetime3.C b/gcc/testsuite/g++.dg/init/lifetime3.C
new file mode 100644
index 0000000..d099699
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/lifetime3.C
@@ -0,0 +1,37 @@ 
+// PR c++/26714
+// { dg-do run }
+
+extern "C" void abort();
+
+bool ok = false;
+struct A {
+  A() { }
+  ~A() { if (!ok) abort(); }
+};
+
+struct B {
+  static A foo() { return A(); }
+};
+
+B b_g;
+
+struct scoped_ptr {
+  B* operator->() const { return &b_g; }
+  B* get() const { return &b_g; }
+};
+
+B *get() { return &b_g; }
+
+int main()
+{
+  scoped_ptr f;
+  const A& ref1 = f->foo();
+  const A& ref2 = f.get()->foo();
+  const A& ref3 = get()->foo();
+  const A& ref4 = B::foo();
+  B *pf = f.get();
+  const A& ref5 = pf->foo();
+
+
+  ok = true;
+}