From patchwork Mon Aug 8 20:27:10 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 434 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6a10:20da:b0:2d3:3019:e567 with SMTP id n26csp2083439pxc; Mon, 8 Aug 2022 13:28:03 -0700 (PDT) X-Google-Smtp-Source: AA6agR7ZtQ6sXr/DGInP29fMbTCc4JM+GA0IN6cmMI4iaNLlRBVSH7PcaqZ5yXdzSrL3zbmLhhLl X-Received: by 2002:a17:907:6890:b0:731:41a9:bb03 with SMTP id qy16-20020a170907689000b0073141a9bb03mr6392493ejc.302.1659990483415; Mon, 08 Aug 2022 13:28:03 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1659990483; cv=none; d=google.com; s=arc-20160816; b=Sks5cBo2/2BhXjCO7obfJGZrVBCgKEP9hnR7aO25b02BtpmuW5fJw1itHNNT3K9n0y 6DAFPNdaEZdFuoQn6pkkrT296e4qaEl8q9JiQXZQNha7gMn61Qe3S5ro4caopsrMHCVT Ia/7WdrEOUr/OxkDEZ7kmBlFb/QC5Lfm8gbCx22yb5c5Bj3xPEN7tFlj0RrMYXQQ10+n 1GwOcVSvhTrI/4FLHV06yf/mSIIDH3To1CO1GtmiwpmC+VkEGpIqUJEX5dZQjjw7eH9k cPMGgXuZsP9us1lO+E28juT4MUF6sGDAXYP4MDKYXkoJ2D7Cry/kC0VOO+CASdUnGjZR R3jg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:cc:reply-to:from:list-subscribe:list-help :list-post:list-archive:list-unsubscribe:list-id:precedence :content-disposition:user-agent:in-reply-to:mime-version:references :message-id:subject:to:date:dmarc-filter:delivered-to:dkim-signature :dkim-filter; bh=6HACZ4x9rCq61EnH98w3qmYgD+GrGwDiGdQtJ/7QJK8=; b=VsMo3ZFdcZJBsbbXL3/REmJMK28UDtSFeimnb8Yuh9BTPn3LcvXriSIi08sTq27T9D nfkvVMG+Gp8lrgFAHggIb8rAcG0STY4rQtqG7SP00QKp7oMfazVf8R9rURrzhtpRCO+y s7GIfjNHC7xRIiocUABf+hVjL/bzd95NirO5W1KCSVPqS19U3O5wQWUol4xMwxdktDGS +KY7b6yvq3HFOVkMBXeIzemVgcpu6Ut89IugQrVIq3vlkykuql5ZGfQ6ygU73v+uyDvy TKWoae9FoAW7b0C2tNYuHoIPJP9rghQZjj7k4pPwMS7+cDuRIB94C/i4oIUerY3Up5FT eptw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=b4jHREvX; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id ws13-20020a170907704d00b00726c7fc61dfsi461478ejb.103.2022.08.08.13.28.02 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Aug 2022 13:28:03 -0700 (PDT) Received-SPF: pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=b4jHREvX; spf=pass (google.com: domain of gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=gnu.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2117B3857033 for ; Mon, 8 Aug 2022 20:28:02 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2117B3857033 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1659990482; bh=6HACZ4x9rCq61EnH98w3qmYgD+GrGwDiGdQtJ/7QJK8=; h=Date:To:Subject:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=b4jHREvXMAUrEx8naBs++NM5Zr+yXIZFB1l9M7E+oQoZsxiTCrUuoIbYblAalPKyR jVC8ThjdCWeZ2Gdbd2/34DJ2pOuOMj5RwCef1amiSa4X0sr1AmgY3aCbqWrjMC2BJA 2f5jHws0taRBZfghlbBg/hi8oxSI+eaj4EmTpVh0= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 3B5C13857033 for ; Mon, 8 Aug 2022 20:27:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3B5C13857033 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-91-F8fPvcTzM1acJnZqfquycQ-1; Mon, 08 Aug 2022 16:27:14 -0400 X-MC-Unique: F8fPvcTzM1acJnZqfquycQ-1 Received: by mail-qk1-f197.google.com with SMTP id i15-20020a05620a404f00b006b55998179bso8682760qko.4 for ; Mon, 08 Aug 2022 13:27:14 -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:cc:to:from:date:x-gm-message-state:from:to:cc; bh=6HACZ4x9rCq61EnH98w3qmYgD+GrGwDiGdQtJ/7QJK8=; b=cxVTKgZ4j6jTUsAf8tSwPrqbRTQocuozuc7KVgE88S8WXZjNVbD0M3kGcn80AX2EPn Q5cIRJuo5/QSuZOwKTnbMRqdmoYWlMAr4qmzts1aijq1EQRxd3BLKNdH9qwo6YBT2+i3 Ek8eVO/EhtpmBfEmDx4Cz1f8fj64/YuqbsW/k/CSSEX/0UwLSPIruyxcogy3N+leuTm/ OkdbRAjFJNRZGHop52vtHO2rK2ecoNf6Q3jBjQhA1C8EY6Mm2qGM7BgCxCoyGs55hB6G c4FFde/+2mplptPcwsAmGWxDW9kHG5Cg52IGs5sYD/U/DtlOeqa7wYsn5tCv2sH4Tdzr nzjw== X-Gm-Message-State: ACgBeo1RrP8Ept/VnwyjlUBDlRqUwm363Zn7wVbXPHPvZCZQXRPPcCal /FUS7DfrNi9ErFKuu7pRGFdkzpm6OqtPXxWtuWCR7SPO8/juV1W5Y6Pg8/IDRTWwvXKL0cQ9sYr 6F6KUsn1uY6DpmSmbjA== X-Received: by 2002:a0c:cb0c:0:b0:476:d5e7:e0ca with SMTP id o12-20020a0ccb0c000000b00476d5e7e0camr17019135qvk.57.1659990433260; Mon, 08 Aug 2022 13:27:13 -0700 (PDT) X-Received: by 2002:a0c:cb0c:0:b0:476:d5e7:e0ca with SMTP id o12-20020a0ccb0c000000b00476d5e7e0camr17019116qvk.57.1659990432793; Mon, 08 Aug 2022 13:27:12 -0700 (PDT) Received: from redhat.com ([2601:184:4780:4310::e531]) by smtp.gmail.com with ESMTPSA id ck5-20020a05622a230500b0031f287f58b4sm8647613qtb.51.2022.08.08.13.27.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Aug 2022 13:27:12 -0700 (PDT) Date: Mon, 8 Aug 2022 16:27:10 -0400 To: Jason Merrill Subject: [PATCH v2] c++: Extend -Wredundant-move for const-qual objects [PR90428] Message-ID: References: <20220806181313.337246-1-polacek@redhat.com> <4c50b759-f207-7c80-b3f0-efa00a9b73a6@redhat.com> MIME-Version: 1.0 In-Reply-To: <4c50b759-f207-7c80-b3f0-efa00a9b73a6@redhat.com> User-Agent: Mutt/2.2.6 (2022-06-05) X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Disposition: inline X-Spam-Status: No, score=-13.7 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_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Marek Polacek via Gcc-patches From: Marek Polacek Reply-To: Marek Polacek Cc: GCC Patches Errors-To: gcc-patches-bounces+ouuuleilei=gmail.com@gcc.gnu.org Sender: "Gcc-patches" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1740436559665565146?= X-GMAIL-MSGID: =?utf-8?q?1740626181054798572?= On Sat, Aug 06, 2022 at 03:58:13PM -0800, Jason Merrill wrote: > On 8/6/22 11:13, Marek Polacek wrote: > > In this PR, Jon suggested extending the -Wredundant-move warning > > to warn when the user is moving a const object as in: > > > > struct T { }; > > > > T f(const T& t) > > { > > return std::move(t); > > } > > > > where the std::move is redundant, because T does not have > > a T(const T&&) constructor (which is very unlikely). Even with > > the std::move, T(T&&) would not be used because it would mean > > losing the const. Instead, T(const T&) will be called. > > > > I had to restructure the function a bit, but it's better now. This patch > > depends on my other recent patches to maybe_warn_pessimizing_move. > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? > > > > PR c++/90428 > > > > gcc/cp/ChangeLog: > > > > * typeck.cc (maybe_warn_pessimizing_move): Extend the > > -Wredundant-move warning to warn about std::move on a > > const-qualified object. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/cpp0x/Wredundant-move1.C: Adjust dg-warning. > > * g++.dg/cpp0x/Wredundant-move9.C: Likewise. > > * g++.dg/cpp0x/Wredundant-move10.C: New test. > > --- > > gcc/cp/typeck.cc | 157 +++++++++++------- > > gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C | 3 +- > > .../g++.dg/cpp0x/Wredundant-move10.C | 61 +++++++ > > gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C | 3 +- > > 4 files changed, 162 insertions(+), 62 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C > > > > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc > > index 70a5efc45de..802bc9c43fb 100644 > > --- a/gcc/cp/typeck.cc > > +++ b/gcc/cp/typeck.cc > > @@ -10411,72 +10411,109 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) > > return; > > } > > - /* We're looking for *std::move ((T &) &arg). */ > > - if (REFERENCE_REF_P (expr) > > - && TREE_CODE (TREE_OPERAND (expr, 0)) == CALL_EXPR) > > - { > > - tree fn = TREE_OPERAND (expr, 0); > > - if (is_std_move_p (fn)) > > - { > > - tree arg = CALL_EXPR_ARG (fn, 0); > > - tree moved; > > - if (TREE_CODE (arg) != NOP_EXPR) > > - return; > > - arg = TREE_OPERAND (arg, 0); > > - if (TREE_CODE (arg) != ADDR_EXPR) > > - return; > > - arg = TREE_OPERAND (arg, 0); > > - arg = convert_from_reference (arg); > > - if (can_do_rvo_p (arg, type)) > > - { > > - auto_diagnostic_group d; > > - if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) > > - && warning_at (loc, OPT_Wpessimizing_move, > > - "moving a temporary object prevents copy " > > - "elision")) > > - inform (loc, "remove % call"); > > - } > > - /* The rest of the warnings is only relevant for when we are > > - returning from a function. */ > > - else if (!return_p) > > - return; > > - /* Warn if we could do copy elision were it not for the move. */ > > - else if (can_do_nrvo_p (arg, type)) > > + /* First, check if this is a call to std::move. */ > > + if (!REFERENCE_REF_P (expr) > > + || TREE_CODE (TREE_OPERAND (expr, 0)) != CALL_EXPR) > > + return; > > + tree fn = TREE_OPERAND (expr, 0); > > + if (!is_std_move_p (fn)) > > + return; > > + tree arg = CALL_EXPR_ARG (fn, 0); > > + if (TREE_CODE (arg) != NOP_EXPR) > > + return; > > + /* If we're looking at *std::move ((T &) &arg), do the pessimizing N/RVO > > + and implicitly-movable warnings. */ > > + if (TREE_CODE (TREE_OPERAND (arg, 0)) == ADDR_EXPR) > > + { > > + arg = TREE_OPERAND (arg, 0); > > + arg = TREE_OPERAND (arg, 0); > > + arg = convert_from_reference (arg); > > + if (can_do_rvo_p (arg, type)) > > Incidentally, this function should probably have a different name if we're > checking it in non-return situations. I've renamed it to can_elide_copy_prvalue_p in this patch. > > + { > > + auto_diagnostic_group d; > > + if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) > > + && warning_at (loc, OPT_Wpessimizing_move, > > + "moving a temporary object prevents copy elision")) > > + inform (loc, "remove % call"); > > + } > > + /* The rest of the warnings is only relevant for when we are returning > > + from a function. */ > > + if (!return_p) > > + return; > > + > > + tree moved; > > + /* Warn if we could do copy elision were it not for the move. */ > > + if (can_do_nrvo_p (arg, type)) > > + { > > + auto_diagnostic_group d; > > + if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) > > + && warning_at (loc, OPT_Wpessimizing_move, > > + "moving a local object in a return statement " > > + "prevents copy elision")) > > + inform (loc, "remove % call"); > > + } > > + /* Warn if the move is redundant. It is redundant when we would > > + do maybe-rvalue overload resolution even without std::move. */ > > + else if (warn_redundant_move > > + && !warning_suppressed_p (expr, OPT_Wredundant_move) > > + && (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true))) > > + { > > + /* Make sure that overload resolution would actually succeed > > + if we removed the std::move call. */ > > + tree t = convert_for_initialization (NULL_TREE, type, > > + moved, > > + (LOOKUP_NORMAL > > + | LOOKUP_ONLYCONVERTING > > + | LOOKUP_PREFER_RVALUE), > > + 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. So do warn. */ > > + || (CP_TYPE_CONST_P (TREE_TYPE (arg)) > > + && same_type_ignoring_top_level_qualifiers_p > > + (TREE_TYPE (arg), type))) > > I'm confused by the || case here. Overload resolution fails if we drop the > move call, but we're still going to warn? This is to warn about this: T f5(const T t) { return std::move(t); // { dg-warning "redundant move" } } where OR fails because there's no T(const T&&) (or it's deleted in which case convert_for_initialization also returns error_mark_node). This OR is going to fail with std::move but also without std::move when we're trying to treat an lvalue as an rvalue. So the std::move has no effect, because T(const T&) will be called in either case. Now, if there was a T(const T&&), we'd *still* warn, because the std::move would still be redundant. Does that make sense? Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? -- >8 -- In this PR, Jon suggested extending the -Wredundant-move warning to warn when the user is moving a const object as in: struct T { }; T f(const T& t) { return std::move(t); } where the std::move is redundant, because T does not have a T(const T&&) constructor (which is very unlikely). Even with the std::move, T(T&&) would not be used because it would mean losing the const. Instead, T(const T&) will be called. I had to restructure the function a bit, but it's better now. This patch depends on my other recent patches to maybe_warn_pessimizing_move. PR c++/90428 gcc/cp/ChangeLog: * typeck.cc (can_do_rvo_p): Rename to ... (can_elide_copy_prvalue_p): ... this. (maybe_warn_pessimizing_move): Extend the -Wredundant-move warning to warn about std::move on a const-qualified object. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/Wredundant-move1.C: Adjust dg-warning. * g++.dg/cpp0x/Wredundant-move9.C: Likewise. * g++.dg/cpp0x/Wredundant-move10.C: New test. --- gcc/cp/typeck.cc | 157 +++++++++++------- gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C | 3 +- .../g++.dg/cpp0x/Wredundant-move10.C | 61 +++++++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C | 3 +- 4 files changed, 162 insertions(+), 62 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C base-commit: 053876cdbe8057210e6f4da4eec2df58f92ccd4c prerequisite-patch-id: f4862bc588ce5fed1d21016fecc4b61feb71eba5 prerequisite-patch-id: ce490c3c0b991fa7f27315387949c145c66223a4 prerequisite-patch-id: bf551f0fe75fa8f6775fe53469b61ae94d8cab81 diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 70a5efc45de..5be79b83871 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10297,7 +10297,7 @@ can_do_nrvo_p (tree retval, tree functype) prvalue. */ static bool -can_do_rvo_p (tree retval, tree functype) +can_elide_copy_prvalue_p (tree retval, tree functype) { if (functype == error_mark_node) return false; @@ -10411,72 +10411,109 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) return; } - /* We're looking for *std::move ((T &) &arg). */ - if (REFERENCE_REF_P (expr) - && TREE_CODE (TREE_OPERAND (expr, 0)) == CALL_EXPR) + /* First, check if this is a call to std::move. */ + if (!REFERENCE_REF_P (expr) + || TREE_CODE (TREE_OPERAND (expr, 0)) != CALL_EXPR) + return; + tree fn = TREE_OPERAND (expr, 0); + if (!is_std_move_p (fn)) + return; + tree arg = CALL_EXPR_ARG (fn, 0); + if (TREE_CODE (arg) != NOP_EXPR) + return; + /* If we're looking at *std::move ((T &) &arg), do the pessimizing N/RVO + and implicitly-movable warnings. */ + if (TREE_CODE (TREE_OPERAND (arg, 0)) == ADDR_EXPR) { - tree fn = TREE_OPERAND (expr, 0); - if (is_std_move_p (fn)) - { - tree arg = CALL_EXPR_ARG (fn, 0); - tree moved; - if (TREE_CODE (arg) != NOP_EXPR) - return; - arg = TREE_OPERAND (arg, 0); - if (TREE_CODE (arg) != ADDR_EXPR) - return; - arg = TREE_OPERAND (arg, 0); - arg = convert_from_reference (arg); - if (can_do_rvo_p (arg, type)) - { - auto_diagnostic_group d; - if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) - && warning_at (loc, OPT_Wpessimizing_move, - "moving a temporary object prevents copy " - "elision")) - inform (loc, "remove % call"); - } - /* The rest of the warnings is only relevant for when we are - returning from a function. */ - else if (!return_p) - return; - /* Warn if we could do copy elision were it not for the move. */ - else if (can_do_nrvo_p (arg, type)) + arg = TREE_OPERAND (arg, 0); + arg = TREE_OPERAND (arg, 0); + arg = convert_from_reference (arg); + if (can_elide_copy_prvalue_p (arg, type)) + { + auto_diagnostic_group d; + if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) + && warning_at (loc, OPT_Wpessimizing_move, + "moving a temporary object prevents copy elision")) + inform (loc, "remove % call"); + } + /* The rest of the warnings is only relevant for when we are returning + from a function. */ + if (!return_p) + return; + + tree moved; + /* Warn if we could do copy elision were it not for the move. */ + if (can_do_nrvo_p (arg, type)) + { + auto_diagnostic_group d; + if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) + && warning_at (loc, OPT_Wpessimizing_move, + "moving a local object in a return statement " + "prevents copy elision")) + inform (loc, "remove % call"); + } + /* Warn if the move is redundant. It is redundant when we would + do maybe-rvalue overload resolution even without std::move. */ + else if (warn_redundant_move + && !warning_suppressed_p (expr, OPT_Wredundant_move) + && (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true))) + { + /* Make sure that overload resolution would actually succeed + if we removed the std::move call. */ + tree t = convert_for_initialization (NULL_TREE, type, + moved, + (LOOKUP_NORMAL + | LOOKUP_ONLYCONVERTING + | LOOKUP_PREFER_RVALUE), + 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. So do warn. */ + || (CP_TYPE_CONST_P (TREE_TYPE (arg)) + && same_type_ignoring_top_level_qualifiers_p + (TREE_TYPE (arg), type))) { auto_diagnostic_group d; - if (!warning_suppressed_p (expr, OPT_Wpessimizing_move) - && warning_at (loc, OPT_Wpessimizing_move, - "moving a local object in a return statement " - "prevents copy elision")) + if (warning_at (loc, OPT_Wredundant_move, + "redundant move in return statement")) inform (loc, "remove % call"); } - /* Warn if the move is redundant. It is redundant when we would - do maybe-rvalue overload resolution even without std::move. */ - else if (warn_redundant_move - && !warning_suppressed_p (expr, OPT_Wredundant_move) - && (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true))) - { - /* Make sure that overload resolution would actually succeed - if we removed the std::move call. */ - tree t = convert_for_initialization (NULL_TREE, type, - moved, - (LOOKUP_NORMAL - | LOOKUP_ONLYCONVERTING - | LOOKUP_PREFER_RVALUE), - 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) - { - auto_diagnostic_group d; - if (warning_at (loc, OPT_Wredundant_move, - "redundant move in return statement")) - inform (loc, "remove % call"); - } - } } } + /* Also try to warn about redundant std::move in code such as + T f (const T& t) + { + return std::move(t); + } + for which EXPR will be something like + *std::move ((const struct T &) (const struct T *) t) + and where the std::move does nothing if T does not have a T(const T&&) + constructor, because the argument is const. It will not use T(T&&) + because that would mean losing the const. */ + else if (TREE_CODE (TREE_TYPE (arg)) == REFERENCE_TYPE + && CP_TYPE_CONST_P (TREE_TYPE (TREE_TYPE (arg)))) + { + tree rtype = TREE_TYPE (TREE_TYPE (arg)); + if (!same_type_ignoring_top_level_qualifiers_p (rtype, type)) + return; + /* Check for the unlikely case there's T(const T&&) (we don't care if + it's deleted). */ + for (tree fn : ovl_range (CLASSTYPE_CONSTRUCTORS (rtype))) + if (move_fn_p (fn)) + { + tree t = TREE_VALUE (FUNCTION_FIRST_USER_PARMTYPE (fn)); + if (UNLIKELY (CP_TYPE_CONST_P (TREE_TYPE (t)))) + return; + } + auto_diagnostic_group d; + if (warning_at (loc, OPT_Wredundant_move, + "redundant move in return statement")) + inform (loc, "remove % call"); + } } /* Check that returning RETVAL from the current function is valid. diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C index ce4087b476f..c227019cce1 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C @@ -60,7 +60,8 @@ fn4 (const T t) { // t is const: will decay into copy despite std::move, so it's redundant. // We used to warn about this, but no longer since c++/87378. - return std::move (t); // { dg-warning "redundant move" "" { target c++20 } } + // Now we warn again since c++/90428. + return std::move (t); // { dg-warning "redundant move" } } int diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C new file mode 100644 index 00000000000..a215a4774d6 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move10.C @@ -0,0 +1,61 @@ +// PR c++/90428 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template + struct remove_reference + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast::type&&>(__t); } +} + +struct T { T(); T(const T&); T(T&&) = delete; }; +struct S : T { }; +struct W { W(const W&); W(W&&) = delete; W(const W&&); }; + +T f1(T t) +{ + const T& rt = t; + return std::move(rt); // { dg-warning "redundant move" } +} + +T f2(const T& t) +{ + return std::move(t); // { dg-warning "redundant move" } +} + +W f3(const W& w) +{ + return std::move(w); +} + +T f4(const S& s) +{ + return std::move(s); +} + +T f5(const T t) +{ + return std::move(t); // { dg-warning "redundant move" } +} + +struct S1 { S1(S1 &&) = delete; S1(const S1&); }; +struct S2: S1 {}; + +S1 f3(const S2 s) +{ + return std::move(s); // { dg-warning "redundant move" "" { target c++20 } } +} diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C index ca1e23b7a4b..489ecd2b9c9 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move9.C @@ -61,7 +61,8 @@ fn4 (const T t) { // t is const: will decay into copy despite std::move, so it's redundant. // We used to warn about this, but no longer since c++/87378. - return std::move (t); // { dg-warning "redundant move" "" { target c++20 } } + // Now we warn again since c++/90428. + return std::move (t); // { dg-warning "redundant move" } } int