On 1/5/21 10:26 AM, Jakub Jelinek wrote:
On Mon, Jan 04, 2021 at 04:01:25PM -0500, Jason Merrill via Gcc-patches wrote:
On 1/4/21 3:48 PM, Jakub Jelinek wrote:
On Mon, Jan 04, 2021 at 03:44:46PM -0500, Jason Merrill wrote:
This change is OK, but part of the problem is that we're trying to do
overload resolution for an S copy/move constructor, which we shouldn't be
because bit_cast is a prvalue, so in C++17 and up we should use it to
directly initialize the target without any implied constructor call.

It seems we're mishandling this because the code in
build_special_member_call specifically looks for TARGET_EXPR or CONSTRUCTOR,
and BIT_CAST_EXPR is neither of those.

Wrapping a BIT_CAST_EXPR of aggregate type in a TARGET_EXPR would address
this, and any other places that expect a class prvalue to come in the form
of a TARGET_EXPR.

I can try that tomorrow.  Won't that cause copying through extra temporary
in some cases though, or is that guaranteed to be optimized?

It won't cause any extra copying when it's used to initialize another object
(like the return value of std::bit_cast).  Class prvalues are always
expressed with a TARGET_EXPR in the front end; the TARGET_EXPR melts away
when used as an initializer, it only creates a temporary when it's used in
another way.

Ok, this version wraps it into a TARGET_EXPR then, it alone fixes the bug,
but I've kept the constexpr.c change too.

This patch corrects this and one other place to not be as dependent on TARGET_EXPR, but I think I'm going to save it for stage 1.

Jason
commit 0d732b8c7fb3f8378dc1c894358bb5d766e6be5d
Author: Jason Merrill <ja...@redhat.com>
Date:   Mon Jan 4 16:11:08 2021 -0500

    c++: Tweak prvalue test [PR98469]
    
    Discussing the 98469 patch and class prvalues with Jakub also inspired me to
    change the place that was mishandling BIT_CAST_EXPR and one other to use the
    lvalue_kind machinery to decide whether something is a prvalue, instead of
    looking specifically for a TARGET_EXPR.
    
    gcc/cp/ChangeLog:
    
            * call.c (build_special_member_call): Use !glvalue_p rather
            than specific tree codes to test for prvalue.
            (conv_is_prvalue): Likewise.
            (implicit_conversion): Check CLASS_TYPE_P first.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 218157088ef..e2d2b23e449 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -2118,8 +2118,8 @@ implicit_conversion (tree to, tree from, tree expr, bool c_cast_p,
 					    flags, complain);
   if (!conv || conv->bad_p)
     return conv;
-  if (conv_is_prvalue (conv)
-      && CLASS_TYPE_P (conv->type)
+  if (CLASS_TYPE_P (conv->type)
+      && conv_is_prvalue (conv)
       && CLASSTYPE_PURE_VIRTUALS (conv->type))
     conv->bad_p = true;
   return conv;
@@ -8500,8 +8500,7 @@ conv_is_prvalue (conversion *c)
     return true;
   if (c->kind == ck_user && !TYPE_REF_P (c->type))
     return true;
-  if (c->kind == ck_identity && c->u.expr
-      && TREE_CODE (c->u.expr) == TARGET_EXPR)
+  if (c->kind == ck_identity && c->u.expr && !glvalue_p (c->u.expr))
     return true;
 
   return false;
@@ -9950,8 +9949,7 @@ build_special_member_call (tree instance, tree name, vec<tree, va_gc> **args,
 	  && CONSTRUCTOR_NELTS (arg) == 1)
 	arg = CONSTRUCTOR_ELT (arg, 0)->value;
 
-      if ((TREE_CODE (arg) == TARGET_EXPR
-	   || TREE_CODE (arg) == CONSTRUCTOR)
+      if (!glvalue_p (arg)
 	  && (same_type_ignoring_top_level_qualifiers_p
 	      (class_type, TREE_TYPE (arg))))
 	{

Reply via email to