public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Remove maybe-rvalue OR in implicit move
@ 2022-09-28 21:26 Marek Polacek
  2022-10-10 19:19 ` Marek Polacek
  2022-10-10 20:15 ` Jason Merrill
  0 siblings, 2 replies; 3+ messages in thread
From: Marek Polacek @ 2022-09-28 21:26 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill

This patch removes the two-stage overload resolution when performing
implicit move, whereby the compiler does two separate overload resolutions:
one treating the operand as an rvalue, and then (if that resolution fails)
another one treating the operand as an lvalue.  In the standard this was
introduced via CWG 1579 and implemented in gcc in r251035.  In r11-2412,
we disabled the fallback OR in C++20 (but not in C++17).  Then C++23 P2266
removed the fallback overload resolution, and changed the implicit move rules
once again.  So we wound up with three different behaviors.

The two overload resolutions approach was complicated and quirky, so
users should transition to the newer model.  Removing the maybe-rvalue
OR also allows us to simplify our code, for instance, now we can get
rid of LOOKUP_PREFER_RVALUE altogether.

This change means that code that previously didn't compile in C++17 will
now compile, for example:

  struct S1 { S1(S1 &&); };
  struct S2 : S1 {};

  S1
  f (S2 s)
  {
    return s; // OK, derived-to-base, use S1::S1(S1&&)
  }

And conversely, code that used to work in C++17 may not compile anymore:

  struct W {
    W();
  };

  struct F {
    F(W&);
    F(W&&) = delete;
  };

  F fn ()
  {
    W w;
    return w; // use w as rvalue -> use of deleted function F::F(W&&)
  }

I plan to add a note to porting_to.html.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/cp/ChangeLog:

	* call.cc (standard_conversion): Remove LOOKUP_PREFER_RVALUE code.
	(reference_binding): Honor clk_implicit_rval even pre-C++20.
	(implicit_conversion_1): Remove LOOKUP_PREFER_RVALUE code.
	(build_user_type_conversion_1): Likewise.
	(convert_like_internal): Likewise.
	(build_over_call): Likewise.
	* cp-tree.h (LOOKUP_PREFER_RVALUE): Remove.
	(LOOKUP_NO_NARROWING): Adjust definition.
	* except.cc (build_throw): Don't perform two overload resolutions.
	* typeck.cc (maybe_warn_pessimizing_move): Don't use
	LOOKUP_PREFER_RVALUE.
	(check_return_expr): Don't perform two overload resolutions.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/Wredundant-move10.C: Adjust dg-warning.
	* g++.dg/cpp0x/Wredundant-move7.C: Likewise.
	* g++.dg/cpp0x/move-return2.C: Remove dg-error.
	* g++.dg/cpp0x/move-return4.C: Likewise.
	* g++.dg/cpp0x/ref-qual20.C: Adjust expected return value.
	* g++.dg/cpp0x/move-return5.C: New test.
---
 gcc/cp/call.cc                                | 41 ++-----------------
 gcc/cp/cp-tree.h                              |  6 +--
 gcc/cp/except.cc                              | 23 ++---------
 gcc/cp/typeck.cc                              | 34 +++------------
 .../g++.dg/cpp0x/Wredundant-move10.C          |  2 +-
 gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C |  6 +--
 gcc/testsuite/g++.dg/cpp0x/move-return2.C     |  2 +-
 gcc/testsuite/g++.dg/cpp0x/move-return4.C     |  2 +-
 gcc/testsuite/g++.dg/cpp0x/move-return5.C     | 20 +++++++++
 gcc/testsuite/g++.dg/cpp0x/ref-qual20.C       |  2 +-
 10 files changed, 41 insertions(+), 97 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/move-return5.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 3506b0fcfbb..e100913ea29 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -1272,9 +1272,6 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
 	    }
 	}
       conv = build_conv (ck_rvalue, from, conv);
-      if (flags & LOOKUP_PREFER_RVALUE)
-	/* Tell convert_like to set LOOKUP_PREFER_RVALUE.  */
-	conv->rvaluedness_matches_p = true;
       /* If we're performing copy-initialization, remember to skip
 	 explicit constructors.  */
       if (flags & LOOKUP_ONLYCONVERTING)
@@ -1572,9 +1569,6 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
 	 type.  A temporary object is created to hold the result of
 	 the conversion unless we're binding directly to a reference.  */
       conv->need_temporary_p = !(flags & LOOKUP_NO_TEMP_BIND);
-      if (flags & LOOKUP_PREFER_RVALUE)
-	/* Tell convert_like to set LOOKUP_PREFER_RVALUE.  */
-	conv->rvaluedness_matches_p = true;
       /* If we're performing copy-initialization, remember to skip
 	 explicit constructors.  */
       if (flags & LOOKUP_ONLYCONVERTING)
@@ -1883,7 +1877,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
 	  /* Unless it's really a C++20 lvalue being treated as an xvalue.
 	     But in C++23, such an expression is just an xvalue, not a special
 	     lvalue, so the binding is once again ill-formed.  */
