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

login
register
mail settings
Submitter Ollie Wild
Date Aug. 13, 2012, 9:01 p.m.
Message ID <CAFOgFcTDQBwaA1Mq+9yy5FzAj5tHCuqfV3E+bKSFqR6gZogpog@mail.gmail.com>
Download mbox | patch
Permalink /patch/177114/
State New
Headers show

Comments

Ollie Wild - Aug. 13, 2012, 9:01 p.m.
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.
Ollie Wild - Aug. 15, 2012, 2:52 p.m.
(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.
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.
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.
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.
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.
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.
OK, sorry for the delay.

Jason
Ollie Wild - Aug. 31, 2012, 5:19 p.m.
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

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;
+}