Bug 54293 - When a reference is bound to subobject of a temporary, lifetime of the temporary is not extended
Summary: When a reference is bound to subobject of a temporary, lifetime of the tempor...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.7.1
: P3 normal
Target Milestone: 7.0
Assignee: Jason Merrill
URL:
Keywords: wrong-code
: 60297 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-08-16 22:28 UTC by Paul Pluzhnikov
Modified: 2021-01-14 11:50 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Pluzhnikov 2012-08-16 22:28:07 UTC
Google reference: b/6996555

"ISO/IEC 14882:2011(E) 12.2.5 [class.temporary]

The temporary to which the reference is bound or the temporary that is the complete object of a subobject to which the reference is bound persists for the lifetime of the reference except: [etc]"

With gcc-4.6, the lifetime extension is not happening at all:

FAIL int
FAIL Obj

With gcc-4.7 and 4.8 (rev 190453), lifetime is extended for Obj subobject, but not for 'int' (or 'char', or other primitive types):

FAIL int

The test:

#include <set>
#include <iostream>

std::set<const void*> subobjs;

template <typename T>
struct ValueHolder {
  explicit ValueHolder() {
    subobjs.insert(&v);
  }
  ~ValueHolder() {
    subobjs.erase(&v);
  }
  T v;
};

struct Obj { };

bool IsValid(const void* p) {
  return subobjs.find(p) != subobjs.end();
}