-	  && !(cxx_dialect == cxx20
+	  && !(cxx_dialect <= cxx20
 	       && (gl_kind & clk_implicit_rval))
 	  && (!CP_TYPE_CONST_NON_VOLATILE_P (to)
 	      || (flags & LOOKUP_NO_RVAL_BIND))
@@ -2044,9 +2038,8 @@ implicit_conversion_1 (tree to, tree from, tree expr, bool c_cast_p,
   /* Other flags only apply to the primary function in overload
      resolution, or after we've chosen one.  */
   flags &= (LOOKUP_ONLYCONVERTING|LOOKUP_NO_CONVERSION|LOOKUP_COPY_PARM
-	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_PREFER_RVALUE
-	    |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL
-	    |LOOKUP_SHORTCUT_BAD_CONVS);
+	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_NO_NARROWING
+	    |LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL|LOOKUP_SHORTCUT_BAD_CONVS);
 
   /* FIXME: actually we don't want warnings either, but we can't just
      have 'complain &= ~(tf_warning|tf_error)' because it would cause
@@ -4451,14 +4444,6 @@ build_user_type_conversion_1 (tree totype, tree expr, int flags,
   if (cand->viable == -1)
     conv->bad_p = true;
 
-  /* We're performing the maybe-rvalue overload resolution and
-     a conversion function is in play.  Reject converting the return
-     value of the conversion function to a base class.  */
-  if ((flags & LOOKUP_PREFER_RVALUE) && !DECL_CONSTRUCTOR_P (cand->fn))
-    for (conversion *t = cand->second_conv; t; t = next_conversion (t))
-      if (t->kind == ck_base)
-	return NULL;
-
   /* Remember that this was a list-initialization.  */
   if (flags & LOOKUP_NO_NARROWING)
     conv->check_narrowing = true;
@@ -8281,9 +8266,6 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
 	 explicit constructors.  */
       if (convs->copy_init_p)
 	flags |= LOOKUP_ONLYCONVERTING;
-      if (convs->rvaluedness_matches_p)
-	/* standard_conversion got LOOKUP_PREFER_RVALUE.  */
-	flags |= LOOKUP_PREFER_RVALUE;
       expr = build_temp (expr, totype, flags, &diag_kind, complain);
       if (diag_kind && complain)
 	{
@@ -9529,23 +9511,6 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 	  ++arg_index;
 	  parm = TREE_CHAIN (parm);
 	}
-
-      if (cxx_dialect < cxx20
-	  && (cand->flags & LOOKUP_PREFER_RVALUE))
-	{
-	  /* The implicit move specified in 15.8.3/3 fails "...if the type of
-	     the first parameter of the selected constructor is not an rvalue
-	     reference to the object's type (possibly cv-qualified)...." */
-	  gcc_assert (!(complain & tf_error));
-	  tree ptype = convs[0]->type;
-	  /* Allow calling a by-value converting constructor even though it
-	     isn't permitted by the above, because we've allowed it since GCC 5
-	     (PR58051) and it's allowed in C++20.  But don't call a copy
-	     constructor.  */
-	  if ((TYPE_REF_P (ptype) && !TYPE_REF_IS_RVALUE (ptype))
-	      || CONVERSION_RANK (convs[0]) > cr_exact)
-	    return error_mark_node;
-	}
     }
   /* Bypass access control for 'this' parameter.  */
   else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index d0f1b18b015..7acb5373513 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5853,12 +5853,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG };
 #define LOOKUP_DESTRUCTOR (1 << 5)
 /* Do not permit references to bind to temporaries.  */
 #define LOOKUP_NO_TEMP_BIND (1 << 6)
-/* We're trying to treat an lvalue as an rvalue.  */
-/* FIXME remove when we extend the P1825 semantics to all standard modes, the
-   C++20 approach uses IMPLICIT_RVALUE_P instead.  */
-#define LOOKUP_PREFER_RVALUE (LOOKUP_NO_TEMP_BIND << 1)
 /* We're inside an init-list, so narrowing conversions are ill-formed.  */
-#define LOOKUP_NO_NARROWING (LOOKUP_PREFER_RVALUE << 1)
+#define LOOKUP_NO_NARROWING (LOOKUP_NO_TEMP_BIND << 1)
 /* We're looking up a constructor for list-initialization.  */
 #define LOOKUP_LIST_INIT_CTOR (LOOKUP_NO_NARROWING << 1)
 /* This is the first parameter of a copy constructor.  */
diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
index 048612de400..1d84b451c9f 100644
--- a/gcc/cp/except.cc
+++ b/gcc/cp/except.cc
@@ -715,25 +715,10 @@ build_throw (location_t loc, tree exp)
 	     treated as an rvalue for the purposes of overload resolution
 	     to favor move constructors over copy constructors.  */
 	  if (tree moved = treat_lvalue_as_rvalue_p (exp, /*return*/false))
-	    {
-	      if (cxx_dialect < cxx20)
-		{
-		  releasing_vec exp_vec (make_tree_vector_single (moved));
-		  moved = (build_special_member_call
-			   (object, complete_ctor_identifier, &exp_vec,
-			    TREE_TYPE (object), flags|LOOKUP_PREFER_RVALUE,
-			    tf_none));
-		  if (moved != error_mark_node)
-		    {
-		      exp = moved;
-		      converted = true;
-		    }
-		}
-	      else
-		/* In C++20 we just treat the return value as an rvalue that
-		   can bind to lvalue refs.  */
-		exp = moved;
-	    }
+	    /* In C++20 we just treat the return value as an rvalue that
+	       can bind to lvalue refs.  In C++23, such an expression is just
+	       an xvalue.  */
+	    exp = moved;
 
 	  /* Call the copy constructor.  */
 	  if (!converted)
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 5f16c4d2426..c525e880383 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10692,21 +10692,12 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
 	  tree t = convert_for_initialization (NULL_TREE, type,
 					       moved,
 					       (LOOKUP_NORMAL
-						| LOOKUP_ONLYCONVERTING
-						| LOOKUP_PREFER_RVALUE),
+						| LOOKUP_ONLYCONVERTING),
 					       ICR_RETURN, NULL_TREE, 0,
 					       tf_none);
 	  /* If this worked, implicit rvalue would work, so the call to
 	     std::move is redundant.  */
