From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id C64B53858D37 for ; Mon, 10 Oct 2022 19:19:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C64B53858D37 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1665429574; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=W/Wk2goMQgHp2PwvjTktDenotPcHEgF7aPjkbmKn5uw=; b=gUnTCiBz4bGI72AroGTNZh20kbq7g7fz2vWS1dT1HI2mFziKb+uPHz3g3ltS8SNGywkRsB 1HMOqnv0sSh8YD8vhpg/oR4Uwl0+AP4jWOSLy60cFjfnNjg2Thm7luk0mC6m1nraTyqtb4 9g5tNhAASTshXZCge3UflUCmQ73Lk9I= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-372-7tqGWCyVNrS40OGtx4EYxA-1; Mon, 10 Oct 2022 15:19:33 -0400 X-MC-Unique: 7tqGWCyVNrS40OGtx4EYxA-1 Received: by mail-qt1-f199.google.com with SMTP id u14-20020a05622a17ce00b003947c24705dso7512039qtk.15 for ; Mon, 10 Oct 2022 12:19:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=user-agent:in-reply-to:content-disposition:mime-version:references :message-id:subject:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=W/Wk2goMQgHp2PwvjTktDenotPcHEgF7aPjkbmKn5uw=; b=dSYU154bAdt6vlBcEKbCuZ7IQrabdiF/wBoe4tRKfYK7jS3aOL3eYsePICghyj4E7n LwZ+UhyKH1xGuhJSyI8QnygPwQV2oVWi5kNj7+aiFHx4x/q6/KxuJL9YihzCF+eoiAJ4 V0UfxLDItJUYYjrQ8BoKSl+D9p3nZU4B3MwXfN7MVinB4aapYnZ4b2u0VRCAT7fT2BTy 59CCNJ91aiq48bhgQUA6R9RgE9q97eEVmJs13n23qKA/nxuBq4SjnAr1Md+3J75Ebm18 ocC4kLYGwAWy4SMhrlU+rXB/lUbUYkLJLTLhi/ae5+eInR4ynB1hSfQVAUc8Mhr8Hszh UomQ== X-Gm-Message-State: ACrzQf19hJKZaPCqVk/bf5RRwjcyakQc3Og1Og8LB+bF2cqmF1nmb4wk h/pTb7XPwRC/NWu7ZHVdfYu0dQGi1m4HdIlo8aaGYvHCCA0jSrk9aSdVqg0PLXdlUspHwr1HMPB hrBFfUGPEvBohdgurs4E2l2Ozn3+a3hH9pkGwK3sxLnvQSA41xhiyecK1T8L5JahcVkZx X-Received: by 2002:a05:622a:13c6:b0:35b:a742:28ae with SMTP id p6-20020a05622a13c600b0035ba74228aemr15912053qtk.435.1665429572532; Mon, 10 Oct 2022 12:19:32 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4Hx3U9XMAMtx3PJVuJta22/tUBAft+UaDgParYCrw+x5sDpDXFiOf2z7ACQiyHcYauBDva7g== X-Received: by 2002:a05:622a:13c6:b0:35b:a742:28ae with SMTP id p6-20020a05622a13c600b0035ba74228aemr15912022qtk.435.1665429572020; Mon, 10 Oct 2022 12:19:32 -0700 (PDT) Received: from redhat.com (2603-7000-9500-2e39-0000-0000-0000-1db4.res6.spectrum.com. [2603:7000:9500:2e39::1db4]) by smtp.gmail.com with ESMTPSA id x14-20020ac87a8e000000b003988b3d5280sm5321871qtr.70.2022.10.10.12.19.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Oct 2022 12:19:31 -0700 (PDT) Date: Mon, 10 Oct 2022 15:19:29 -0400 From: Marek Polacek To: GCC Patches , Jason Merrill Subject: Re: [PATCH] c++: Remove maybe-rvalue OR in implicit move Message-ID: References: <20220928212634.1275032-1-polacek@redhat.com> MIME-Version: 1.0 In-Reply-To: <20220928212634.1275032-1-polacek@redhat.com> User-Agent: Mutt/2.2.7 (2022-08-07) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-12.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 foo() > { > A 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