On 12/6/19 11:28 AM, Jakub Jelinek wrote:
Hi!
This is a reason latest firefox is miscompiled by G++ 9, seems DR 1299
says in [class.temporary]/(6.7) that reference binding to a temporary
object from the second or third operand of a conditional expression
should have extended lifetime too, but extend_ref_init_temps_1 was
apparently handling only comma expressions, some casts, . (COMPONENT_REF),
[] (ARRAY_REF), but not COND_EXPR.
The following patch handles COND_EXPR in there too, in a similar way how
the gimplifier handles the cleanups of the expressions in the COND_EXPR
operand if there is no lifetime extension due to reference binding.
In particular there are bool temporaries added, initialized to false and set
to true if the particular (leaf) COND_EXPR argument has been invoked and the
corresponding cleanup guarded that way.
I admit I haven't tried to construct testcases for all the things CWG 1299
added, e.g. don't know if ( expression ) will work, if all the *_cast cases
that need to be extended work (some of them do, as the testcase shows), or
if e.g. .* works, so I'm not claiming the DR is completely implemented.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
and after a while for release branches?
OK.
Note, just to double check g++ doesn't ICE on expr1 ? : expr2 GNU extension,
I also wrote the attached testcase, but unlike the testcase included in the
patch which behaves the same with patched g++ as does recent clang++, the
testcase with expr1 ? : expr2 (omitted second operand) actually never
extends the lifetime of any temporary (that is the baz function), unlike
clang++ which extends the lifetime of expr2 if expr1 is false, and doesn't
extend lifetime of anything if expr1 is true. This is outside of the scope
of the standard, so no idea what is correct, so I'm just asking how it
should behave.
I would expect one or the other to be extended. I imagine that clang
not extending expr1 is a bug due to however they avoid evaluating it twice.
extend_ref_init_temps_1 is never called in that case when the expression is
COND_EXPR.
I think the problem is in build_conditional_expr_1, which needs to treat
xvalues properly in the handling of this extension, i.e.
- if (lvalue_p (arg1))
+ if (glvalue_p (arg1))
arg2 = arg1 = cp_stabilize_reference (arg1);
Jason