-	  if (t != error_mark_node
-	      /* Trying to move something const will never succeed unless
-		 there's T(const T&&), which it almost never is, and if
-		 so, T wouldn't be error_mark_node now: the above convert_
-		 call with LOOKUP_PREFER_RVALUE returns an error if a const T&
-		 overload is selected.  */
-	      || (CP_TYPE_CONST_P (TREE_TYPE (arg))
-		  && same_type_ignoring_top_level_qualifiers_p
-		  (TREE_TYPE (arg), type)))
+	  if (t != error_mark_node)
 	    {
 	      auto_diagnostic_group d;
 	      if (warning_at (loc, OPT_Wredundant_move,
@@ -11049,23 +11040,10 @@ check_return_expr (tree retval, bool *no_warning)
 	     ? CLASS_TYPE_P (functype)
 	     : !SCALAR_TYPE_P (functype) || !SCALAR_TYPE_P (TREE_TYPE (retval)))
 	    && (moved = treat_lvalue_as_rvalue_p (retval, /*return*/true)))
-	{
-	  if (cxx_dialect < cxx20)
-	    {
-	      moved = convert_for_initialization
-		(NULL_TREE, functype, moved, flags|LOOKUP_PREFER_RVALUE,
-		 ICR_RETURN, NULL_TREE, 0, tf_none);
-	      if (moved != error_mark_node)
-		{
-		  retval = moved;
-		  converted = true;
-		}
-	    }
-	  else
-	    /* In C++20 we just treat the return value as an rvalue that
-	       can bind to lvalue refs.  */
-	    retval = moved;
-	}
+	  /* In C++20 and earlier we just treat the return value as an rvalue
+	     that can bind to lvalue refs.  In C++23, such an expression is just
+	     an xvalue (see reference_binding).  */
+	  retval = moved;
 
       /* The call in a (lambda) thunk needs no conversions.  */
       if (TREE_CODE (retval) == CALL_EXPR
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
index a215a4774d6..17dd807aea8 100644
--- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
+++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
@@ -57,5 +57,5 @@ struct S2: S1 {};
 
 S1 f3(const S2 s)
 {
-  return std::move(s); // { dg-warning "redundant move" "" { target c++20 } }
+  return std::move(s); // { dg-warning "redundant move" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
index 3fec525879d..6547777c007 100644
--- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
+++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
@@ -28,7 +28,7 @@ struct S2 : S1 {};
 S1
 f (S2 s)
 {
-  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
+  return std::move(s); // { dg-warning "redundant move in return statement" }
 }
 
 struct R1 {
@@ -40,7 +40,7 @@ struct R2 : R1 {};
 R1
 f2 (const R2 s)
 {
-  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
+  return std::move(s); // { dg-warning "redundant move in return statement" }
 }
 
 struct T1 {
@@ -55,5 +55,5 @@ f3 (const T2 s)
 {
   // Without std::move: const T1 &
   // With std::move: const T1 &&
-  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
+  return std::move(s); // { dg-warning "redundant move in return statement" }
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return2.C b/gcc/testsuite/g++.dg/cpp0x/move-return2.C
index 999f2c95c49..8e750efb870 100644
--- a/gcc/testsuite/g++.dg/cpp0x/move-return2.C
+++ b/gcc/testsuite/g++.dg/cpp0x/move-return2.C
@@ -7,5 +7,5 @@ struct S2 : S1 {};
 S1
 f (S2 s)
 {
-  return s; // { dg-error "use of deleted function" "" { target c++17_down } }
+  return s;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return4.C b/gcc/testsuite/g++.dg/cpp0x/move-return4.C
index 3fc58089319..0f0ca1fc253 100644
--- a/gcc/testsuite/g++.dg/cpp0x/move-return4.C
+++ b/gcc/testsuite/g++.dg/cpp0x/move-return4.C
@@ -13,5 +13,5 @@ struct A : Base
 A<int> foo()
 {
     A<double> v;
-    return v; // { dg-error "cannot bind rvalue reference" "" { target c++17_down } }
+    return v;
 }
diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return5.C b/gcc/testsuite/g++.dg/cpp0x/move-return5.C
new file mode 100644
index 00000000000..695000b2687
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/move-return5.C
@@ -0,0 +1,20 @@
+// { dg-do compile { target c++11 } }
+// This used to compile in C++11...17 because we performed two
+// separate overload resolutions: one treating the operand as
+// an rvalue, and then (if that resolution fails) another one
+// treating the operand as an lvalue.
+
+struct W {
+  W();
+};
+
+struct F {
+  F(W&);
+  F(W&&) = delete;
+};
+
+F fn ()
+{
+  W w;
+  return w; // { dg-error "use of deleted function" }
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C b/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
index cfbef300226..314f19bb864 100644
--- a/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
+++ b/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
@@ -52,7 +52,7 @@ f5 ()
 int
 main ()
 {
-  int return_lval = __cplusplus > 201703L ? -1 : 2;
+  int return_lval = -1;
   Y y1 = f (A());
   if (y1.y != return_lval)
     __builtin_abort ();

base-commit: 9f65eecdbef6027722e93aadf3fa6b3cc342eb4f
-- 
2.37.3


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] c++: Remove maybe-rvalue OR in implicit move
  2022-09-28 21:26 [PATCH] c++: Remove maybe-rvalue OR in implicit move Marek Polacek
@ 2022-10-10 19:19 ` Marek Polacek
  2022-10-10 20:15 ` Jason Merrill
  1 sibling, 0 replies; 3+ messages in thread
From: Marek Polacek @ 2022-10-10 19:19 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill

Ping.

On Wed, Sep 28, 2022 at 05:26:34PM -0400, Marek Polacek via Gcc-patches wrote:
> This patch removes the two-stage overload resolution when performing
> implicit move, whereby the compiler does two separate overload resolutions:
> one treating the operand as an rvalue, and then (if that resolution fails)
> another one treating the operand as an lvalue.  In the standard this was
> introduced via CWG 1579 and implemented in gcc in r251035.  In r11-2412,
> we disabled the fallback OR in C++20 (but not in C++17).  Then C++23 P2266
> removed the fallback overload resolution, and changed the implicit move rules
> once again.  So we wound up with three different behaviors.
> 
> The two overload resolutions approach was complicated and quirky, so
> users should transition to the newer model.  Removing the maybe-rvalue
> OR also allows us to simplify our code, for instance, now we can get
> rid of LOOKUP_PREFER_RVALUE altogether.
> 
> This change means that code that previously didn't compile in C++17 will
> now compile, for example:
> 
>   struct S1 { S1(S1 &&); };
>   struct S2 : S1 {};
> 
>   S1
>   f (S2 s)
>   {
>     return s; // OK, derived-to-base, use S1::S1(S1&&)
>   }
> 
> And conversely, code that used to work in C++17 may not compile anymore:
> 
>   struct W {
>     W();
>   };
> 
>   struct F {
>     F(W&);
>     F(W&&) = delete;
>   };
> 
>   F fn ()
>   {
>     W w;
>     return w; // use w as rvalue -> use of deleted function F::F(W&&)
>   }
> 
> I plan to add a note to porting_to.html.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (standard_conversion): Remove LOOKUP_PREFER_RVALUE code.
> 	(reference_binding): Honor clk_implicit_rval even pre-C++20.
> 	(implicit_conversion_1): Remove LOOKUP_PREFER_RVALUE code.
> 	(build_user_type_conversion_1): Likewise.
> 	(convert_like_internal): Likewise.
> 	(build_over_call): Likewise.
> 	* cp-tree.h (LOOKUP_PREFER_RVALUE): Remove.
> 	(LOOKUP_NO_NARROWING): Adjust definition.
> 	* except.cc (build_throw): Don't perform two overload resolutions.
> 	* typeck.cc (maybe_warn_pessimizing_move): Don't use
> 	LOOKUP_PREFER_RVALUE.
> 	(check_return_expr): Don't perform two overload resolutions.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/Wredundant-move10.C: Adjust dg-warning.
> 	* g++.dg/cpp0x/Wredundant-move7.C: Likewise.
> 	* g++.dg/cpp0x/move-return2.C: Remove dg-error.
> 	* g++.dg/cpp0x/move-return4.C: Likewise.
> 	* g++.dg/cpp0x/ref-qual20.C: Adjust expected return value.
> 	* g++.dg/cpp0x/move-return5.C: New test.
> ---
>  gcc/cp/call.cc                                | 41 ++-----------------
>  gcc/cp/cp-tree.h                              |  6 +--
>  gcc/cp/except.cc                              | 23 ++---------
>  gcc/cp/typeck.cc                              | 34 +++------------
>  .../g++.dg/cpp0x/Wredundant-move10.C          |  2 +-
>  gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C |  6 +--
>  gcc/testsuite/g++.dg/cpp0x/move-return2.C     |  2 +-
>  gcc/testsuite/g++.dg/cpp0x/move-return4.C     |  2 +-
>  gcc/testsuite/g++.dg/cpp0x/move-return5.C     | 20 +++++++++
>  gcc/testsuite/g++.dg/cpp0x/ref-qual20.C       |  2 +-
>  10 files changed, 41 insertions(+), 97 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/move-return5.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 3506b0fcfbb..e100913ea29 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -1272,9 +1272,6 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
>  	    }
>  	}
>        conv = build_conv (ck_rvalue, from, conv);
> -      if (flags & LOOKUP_PREFER_RVALUE)
> -	/* Tell convert_like to set LOOKUP_PREFER_RVALUE.  */
> -	conv->rvaluedness_matches_p = true;
>        /* If we're performing copy-initialization, remember to skip
>  	 explicit constructors.  */
>        if (flags & LOOKUP_ONLYCONVERTING)
> @@ -1572,9 +1569,6 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
>  	 type.  A temporary object is created to hold the result of
>  	 the conversion unless we're binding directly to a reference.  */
>        conv->need_temporary_p = !(flags & LOOKUP_NO_TEMP_BIND);
> -      if (flags & LOOKUP_PREFER_RVALUE)
> -	/* Tell convert_like to set LOOKUP_PREFER_RVALUE.  */
> -	conv->rvaluedness_matches_p = true;
>        /* If we're performing copy-initialization, remember to skip
>  	 explicit constructors.  */
>        if (flags & LOOKUP_ONLYCONVERTING)
> @@ -1883,7 +1877,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>  	  /* Unless it's really a C++20 lvalue being treated as an xvalue.
>  	     But in C++23, such an expression is just an xvalue, not a special
>  	     lvalue, so the binding is once again ill-formed.  */
> -	  && !(cxx_dialect == cxx20
> +	  && !(cxx_dialect <= cxx20
>  	       && (gl_kind & clk_implicit_rval))
>  	  && (!CP_TYPE_CONST_NON_VOLATILE_P (to)
>  	      || (flags & LOOKUP_NO_RVAL_BIND))
> @@ -2044,9 +2038,8 @@ implicit_conversion_1 (tree to, tree from, tree expr, bool c_cast_p,
>    /* Other flags only apply to the primary function in overload
>       resolution, or after we've chosen one.  */
>    flags &= (LOOKUP_ONLYCONVERTING|LOOKUP_NO_CONVERSION|LOOKUP_COPY_PARM
> -	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_PREFER_RVALUE
> -	    |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL
> -	    |LOOKUP_SHORTCUT_BAD_CONVS);
> +	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_NO_NARROWING
> +	    |LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL|LOOKUP_SHORTCUT_BAD_CONVS);
>  
>    /* FIXME: actually we don't want warnings either, but we can't just
>       have 'complain &= ~(tf_warning|tf_error)' because it would cause
> @@ -4451,14 +4444,6 @@ build_user_type_conversion_1 (tree totype, tree expr, int flags,
>    if (cand->viable == -1)
>      conv->bad_p = true;
>  
> -  /* We're performing the maybe-rvalue overload resolution and
> -     a conversion function is in play.  Reject converting the return
> -     value of the conversion function to a base class.  */
> -  if ((flags & LOOKUP_PREFER_RVALUE) && !DECL_CONSTRUCTOR_P (cand->fn))
> -    for (conversion *t = cand->second_conv; t; t = next_conversion (t))
> -      if (t->kind == ck_base)
> -	return NULL;
> -
>    /* Remember that this was a list-initialization.  */
>    if (flags & LOOKUP_NO_NARROWING)
>      conv->check_narrowing = true;
> @@ -8281,9 +8266,6 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>  	 explicit constructors.  */
>        if (convs->copy_init_p)
>  	flags |= LOOKUP_ONLYCONVERTING;
> -      if (convs->rvaluedness_matches_p)
> -	/* standard_conversion got LOOKUP_PREFER_RVALUE.  */
> -	flags |= LOOKUP_PREFER_RVALUE;
>        expr = build_temp (expr, totype, flags, &diag_kind, complain);
>        if (diag_kind && complain)
>  	{
> @@ -9529,23 +9511,6 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
>  	  ++arg_index;
>  	  parm = TREE_CHAIN (parm);
>  	}
> -
> -      if (cxx_dialect < cxx20
> -	  && (cand->flags & LOOKUP_PREFER_RVALUE))
> -	{
> -	  /* The implicit move specified in 15.8.3/3 fails "...if the type of
> -	     the first parameter of the selected constructor is not an rvalue
> -	     reference to the object's type (possibly cv-qualified)...." */
> -	  gcc_assert (!(complain & tf_error));
> -	  tree ptype = convs[0]->type;
> -	  /* Allow calling a by-value converting constructor even though it
> -	     isn't permitted by the above, because we've allowed it since GCC 5
> -	     (PR58051) and it's allowed in C++20.  But don't call a copy
> -	     constructor.  */
> -	  if ((TYPE_REF_P (ptype) && !TYPE_REF_IS_RVALUE (ptype))
> -	      || CONVERSION_RANK (convs[0]) > cr_exact)
> -	    return error_mark_node;
> -	}
>      }
>    /* Bypass access control for 'this' parameter.  */
>    else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index d0f1b18b015..7acb5373513 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -5853,12 +5853,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG };
>  #define LOOKUP_DESTRUCTOR (1 << 5)
>  /* Do not permit references to bind to temporaries.  */
>  #define LOOKUP_NO_TEMP_BIND (1 << 6)
> -/* We're trying to treat an lvalue as an rvalue.  */
> -/* FIXME remove when we extend the P1825 semantics to all standard modes, the
> -   C++20 approach uses IMPLICIT_RVALUE_P instead.  */
> -#define LOOKUP_PREFER_RVALUE (LOOKUP_NO_TEMP_BIND << 1)
>  /* We're inside an init-list, so narrowing conversions are ill-formed.  */
> -#define LOOKUP_NO_NARROWING (LOOKUP_PREFER_RVALUE << 1)
> +#define LOOKUP_NO_NARROWING (LOOKUP_NO_TEMP_BIND << 1)
>  /* We're looking up a constructor for list-initialization.  */
>  #define LOOKUP_LIST_INIT_CTOR (LOOKUP_NO_NARROWING << 1)
>  /* This is the first parameter of a copy constructor.  */
> diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> index 048612de400..1d84b451c9f 100644
> --- a/gcc/cp/except.cc
> +++ b/gcc/cp/except.cc
> @@ -715,25 +715,10 @@ build_throw (location_t loc, tree exp)
>  	     treated as an rvalue for the purposes of overload resolution
>  	     to favor move constructors over copy constructors.  */
>  	  if (tree moved = treat_lvalue_as_rvalue_p (exp, /*return*/false))
> -	    {
> -	      if (cxx_dialect < cxx20)
> -		{
> -		  releasing_vec exp_vec (make_tree_vector_single (moved));
> -		  moved = (build_special_member_call
> -			   (object, complete_ctor_identifier, &exp_vec,
> -			    TREE_TYPE (object), flags|LOOKUP_PREFER_RVALUE,
> -			    tf_none));
> -		  if (moved != error_mark_node)
> -		    {
> -		      exp = moved;
> -		      converted = true;
> -		    }
> -		}
> -	      else
> -		/* In C++20 we just treat the return value as an rvalue that
> -		   can bind to lvalue refs.  */
> -		exp = moved;
> -	    }
> +	    /* In C++20 we just treat the return value as an rvalue that
> +	       can bind to lvalue refs.  In C++23, such an expression is just
> +	       an xvalue.  */
> +	    exp = moved;
>  
>  	  /* Call the copy constructor.  */
>  	  if (!converted)
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 5f16c4d2426..c525e880383 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -10692,21 +10692,12 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
>  	  tree t = convert_for_initialization (NULL_TREE, type,
>  					       moved,
>  					       (LOOKUP_NORMAL
> -						| LOOKUP_ONLYCONVERTING
> -						| LOOKUP_PREFER_RVALUE),
> +						| LOOKUP_ONLYCONVERTING),
>  					       ICR_RETURN, NULL_TREE, 0,
>  					       tf_none);
>  	  /* If this worked, implicit rvalue would work, so the call to
>  	     std::move is redundant.  */
> -	  if (t != error_mark_node
> -	      /* Trying to move something const will never succeed unless
> -		 there's T(const T&&), which it almost never is, and if
> -		 so, T wouldn't be error_mark_node now: the above convert_
> -		 call with LOOKUP_PREFER_RVALUE returns an error if a const T&
> -		 overload is selected.  */
> -	      || (CP_TYPE_CONST_P (TREE_TYPE (arg))
> -		  && same_type_ignoring_top_level_qualifiers_p
> -		  (TREE_TYPE (arg), type)))
> +	  if (t != error_mark_node)
>  	    {
>  	      auto_diagnostic_group d;
>  	      if (warning_at (loc, OPT_Wredundant_move,
> @@ -11049,23 +11040,10 @@ check_return_expr (tree retval, bool *no_warning)
>  	     ? CLASS_TYPE_P (functype)
>  	     : !SCALAR_TYPE_P (functype) || !SCALAR_TYPE_P (TREE_TYPE (retval)))
>  	    && (moved = treat_lvalue_as_rvalue_p (retval, /*return*/true)))
> -	{
> -	  if (cxx_dialect < cxx20)
> -	    {
> -	      moved = convert_for_initialization
> -		(NULL_TREE, functype, moved, flags|LOOKUP_PREFER_RVALUE,
> -		 ICR_RETURN, NULL_TREE, 0, tf_none);
> -	      if (moved != error_mark_node)
> -		{
> -		  retval = moved;
> -		  converted = true;
> -		}
> -	    }
> -	  else
> -	    /* In C++20 we just treat the return value as an rvalue that
> -	       can bind to lvalue refs.  */
> -	    retval = moved;
> -	}
> +	  /* In C++20 and earlier we just treat the return value as an rvalue
> +	     that can bind to lvalue refs.  In C++23, such an expression is just
> +	     an xvalue (see reference_binding).  */
> +	  retval = moved;
>  
>        /* The call in a (lambda) thunk needs no conversions.  */
>        if (TREE_CODE (retval) == CALL_EXPR
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
> index a215a4774d6..17dd807aea8 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
> @@ -57,5 +57,5 @@ struct S2: S1 {};
>  
>  S1 f3(const S2 s)
>  {
> -  return std::move(s); // { dg-warning "redundant move" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move" }
>  }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
> index 3fec525879d..6547777c007 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
> @@ -28,7 +28,7 @@ struct S2 : S1 {};
>  S1
>  f (S2 s)
>  {
> -  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move in return statement" }
>  }
>  
>  struct R1 {
> @@ -40,7 +40,7 @@ struct R2 : R1 {};
>  R1
>  f2 (const R2 s)
>  {
> -  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move in return statement" }
>  }
>  
>  struct T1 {
> @@ -55,5 +55,5 @@ f3 (const T2 s)
>  {
>    // Without std::move: const T1 &
>    // With std::move: const T1 &&
> -  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move in return statement" }
>  }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return2.C b/gcc/testsuite/g++.dg/cpp0x/move-return2.C
> index 999f2c95c49..8e750efb870 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/move-return2.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/move-return2.C
> @@ -7,5 +7,5 @@ struct S2 : S1 {};
>  S1
>  f (S2 s)
>  {
> -  return s; // { dg-error "use of deleted function" "" { target c++17_down } }
> +  return s;
>  }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return4.C b/gcc/testsuite/g++.dg/cpp0x/move-return4.C
> index 3fc58089319..0f0ca1fc253 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/move-return4.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/move-return4.C
> @@ -13,5 +13,5 @@ struct A : Base
>  A<int> foo()
>  {
>      A<double> v;
> -    return v; // { dg-error "cannot bind rvalue reference" "" { target c++17_down } }
> +    return v;
>  }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return5.C b/gcc/testsuite/g++.dg/cpp0x/move-return5.C
> new file mode 100644
> index 00000000000..695000b2687
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/move-return5.C
> @@ -0,0 +1,20 @@
> +// { dg-do compile { target c++11 } }
> +// This used to compile in C++11...17 because we performed two
> +// separate overload resolutions: one treating the operand as
> +// an rvalue, and then (if that resolution fails) another one
> +// treating the operand as an lvalue.
> +
> +struct W {
> +  W();
> +};
> +
> +struct F {
> +  F(W&);
> +  F(W&&) = delete;
> +};
> +
> +F fn ()
> +{
> +  W w;
> +  return w; // { dg-error "use of deleted function" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C b/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
> index cfbef300226..314f19bb864 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
> @@ -52,7 +52,7 @@ f5 ()
>  int
>  main ()
>  {
> -  int return_lval = __cplusplus > 201703L ? -1 : 2;
> +  int return_lval = -1;
>    Y y1 = f (A());
>    if (y1.y != return_lval)
>      __builtin_abort ();
> 
> base-commit: 9f65eecdbef6027722e93aadf3fa6b3cc342eb4f
> -- 
> 2.37.3
> 

Marek


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] c++: Remove maybe-rvalue OR in implicit move
  2022-09-28 21:26 [PATCH] c++: Remove maybe-rvalue OR in implicit move Marek Polacek
  2022-10-10 19:19 ` Marek Polacek
@ 2022-10-10 20:15 ` Jason Merrill
  1 sibling, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2022-10-10 20:15 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches

On 9/28/22 17:26, Marek Polacek wrote:
> This patch removes the two-stage overload resolution when performing
> implicit move, whereby the compiler does two separate overload resolutions:
> one treating the operand as an rvalue, and then (if that resolution fails)
> another one treating the operand as an lvalue.  In the standard this was
> introduced via CWG 1579 and implemented in gcc in r251035.  In r11-2412,
> we disabled the fallback OR in C++20 (but not in C++17).  Then C++23 P2266
> removed the fallback overload resolution, and changed the implicit move rules
> once again.  So we wound up with three different behaviors.
> 
> The two overload resolutions approach was complicated and quirky, so
> users should transition to the newer model.  Removing the maybe-rvalue
> OR also allows us to simplify our code, for instance, now we can get
> rid of LOOKUP_PREFER_RVALUE altogether.
> 
> This change means that code that previously didn't compile in C++17 will
> now compile, for example:
> 
>    struct S1 { S1(S1 &&); };
>    struct S2 : S1 {};
> 
>    S1
>    f (S2 s)
>    {
>      return s; // OK, derived-to-base, use S1::S1(S1&&)
>    }
> 
> And conversely, code that used to work in C++17 may not compile anymore:
> 
>    struct W {
>      W();
>    };
> 
>    struct F {
>      F(W&);
>      F(W&&) = delete;
>    };
> 
>    F fn ()
>    {
>      W w;
>      return w; // use w as rvalue -> use of deleted function F::F(W&&)
>    }
> 
> I plan to add a note to porting_to.html.

It surprises me that this was a change: I guess it depends on how you 
interpret the old wording's "if the overload resolution fails".  I 
vaguely remember there being a bug report about this, before the new 
approach made it moot.

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK with the comment tweaks below:

> gcc/cp/ChangeLog:
> 
> 	* call.cc (standard_conversion): Remove LOOKUP_PREFER_RVALUE code.
> 	(reference_binding): Honor clk_implicit_rval even pre-C++20.
> 	(implicit_conversion_1): Remove LOOKUP_PREFER_RVALUE code.
> 	(build_user_type_conversion_1): Likewise.
> 	(convert_like_internal): Likewise.
> 	(build_over_call): Likewise.
> 	* cp-tree.h (LOOKUP_PREFER_RVALUE): Remove.
> 	(LOOKUP_NO_NARROWING): Adjust definition.
> 	* except.cc (build_throw): Don't perform two overload resolutions.
> 	* typeck.cc (maybe_warn_pessimizing_move): Don't use
> 	LOOKUP_PREFER_RVALUE.
> 	(check_return_expr): Don't perform two overload resolutions.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/Wredundant-move10.C: Adjust dg-warning.
> 	* g++.dg/cpp0x/Wredundant-move7.C: Likewise.
> 	* g++.dg/cpp0x/move-return2.C: Remove dg-error.
> 	* g++.dg/cpp0x/move-return4.C: Likewise.
> 	* g++.dg/cpp0x/ref-qual20.C: Adjust expected return value.
> 	* g++.dg/cpp0x/move-return5.C: New test.
> ---
>   gcc/cp/call.cc                                | 41 ++-----------------
>   gcc/cp/cp-tree.h                              |  6 +--
>   gcc/cp/except.cc                              | 23 ++---------
>   gcc/cp/typeck.cc                              | 34 +++------------
>   .../g++.dg/cpp0x/Wredundant-move10.C          |  2 +-
>   gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C |  6 +--
>   gcc/testsuite/g++.dg/cpp0x/move-return2.C     |  2 +-
>   gcc/testsuite/g++.dg/cpp0x/move-return4.C     |  2 +-
>   gcc/testsuite/g++.dg/cpp0x/move-return5.C     | 20 +++++++++
>   gcc/testsuite/g++.dg/cpp0x/ref-qual20.C       |  2 +-
>   10 files changed, 41 insertions(+), 97 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/move-return5.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index 3506b0fcfbb..e100913ea29 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -1272,9 +1272,6 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
>   	    }
>   	}
>         conv = build_conv (ck_rvalue, from, conv);
> -      if (flags & LOOKUP_PREFER_RVALUE)
> -	/* Tell convert_like to set LOOKUP_PREFER_RVALUE.  */
> -	conv->rvaluedness_matches_p = true;
>         /* If we're performing copy-initialization, remember to skip
>   	 explicit constructors.  */
>         if (flags & LOOKUP_ONLYCONVERTING)
> @@ -1572,9 +1569,6 @@ standard_conversion (tree to, tree from, tree expr, bool c_cast_p,
>   	 type.  A temporary object is created to hold the result of
>   	 the conversion unless we're binding directly to a reference.  */
>         conv->need_temporary_p = !(flags & LOOKUP_NO_TEMP_BIND);
> -      if (flags & LOOKUP_PREFER_RVALUE)
> -	/* Tell convert_like to set LOOKUP_PREFER_RVALUE.  */
> -	conv->rvaluedness_matches_p = true;
>         /* If we're performing copy-initialization, remember to skip
>   	 explicit constructors.  */
>         if (flags & LOOKUP_ONLYCONVERTING)
> @@ -1883,7 +1877,7 @@ reference_binding (tree rto, tree rfrom, tree expr, bool c_cast_p, int flags,
>   	  /* Unless it's really a C++20 lvalue being treated as an xvalue.
>   	     But in C++23, such an expression is just an xvalue, not a special
>   	     lvalue, so the binding is once again ill-formed.  */
> -	  && !(cxx_dialect == cxx20
> +	  && !(cxx_dialect <= cxx20
>   	       && (gl_kind & clk_implicit_rval))
>   	  && (!CP_TYPE_CONST_NON_VOLATILE_P (to)
>   	      || (flags & LOOKUP_NO_RVAL_BIND))
> @@ -2044,9 +2038,8 @@ implicit_conversion_1 (tree to, tree from, tree expr, bool c_cast_p,
>     /* Other flags only apply to the primary function in overload
>        resolution, or after we've chosen one.  */
>     flags &= (LOOKUP_ONLYCONVERTING|LOOKUP_NO_CONVERSION|LOOKUP_COPY_PARM
> -	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_PREFER_RVALUE
> -	    |LOOKUP_NO_NARROWING|LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL
> -	    |LOOKUP_SHORTCUT_BAD_CONVS);
> +	    |LOOKUP_NO_TEMP_BIND|LOOKUP_NO_RVAL_BIND|LOOKUP_NO_NARROWING
> +	    |LOOKUP_PROTECT|LOOKUP_NO_NON_INTEGRAL|LOOKUP_SHORTCUT_BAD_CONVS);
>   
>     /* FIXME: actually we don't want warnings either, but we can't just
>        have 'complain &= ~(tf_warning|tf_error)' because it would cause
> @@ -4451,14 +4444,6 @@ build_user_type_conversion_1 (tree totype, tree expr, int flags,
>     if (cand->viable == -1)
>       conv->bad_p = true;
>   
> -  /* We're performing the maybe-rvalue overload resolution and
> -     a conversion function is in play.  Reject converting the return
> -     value of the conversion function to a base class.  */
> -  if ((flags & LOOKUP_PREFER_RVALUE) && !DECL_CONSTRUCTOR_P (cand->fn))
> -    for (conversion *t = cand->second_conv; t; t = next_conversion (t))
> -      if (t->kind == ck_base)
> -	return NULL;
> -
>     /* Remember that this was a list-initialization.  */
>     if (flags & LOOKUP_NO_NARROWING)
>       conv->check_narrowing = true;
> @@ -8281,9 +8266,6 @@ convert_like_internal (conversion *convs, tree expr, tree fn, int argnum,
>   	 explicit constructors.  */
>         if (convs->copy_init_p)
>   	flags |= LOOKUP_ONLYCONVERTING;
> -      if (convs->rvaluedness_matches_p)
> -	/* standard_conversion got LOOKUP_PREFER_RVALUE.  */
> -	flags |= LOOKUP_PREFER_RVALUE;
>         expr = build_temp (expr, totype, flags, &diag_kind, complain);
>         if (diag_kind && complain)
>   	{
> @@ -9529,23 +9511,6 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
>   	  ++arg_index;
>   	  parm = TREE_CHAIN (parm);
>   	}
> -
> -      if (cxx_dialect < cxx20
> -	  && (cand->flags & LOOKUP_PREFER_RVALUE))
> -	{
> -	  /* The implicit move specified in 15.8.3/3 fails "...if the type of
> -	     the first parameter of the selected constructor is not an rvalue
> -	     reference to the object's type (possibly cv-qualified)...." */
> -	  gcc_assert (!(complain & tf_error));
> -	  tree ptype = convs[0]->type;
> -	  /* Allow calling a by-value converting constructor even though it
> -	     isn't permitted by the above, because we've allowed it since GCC 5
> -	     (PR58051) and it's allowed in C++20.  But don't call a copy
> -	     constructor.  */
> -	  if ((TYPE_REF_P (ptype) && !TYPE_REF_IS_RVALUE (ptype))
> -	      || CONVERSION_RANK (convs[0]) > cr_exact)
> -	    return error_mark_node;
> -	}
>       }
>     /* Bypass access control for 'this' parameter.  */
>     else if (TREE_CODE (TREE_TYPE (fn)) == METHOD_TYPE)
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index d0f1b18b015..7acb5373513 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -5853,12 +5853,8 @@ enum overload_flags { NO_SPECIAL = 0, DTOR_FLAG, TYPENAME_FLAG };
>   #define LOOKUP_DESTRUCTOR (1 << 5)
>   /* Do not permit references to bind to temporaries.  */
>   #define LOOKUP_NO_TEMP_BIND (1 << 6)
> -/* We're trying to treat an lvalue as an rvalue.  */
> -/* FIXME remove when we extend the P1825 semantics to all standard modes, the
> -   C++20 approach uses IMPLICIT_RVALUE_P instead.  */
> -#define LOOKUP_PREFER_RVALUE (LOOKUP_NO_TEMP_BIND << 1)
>   /* We're inside an init-list, so narrowing conversions are ill-formed.  */
> -#define LOOKUP_NO_NARROWING (LOOKUP_PREFER_RVALUE << 1)
> +#define LOOKUP_NO_NARROWING (LOOKUP_NO_TEMP_BIND << 1)
>   /* We're looking up a constructor for list-initialization.  */
>   #define LOOKUP_LIST_INIT_CTOR (LOOKUP_NO_NARROWING << 1)
>   /* This is the first parameter of a copy constructor.  */
> diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> index 048612de400..1d84b451c9f 100644
> --- a/gcc/cp/except.cc
> +++ b/gcc/cp/except.cc
> @@ -715,25 +715,10 @@ build_throw (location_t loc, tree exp)
>   	     treated as an rvalue for the purposes of overload resolution
>   	     to favor move constructors over copy constructors.  */
>   	  if (tree moved = treat_lvalue_as_rvalue_p (exp, /*return*/false))
> -	    {
> -	      if (cxx_dialect < cxx20)
> -		{
> -		  releasing_vec exp_vec (make_tree_vector_single (moved));
> -		  moved = (build_special_member_call
> -			   (object, complete_ctor_identifier, &exp_vec,
> -			    TREE_TYPE (object), flags|LOOKUP_PREFER_RVALUE,
> -			    tf_none));
> -		  if (moved != error_mark_node)
> -		    {
> -		      exp = moved;
> -		      converted = true;
> -		    }
> -		}
> -	      else
> -		/* In C++20 we just treat the return value as an rvalue that
> -		   can bind to lvalue refs.  */
> -		exp = moved;
> -	    }
> +	    /* In C++20 we just treat the return value as an rvalue that

Let's remove the above "just" since we're removing the more complicated 
code it's contrasting with.

> +	       can bind to lvalue refs.  In C++23, such an expression is just
> +	       an xvalue.  */
> +	    exp = moved;
>   
>   	  /* Call the copy constructor.  */
>   	  if (!converted)
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 5f16c4d2426..c525e880383 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -10692,21 +10692,12 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p)
>   	  tree t = convert_for_initialization (NULL_TREE, type,
>   					       moved,
>   					       (LOOKUP_NORMAL
> -						| LOOKUP_ONLYCONVERTING
> -						| LOOKUP_PREFER_RVALUE),
> +						| LOOKUP_ONLYCONVERTING),
>   					       ICR_RETURN, NULL_TREE, 0,
>   					       tf_none);
>   	  /* If this worked, implicit rvalue would work, so the call to
>   	     std::move is redundant.  */
> -	  if (t != error_mark_node
> -	      /* Trying to move something const will never succeed unless
> -		 there's T(const T&&), which it almost never is, and if
> -		 so, T wouldn't be error_mark_node now: the above convert_
> -		 call with LOOKUP_PREFER_RVALUE returns an error if a const T&
> -		 overload is selected.  */
> -	      || (CP_TYPE_CONST_P (TREE_TYPE (arg))
> -		  && same_type_ignoring_top_level_qualifiers_p
> -		  (TREE_TYPE (arg), type)))
> +	  if (t != error_mark_node)
>   	    {
>   	      auto_diagnostic_group d;
>   	      if (warning_at (loc, OPT_Wredundant_move,
> @@ -11049,23 +11040,10 @@ check_return_expr (tree retval, bool *no_warning)
>   	     ? CLASS_TYPE_P (functype)
>   	     : !SCALAR_TYPE_P (functype) || !SCALAR_TYPE_P (TREE_TYPE (retval)))
>   	    && (moved = treat_lvalue_as_rvalue_p (retval, /*return*/true)))
> -	{
> -	  if (cxx_dialect < cxx20)
> -	    {
> -	      moved = convert_for_initialization
> -		(NULL_TREE, functype, moved, flags|LOOKUP_PREFER_RVALUE,
> -		 ICR_RETURN, NULL_TREE, 0, tf_none);
> -	      if (moved != error_mark_node)
> -		{
> -		  retval = moved;
> -		  converted = true;
> -		}
> -	    }
> -	  else
> -	    /* In C++20 we just treat the return value as an rvalue that
> -	       can bind to lvalue refs.  */
> -	    retval = moved;
> -	}
> +	  /* In C++20 and earlier we just treat the return value as an rvalue

Here, too.

> +	     that can bind to lvalue refs.  In C++23, such an expression is just
> +	     an xvalue (see reference_binding).  */
> +	  retval = moved;
>   
>         /* The call in a (lambda) thunk needs no conversions.  */
>         if (TREE_CODE (retval) == CALL_EXPR
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
> index a215a4774d6..17dd807aea8 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
> @@ -57,5 +57,5 @@ struct S2: S1 {};
>   
>   S1 f3(const S2 s)
>   {
> -  return std::move(s); // { dg-warning "redundant move" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move" }
>   }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
> index 3fec525879d..6547777c007 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
> @@ -28,7 +28,7 @@ struct S2 : S1 {};
>   S1
>   f (S2 s)
>   {
> -  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move in return statement" }
>   }
>   
>   struct R1 {
> @@ -40,7 +40,7 @@ struct R2 : R1 {};
>   R1
>   f2 (const R2 s)
>   {
> -  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move in return statement" }
>   }
>   
>   struct T1 {
> @@ -55,5 +55,5 @@ f3 (const T2 s)
>   {
>     // Without std::move: const T1 &
>     // With std::move: const T1 &&
> -  return std::move(s); // { dg-warning "redundant move in return statement" "" { target c++20 } }
> +  return std::move(s); // { dg-warning "redundant move in return statement" }
>   }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return2.C b/gcc/testsuite/g++.dg/cpp0x/move-return2.C
> index 999f2c95c49..8e750efb870 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/move-return2.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/move-return2.C
> @@ -7,5 +7,5 @@ struct S2 : S1 {};
>   S1
>   f (S2 s)
>   {
> -  return s; // { dg-error "use of deleted function" "" { target c++17_down } }
> +  return s;
>   }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return4.C b/gcc/testsuite/g++.dg/cpp0x/move-return4.C
> index 3fc58089319..0f0ca1fc253 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/move-return4.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/move-return4.C
> @@ -13,5 +13,5 @@ struct A : Base
>   A<int> foo()
>   {
>       A<double> v;
> -    return v; // { dg-error "cannot bind rvalue reference" "" { target c++17_down } }
> +    return v;
>   }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/move-return5.C b/gcc/testsuite/g++.dg/cpp0x/move-return5.C
> new file mode 100644
> index 00000000000..695000b2687
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/move-return5.C
> @@ -0,0 +1,20 @@
> +// { dg-do compile { target c++11 } }
> +// This used to compile in C++11...17 because we performed two
> +// separate overload resolutions: one treating the operand as
> +// an rvalue, and then (if that resolution fails) another one
> +// treating the operand as an lvalue.
> +
> +struct W {
> +  W();
> +};
> +
> +struct F {
> +  F(W&);
> +  F(W&&) = delete;
> +};
> +
> +F fn ()
> +{
> +  W w;
> +  return w; // { dg-error "use of deleted function" }
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C b/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
> index cfbef300226..314f19bb864 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
> @@ -52,7 +52,7 @@ f5 ()
>   int
>   main ()
>   {
> -  int return_lval = __cplusplus > 201703L ? -1 : 2;
> +  int return_lval = -1;
>     Y y1 = f (A());
>     if (y1.y != return_lval)
>       __builtin_abort ();
> 
> base-commit: 9f65eecdbef6027722e93aadf3fa6b3cc342eb4f


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-10-10 20:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28 21:26 [PATCH] c++: Remove maybe-rvalue OR in implicit move Marek Polacek
2022-10-10 19:19 ` Marek Polacek
2022-10-10 20:15 ` Jason Merrill

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).