On Tue, Feb 27, 2018 at 1:05 PM, Alexandre Oliva <aol...@redhat.com> wrote:
> On Feb 15, 2018, Jason Merrill <ja...@redhat.com> wrote:
>
>> On Thu, Feb 8, 2018 at 9:09 PM, Alexandre Oliva <aol...@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