int main() {
  const int& ref_int = ValueHolder<int>().v;
  if (!IsValid(&ref_int)) {
    std::cout << "FAIL int" << std::endl;
  }

  const Obj& ref_obj = ValueHolder<Obj>().v;
  if (!IsValid(&ref_obj)) {
    std::cout << "FAIL Obj" << std::endl;
  }
}
Comment 1 Andrew Pinski 2012-08-16 23:02:07 UTC
I think this is a dup of bug 54197 or at least related to it.
Comment 2 Paul Pluzhnikov 2012-08-16 23:06:58 UTC
(In reply to comment #1)
> I think this is a dup of bug 54197 or at least related to it.

Definitely not a dup. I somewhat doubt it's related.

Ollie, you have a GCC build with your patch for PR54197. Could you please check what (if any) effect it has on this test case?
Comment 3 Ollie Wild 2012-08-17 14:18:59 UTC
No, this is a different failure.  With my patch applied, the testcase still fails exactly as described.
Comment 4 Jiří Paleček 2012-08-17 14:54:50 UTC
Is that even a bug? In the standard (8.5.3/5), there is:

— If the initializer expression
— is an xvalue, *class prvalue*, array prvalue or function lvalue and “cv1 T1” is reference-
compatible with “cv2 T2”, or
— has a class type (i.e., T2 is a class type), where T1 is not reference-related to T2, and can be
implicitly converted to an xvalue, class prvalue, or function lvalue of type “cv3 T3”, where
“cv1 T1” is reference-compatible with “cv3 T3”,

I don't think an rvalue of type int (which you have there as the initializer) is a class prvalue, so the initialization should proceed by the next stanza, ie. creating a temporary of type int thereby not extending the lifetime of ValueHolder temporary.
Comment 5 crowl 2012-08-20 18:14:32 UTC
I think there is a compiler bug.  The ruling text seems to be the
first bullet of 8.5.3/5 and its first sub-bullet:

    - If the reference is an lvalue reference and the initializer
      expression

       - is an lvalue (but is not a bit-field), and "cv1 T1" is
         reference-compatible with "cv2 T2," or

which matches the expression in the test case:

   const int& ref_int = ValueHolder<int>().v;

The lifetime is extended by 12.2/5 where none of the exceptions
apply.
Comment 6 Jiří Paleček 2012-08-20 21:56:39 UTC
(In reply to comment #5)
> I think there is a compiler bug.  The ruling text seems to be the
> first bullet of 8.5.3/5 and its first sub-bullet:
> 
>     - If the reference is an lvalue reference and the initializer
>       expression
> 
>        - is an lvalue (but is not a bit-field), and "cv1 T1" is
>          reference-compatible with "cv2 T2," or
> 
> which matches the expression in the test case:
> 
>    const int& ref_int = ValueHolder<int>().v;

The initializer is NOT an lvalue. ValueHolder<int>() is a class (p)rvalue, and rvalue.something is still an rvalue, cf. 5.2.5/4, the second bullet applies:

— If E2 is a non-static data member and the type of E1 is “cq1 vq1 X”, and the type of E2 is “cq2 vq2
T”, the expression designates the named member of the object designated by the first expression. If
E1 is an lvalue, then E1.E2 is an lvalue; if E1 is an xvalue, then E1.E2 is an xvalue; otherwise, it is a
prvalue. [ IMHO it misses the case when T is a reference type, but that doesn't apply here ]

Therefore, I think the binding should proceed by the last case (because the initializer expression is not a *class* (don't ask me why, but it is in the standard text) prvalue, the first sub-bullet of the second bullet doesn't apply either; only the second sub-bullet is left). In that case, you don't bind to the ValueHolder temporary at all, so you don't extend its lifetime.

BTW you never extend the lifetime of anything when you bind according to the first bullet of 8.5.3/5, because lvalues either don't trace to their object, even if they were obtained from a temporary, so it would be impossible to do; or they are named variables whose lifetime is simply not extended.
Comment 7 Daniel Krügler 2012-08-20 22:17:45 UTC
(In reply to comment #6)
> — If E2 is a non-static data member and the type of E1 is “cq1 vq1 X”, and the
> type of E2 is “cq2 vq2
> T”, the expression designates the named member of the object designated by the
> first expression. If
> E1 is an lvalue, then E1.E2 is an lvalue; if E1 is an xvalue, then E1.E2 is an
> xvalue; otherwise, it is a
> prvalue. [ IMHO it misses the case when T is a reference type, but that doesn't
> apply here ]

Did you mean E2 instead of T here? In this case this is described in the beginning of 5.2.5 p4:

"If E2 is declared to have type “reference to T,” then E1.E2 is an lvalue; the type of E1.E2 is T."

I agree with your analysis, but would like to point out that there is change planned to essentially this part of the wording due to 

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#616

Assuming it becomes accepted E1.E2 will become an xvalue in this case (SE bullet 2 of the P/R)
Comment 8 Jiří Paleček 2012-08-20 22:52:31 UTC
(In reply to comment #7)
> (In reply to comment #6)
> > — If E2 is a non-static data member and the type of E1 is “cq1 vq1 X”, and the
> > type of E2 is “cq2 vq2
> > T”, the expression designates the named member of the object designated by the
> > first expression. If
> > E1 is an lvalue, then E1.E2 is an lvalue; if E1 is an xvalue, then E1.E2 is an
> > xvalue; otherwise, it is a
> > prvalue. [ IMHO it misses the case when T is a reference type, but that doesn't
> > apply here ]
> 
> Did you mean E2 instead of T here? In this case this is described in the
> beginning of 5.2.5 p4:
> 
> "If E2 is declared to have type “reference to T,” then E1.E2 is an lvalue; the
> type of E1.E2 is T."

You're right, my bad. I thought it MUST be there but missed that - anyway, it is not relevant here.

> I agree with your analysis, but would like to point out that there is change
> planned to essentially this part of the wording due to 
> 
> http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#616
> 
> Assuming it becomes accepted E1.E2 will become an xvalue in this case (SE
> bullet 2 of the P/R)

Thanks for the info, it is interesting (although I can't see the relevance of this particular change to the issues it should solve, which are basically about using uninitialized objects).
Comment 9 Daniel Krügler 2012-08-21 06:13:50 UTC
(In reply to comment #8)
> > I agree with your analysis, but would like to point out that there is change
> > planned to essentially this part of the wording due to 
> > 
> > http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#616
> > 
> > Assuming it becomes accepted E1.E2 will become an xvalue in this case (SE
> > bullet 2 of the P/R)
> 
> Thanks for the info, it is interesting (although I can't see the relevance of
> this particular change to the issues it should solve, which are basically about
> using uninitialized objects).

Well, this addition *would* change the expected outcome. Because given the CWG 616 P/R the expression

ValueHolder<int>().v

becomes an xvalue (The special rule about class rvalues is no longer relevant here), this means that the compiler shall *not* copy-initialize a temporary as described in the very last bullet of 8.5.3/5.

In other words: In this case IsValid(&ref_int) will hold for the same reasons as it holds for IsValid(&ref_obj).
Comment 10 Jiří Paleček 2012-08-21 07:51:55 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > > I agree with your analysis, but would like to point out that there is change
> > > planned to essentially this part of the wording due to 
> > > 
> > > http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#616
> > > 
> > > Assuming it becomes accepted E1.E2 will become an xvalue in this case (SE
> > > bullet 2 of the P/R)
> > 
> > Thanks for the info, it is interesting (although I can't see the relevance of
> > this particular change to the issues it should solve, which are basically about
> > using uninitialized objects).
> 
> Well, this addition *would* change the expected outcome. Because given the CWG
> 616 P/R the expression
> 
> ValueHolder<int>().v
> 
> becomes an xvalue (The special rule about class rvalues is no longer relevant
> here), this means that the compiler shall *not* copy-initialize a temporary as
> described in the very last bullet of 8.5.3/5.
> 
> In other words: In this case IsValid(&ref_int) will hold for the same reasons
> as it holds for IsValid(&ref_obj).

That is true, and I didn't object that. I rather didn't understand how is that particular change related to solving issues 616, 129, 240 and some others mentioned there.
Comment 11 Daniel Krügler 2012-08-21 08:07:28 UTC
(In reply to comment #10)
> > In other words: In this case IsValid(&ref_int) will hold for the same reasons
> > as it holds for IsValid(&ref_obj).
> 
> That is true, and I didn't object that. I rather didn't understand how is that
> particular change related to solving issues 616, 129, 240 and some others
> mentioned there.

Sorry, I misunderstood your comment. This particular 616 wording change was a "side-step" change that was done as part of the issue (it was recognized while discussing it), the drafting note during the Kona meeting says:

"Drafting note: This change addresses core issue 240, third item in a minimally-intrusive way [..]"
Comment 12 Jason Merrill 2016-10-05 20:20:06 UTC
*** Bug 60297 has been marked as a duplicate of this bug. ***
Comment 13 Jason Merrill 2016-10-05 22:59:27 UTC
Author: jason
Date: Wed Oct  5 22:58:55 2016
New Revision: 240819

URL: https://gcc.gnu.org/viewcvs?rev=240819&root=gcc&view=rev
Log:
	PR c++/54293 - binding reference to member of temporary

	* call.c (reference_binding): Fix binding to member of temporary.

Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/call.c
    trunk/gcc/testsuite/g++.dg/init/ref19.C
Comment 14 Jason Merrill 2016-10-05 23:14:20 UTC
Fixed for GCC 7.