From: Alexandre Oliva <aoliva@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: gcc-patches List <gcc-patches@gcc.gnu.org>
Subject: Re: [C++] [PR84231] overload on cond_expr in template
Date: Tue, 27 Feb 2018 18:06:00 -0000 [thread overview]
Message-ID: <orefl6qpe2.fsf@lxoliva.fsfla.org> (raw)
In-Reply-To: <CADzB+2=8Kbj6gyr+DG4LoRxA+Cmq9OG5QGqPOJpn-18BPfcs3g@mail.gmail.com> (Jason Merrill's message of "Thu, 15 Feb 2018 14:20:21 -0500")
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
next prev parent reply other threads:[~2018-02-27 18:06 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-09 2:09 Alexandre Oliva
2018-02-15 19:20 ` Jason Merrill
2018-02-27 18:06 ` Alexandre Oliva [this message]
2018-02-27 18:22 ` Jason Merrill
2018-02-28 5:24 ` Alexandre Oliva
2018-02-28 16:51 ` Jason Merrill
2018-03-02 7:57 ` Alexandre Oliva
2018-03-02 17:14 ` Jason Merrill
2018-03-06 6:14 ` Alexandre Oliva
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=orefl6qpe2.fsf@lxoliva.fsfla.org \
--to=aoliva@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).