diff mbox series

[committed] Fix up xvalue handling in ?: with omitted middle operand (PR c++/92831)

Message ID 20191206234640.GK10088@tucnak
State New
Headers show
Series [committed] Fix up xvalue handling in ?: with omitted middle operand (PR c++/92831) | expand

Commit Message

Jakub Jelinek Dec. 6, 2019, 11:46 p.m. UTC 
On Fri, Dec 06, 2019 at 02:31:55PM -0500, Jason Merrill wrote:
> > 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);

Indeed, that fixes it and you've preapproved a patch doing that on IRC, which I've
committed to trunk now that it successfully bootstrapped/regtested on
x86_64-linux and i686-linux.  Thanks.

2019-12-07  Jason Merrill  <jason@redhat.com>
	    Jakub Jelinek  <jakub@redhat.com>

	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.



	Jakub
diff mbox series

Patch

--- gcc/cp/call.c.jj	2019-12-06 21:15:48.842199643 +0100
+++ gcc/cp/call.c	2019-12-06 22:01:18.737636630 +0100
@@ -5077,7 +5077,7 @@  build_conditional_expr_1 (const op_locat
 	warn_for_omitted_condop (loc, arg1);
 
       /* Make sure that lvalues remain lvalues.  See g++.oliva/ext1.C.  */
-      if (lvalue_p (arg1))
+      if (glvalue_p (arg1))
 	arg2 = arg1 = cp_stabilize_reference (arg1);
       else
 	arg2 = arg1 = cp_save_expr (arg1);
--- gcc/testsuite/g++.dg/ext/temp-extend1.C.jj	2019-12-06 22:10:45.489814074 +0100
+++ gcc/testsuite/g++.dg/ext/temp-extend1.C	2019-12-06 22:10:09.495374051 +0100
@@ -0,0 +1,43 @@ 
+// PR c++/92831
+// { dg-do run { target c++11 } }
+// { dg-options "" }
+
+template<typename T> using id = T;
+struct S { S () : s (false) { ++c; } S (bool x) : s (x) { ++c; } ~S () { --c; }; bool s; static int c; };
+int S::c = 0;
+
+void
+foo (int i)
+{
+  const bool&& a
+    = id<S[3]>{false, true, false}[i].s
+      ? id<S[2]>{true, false}[i].s : id<S[4]>{true, false, true, false}[i].s;
+  if (S::c != (i ? 2 : 4))
+    __builtin_abort ();
+}
+
+void
+baz (int i)
+{
+  const bool&& a = id<S[3]>{false, true, false}[i].s
+		   ? : id<S[4]>{true, false, true, false}[i].s;
+  if (S::c != (i ? 3 : 4))
+    __builtin_abort ();
+}
+
+int
+main ()
+{
+  foo (0);
+  if (S::c != 0)
+    __builtin_abort ();
+  foo (1);
+  if (S::c != 0)
+    __builtin_abort ();
+  baz (0);
+  if (S::c != 0)
+    __builtin_abort ();
+  baz (1);
+  if (S::c != 0)
+    __builtin_abort ();
+}