procyberian/gcc
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

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

Browse source 
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.
This commit is contained in:
Marek Polacek 2022-09-28 10:37:34 -04:00
parent a0c1a05910
commit bc0d700b57
10 changed files with 41 additions and 97 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
gcc/cp/call.cc
View file

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

6
gcc/cp/cp-tree.h
View file

@ -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. */

23
gcc/cp/except.cc
View file

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

34
gcc/cp/typeck.cc
View file

@ -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

2
gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C
View file

@ -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" }
}

6
gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
View file

@ -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" }
}

2
gcc/testsuite/g++.dg/cpp0x/move-return2.C
View file

@ -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;
}

2
gcc/testsuite/g++.dg/cpp0x/move-return4.C
View file

@ -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;
}

20
gcc/testsuite/g++.dg/cpp0x/move-return5.C Normal file
View file

@ -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" }
}

2
gcc/testsuite/g++.dg/cpp0x/ref-qual20.C
View file

@ -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 ();