c++: Remove maybe-rvalue OR in implicit move

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.

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.

10 files changed, 41 insertions, 97 deletions

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index 2fa33c5..b9c08d6 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;
@@ -8292,9 +8277,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)
 	{
@@ -9560,23 +9542,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 80037fa..3b67be6 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5874,12 +5874,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 b8a85ed..703d1d5 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 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 b4a8e3c..69c378d 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10697,21 +10697,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,
@@ -11054,23 +11045,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 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 a215a47..17dd807 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 3fec525..6547777 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 999f2c95..8e750ef 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 3fc5808..0f0ca1f 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 0000000..695000b
--- /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 cfbef30..314f19b 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 ();