Re: [C++] [PR84231] overload on cond_expr in template

[Alexandre Oliva <aoliva at redhat dot com>]

On Feb 15, 2018, Jason Merrill <jason@redhat.com> wrote:

> On Thu, Feb 8, 2018 at 9:09 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> +         /* If it was supposed to be an rvalue but it's not, adjust
>> +            one of the operands so that any overload resolution
>> +            taking this COND_EXPR as an operand makes the correct
>> +            decisions.  See c++/84231.  */
>> +         TREE_OPERAND (min, 2) = build1_loc (loc, NON_LVALUE_EXPR,
>> +                                             TREE_TYPE (min),
>> +                                             TREE_OPERAND (min, 2));
>> +         EXPR_LOCATION_WRAPPER_P (TREE_OPERAND (min, 2)) = 1;

> But that's not true, this isn't a location wrapper, it has semantic
> effect.  And would be the first such use of NON_LVALUE_EXPR in a
> template.

Yeah.  At first I thought NON_LVALUE_EXPR was the way to go, as the
traditional way to denote non-lvalues, but when that didn't work, I
investigated and saw if I marked it as a location wrapper, it would have
the intended effect of stopping the template-dependent cond_expr from
being regarded as an lvalue, while being dropped when tsubsting the
cond_expr, so it had no ill effects AFAICT.

> Since we're already using the type of the COND_EXPR to indicate a
> glvalue, maybe lvalue_kind should say that within a template, a
> COND_EXPR which got past the early check for reference type is a
> prvalue.

I suppose you mean something like this:

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 9b9e36a1173f..76148c876b71 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -194,6 +194,14 @@ lvalue_kind (const_tree ref)
       break;
 
     case COND_EXPR:
+      /* Except for type-dependent exprs, a REFERENCE_TYPE will
+	 indicate whether its result is an lvalue or so.
+	 REFERENCE_TYPEs are handled above, so if we reach this point,
+	 we know we got an rvalue, unless we have a type-dependent
+	 expr.  */
+      if (processing_template_decl
+	  && !type_dependent_expression_p (CONST_CAST_TREE (ref)))
+	return clk_none;
       op1_lvalue_kind = lvalue_kind (TREE_OPERAND (ref, 1)
 				    ? TREE_OPERAND (ref, 1)
 				    : TREE_OPERAND (ref, 0));

but there be dragons here.  build_x_conditional_expr wants tests
glvalue_p on the proxy and the template expr, and glvalue_p uses
lvalue_kind, so we have to disable this new piece of logic for the
baseline so that we don't unintentionally change the lvalueness of the
COND_EXPR.

diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 0e7c63dd1973..a34cb6ec175f 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -6565,11 +6565,25 @@ build_x_conditional_expr (location_t loc, tree ifexp, tree op1, tree op2,
     {
       tree min = build_min_non_dep (COND_EXPR, expr,
 				    orig_ifexp, orig_op1, orig_op2);
-      /* Remember that the result is an lvalue or xvalue.  */
-      if (glvalue_p (expr) && !glvalue_p (min))
-	TREE_TYPE (min) = cp_build_reference_type (TREE_TYPE (min),
-						   !lvalue_p (expr));
+      /* Remember that the result is an lvalue or xvalue.  We have to
+	 pretend EXPR is type-dependent, lest we short-circuit the
+	 very logic we want to rely on.  */
+      tree save_expr_type = TREE_TYPE (expr);
+
+      if (!type_dependent_expression_p (expr)
+	  && TREE_CODE (save_expr_type) != REFERENCE_TYPE)
+	TREE_TYPE (expr) = NULL_TREE;
+      
+      bool glvalue = glvalue_p (expr);
+      bool reftype = glvalue && !glvalue_p (min);
+      bool lval = reftype ? lvalue_p (expr) : false;
+
+      TREE_TYPE (expr) = save_expr_type;
+
+      if (reftype)
+	TREE_TYPE (min) = cp_build_reference_type (TREE_TYPE (min), !lval);
       expr = convert_from_reference (min);
+      gcc_assert (glvalue_p (min) == glvalue);
     }
   return expr;
 }


Even then, there are other surprises I'm trying to track down (libstdc++
optimized headers won't build with the two patchlets above); my guess is
that it's out of non-template-dependent cond_exprs' transitions from
non-lvalue to lvalue as we finish template substitution and
processing_template_decl becomes zero.

This is getting hairy enough that I'm wondering if that's really what
you had in mind, so I decided to touch base in case I had to be put back
on the right track (or rather out of the wrong track again ;-)

Thanks,

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist|Red Hat Brasil GNU Toolchain Engineer