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

[Jason Merrill <jason@redhat.com>]

On Tue, Feb 27, 2018 at 1:05 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> 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 ;-)

Perhaps it would be easier to add the REFERENCE_TYPE in
build_conditional_expr_1, adjusting result_type based on
processing_template_decl and is_lvalue.

Jason