http://eel.is/c++draft/class.temporary#6.7 says that even ?: second and third operands need to be handled, but short testcase modified from the standard one: template<typename T> using id = T; int i = 1; int&& a = id<int[3]>{1, 2, 3}[i]; // temporary array has same lifetime as a const int& b = static_cast<const int&>(0); // temporary int has same lifetime as b int&& c = cond ? id<int[3]>{1, 2, 3}[i] : static_cast<int&&>(0); // exactly one of the two temporaries is lifetime-extended like: template<typename T> using id = T; struct S { S () { s++; } ~S () { s--; } S (int) { s++; } static int s; }; int S::s = 0; void bar (bool cond) { if (S::s != (cond ? 7 : 5)) __builtin_abort (); } void foo (bool cond) { int i = 1; // temporary array has same lifetime as a S&& a = id<S[3]>{1, 2, 3}[i]; // temporary S has same lifetime as b const S& b = static_cast<const S&>(0); // exactly one of the two temporaries is lifetime-extended S&& c = cond ? id<S[3]>{1, 2, 3}[i] : static_cast<S&&>(0); bar (cond); } int main () { foo (true); foo (false); } fails with g++ and succeeds with clang++.
Seems this is http://wg21.link/p0727 aka CWG 1299.
Even better testcase that has nested COND_EXPRs: template<typename T> using id = T; struct S { S () { s++; } ~S () { s--; } S (int) { s++; } static int s; }; int S::s = 0; void bar (bool cond, bool cond2) { if (S::s != (cond ? cond2 ? 7 : 5 : cond2 ? 8 : 9)) __builtin_abort (); } void foo (bool cond, bool cond2) { int i = 1; // temporary array has same lifetime as a S&& a = id<S[3]>{1, 2, 3}[i]; // temporary S has same lifetime as b const S& b = static_cast<const S&>(0); // exactly one of the four temporaries is lifetime-extended S&& c = cond ? cond2 ? id<S[3]>{1, 2, 3}[i] : static_cast<S&&>(0) : cond2 ? id<S[4]>{1, 2, 3, 4}[i] : id<S[5]>{1, 2, 3, 4, 5}[i]; bar (cond, cond2); } int main () { foo (true, true); foo (true, false); foo (false, true); foo (false, false); } --- gcc/cp/call.c.jj 2019-12-05 10:03:04.110181312 +0100 +++ gcc/cp/call.c 2019-12-05 20:35:08.386092965 +0100 @@ -12153,12 +12153,19 @@ extend_ref_init_temps_1 (tree decl, tree = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 1), cleanups); return init; } + if (TREE_CODE (sub) == COND_EXPR) + { + TREE_OPERAND (sub, 1) + = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 1), cleanups); + TREE_OPERAND (sub, 2) + = extend_ref_init_temps_1 (decl, TREE_OPERAND (sub, 2), cleanups); + return init; + } if (TREE_CODE (sub) != ADDR_EXPR) return init; /* Deal with binding to a subobject. */ for (p = &TREE_OPERAND (sub, 0); - (TREE_CODE (*p) == COMPONENT_REF - || TREE_CODE (*p) == ARRAY_REF); ) + TREE_CODE (*p) == COMPONENT_REF || TREE_CODE (*p) == ARRAY_REF; ) p = &TREE_OPERAND (*p, 0); if (TREE_CODE (*p) == TARGET_EXPR) { isn't sufficient, because if there are any cleanups added by either of the recursive calls, we need to conditionalize those cleanups on whether the particular COND_EXPR's first operand evaluated to true or false. I think cp_save_expr won't do it for the nested cases though, because the SAVE_EXPR might end up not being initialized. So we need to arrange for a bool temporary that will be initialized to say false early, before the first COND_EXPR is encountered, set it to true at the start of corresponding init code and wrap cleanups with that guard. Any better ideas?
Created attachment 47434 [details] gcc10-pr92831.patch Untested fix.
Author: jakub Date: Fri Dec 6 20:16:27 2019 New Revision: 279064 URL: https://gcc.gnu.org/viewcvs?rev=279064&root=gcc&view=rev Log: PR c++/92831 - CWG 1299, not extending temporary lifetime for ?: * cp-tree.h (extend_ref_init_temps): Add a new argument with NULL default arg. * call.c (set_up_extended_ref_temp): Add COND_GUARD argument, pass it down to extend_ref_init_temps. Before pushing cleanup, if COND_GUARD is non-NULL, create a bool temporary if needed, initialize to false and guard the cleanup with the temporary being true. (extend_ref_init_temps_1): Add COND_GUARD argument, pass it down to recursive calls and set_up_extended_ref_temp. Handle COND_EXPR. (extend_ref_init_temps): Add COND_GUARD argument, pass it down to recursive calls and to extend_ref_init_temps_1. * g++.dg/cpp0x/temp-extend2.C: New test. Added: trunk/gcc/testsuite/g++.dg/cpp0x/temp-extend2.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/cp/cp-tree.h trunk/gcc/testsuite/ChangeLog
Author: jakub Date: Fri Dec 6 23:43:45 2019 New Revision: 279069 URL: https://gcc.gnu.org/viewcvs?rev=279069&root=gcc&view=rev Log: PR c++/92831 * call.c (build_conditional_expr_1): For ?: with omitted middle operand use cp_stabilize_reference if arg1 is glvalue_p rather than just if it is lvalue_p. * g++.dg/ext/temp-extend1.C: New test. Added: trunk/gcc/testsuite/g++.dg/ext/temp-extend1.C Modified: trunk/gcc/cp/ChangeLog trunk/gcc/cp/call.c trunk/gcc/testsuite/ChangeLog
Should be fixed on the trunk so far, will backport to release branches in two weeks or so.
Thank you! I wonder if your fix can also have an optional warning which would let us to fix occurrences of this in Firefox since requiring unreleased compilers is not cool
Author: jakub Date: Fri Dec 20 17:37:45 2019 New Revision: 279669 URL: https://gcc.gnu.org/viewcvs?rev=279669&root=gcc&view=rev Log: Backported from mainline 2019-12-06 Jakub Jelinek <jakub@redhat.com> PR c++/92831 - CWG 1299, not extending temporary lifetime for ?: * cp-tree.h (extend_ref_init_temps): Add a new argument with NULL default arg. * call.c (set_up_extended_ref_temp): Add COND_GUARD argument, pass it down to extend_ref_init_temps. Before pushing cleanup, if COND_GUARD is non-NULL, create a bool temporary if needed, initialize to false and guard the cleanup with the temporary being true. (extend_ref_init_temps_1): Add COND_GUARD argument, pass it down to recursive calls and set_up_extended_ref_temp. Handle COND_EXPR. (extend_ref_init_temps): Add COND_GUARD argument, pass it down to recursive calls and to extend_ref_init_temps_1. * g++.dg/cpp0x/temp-extend2.C: New test. Added: branches/gcc-9-branch/gcc/testsuite/g++.dg/cpp0x/temp-extend2.C Modified: branches/gcc-9-branch/gcc/cp/ChangeLog branches/gcc-9-branch/gcc/cp/call.c branches/gcc-9-branch/gcc/cp/cp-tree.h branches/gcc-9-branch/gcc/testsuite/ChangeLog
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>: https://gcc.gnu.org/g:7192b1ec12484f5ca8b20930d8dc4d28ab4a533a commit r10-5987-g7192b1ec12484f5ca8b20930d8dc4d28ab4a533a Author: Jason Merrill <jason@redhat.com> Date: Wed Jan 15 09:31:11 2020 -0500 PR c++/92871 - bad code with xvalue and GNU ?: extension. I steered Jakub wrong on the desired behavior for temp-extend1.C in the context of bug 92831; it doesn't make sense to try to extend the lifetime of a temporary that we've already materialized to evaluate the test. So this patch munges the stabilized expression so that it won't be subject to lifetime extension. * call.c (prevent_lifetime_extension): New. (build_conditional_expr_1): Use it.
Fixed?
Not backported to 8, but I vaguely recall it was difficult to backport it and I gave up. So let's close it as fixed in 9.3+ and the omitted second operand on ?: for 10.1+ only.