From patchwork Thu Aug 4 18:46:42 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 390 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6a10:b5d6:b0:2b9:3548:2db5 with SMTP id v22csp561192pxt; Thu, 4 Aug 2022 11:47:42 -0700 (PDT) X-Google-Smtp-Source: AA6agR5kPjWm0pfI1tWro8fWNBG6P+YhSRbpRZEvXZ7gYa15B7xOje/CL8DmK4RFsFLRKtIBnrJE X-Received: by 2002:aa7:d159:0:b0:43d:73ba:64cf with SMTP id r25-20020aa7d159000000b0043d73ba64cfmr3450337edo.36.1659638862195; Thu, 04 Aug 2022 11:47:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1659638862; cv=none; d=google.com; s=arc-20160816; b=vgQ7DiKHe6B5xE9ng2wVUqoHIWSPnYmwbfQyNjUtu7thF99W6Q1KMSAzIkWF0kaJY7 cyRX2kWLZA8JgE52On+dmrYC8dqQQdWKyGerVVoO1rGIN4epBsIyemC149wHjOV/S7R+ ZW3rwQ3oKLkw+74F1KMSBFsG1G/FBfgT3VYV0Ydi9eFyd35TKcwLsEGoIEEKi1w0A4wh NJ7NH6xoE/iZNdjQpjVASXJL5SOgW0H+dNJKr1tIlU4l2/MnzFlL9EE8AJN2RJiTgyM+ zw7MXpLF/t0d4GMhFkMcfujC0I9uSZVRNFjuts/v3nUTIXAHHrTqtBg/PvtQUwsnxcGz a7Jg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-transfer-encoding:mime-version:message-id:date:subject:to :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=UuKD114KjLUy5l4qsKuwGsABJCN+1PGrP1rqKfubf7M=; b=WWe6dl59mBy0uMMCkLby6Z/rFhWW0TL2Pr8ser9PW8ddPW9yx2c4S/PeWntpnKcCIz g5yH6oihEqEWONJioosl3vFwjpzRuZHKcXbPBa4raUuSBb9lymHHbJB534vyr1KqSqnP jtZknHVODtrWJz7Nverh710mYp1P/WOZaRRqzlB592cN1Qc9QBuk/T/K5LmGhFW7reeW NGHDFYIH25rE129CYAubXH7SaSgkZQFRUQ52JWc1lrviuL98cb5Ev4JXm2XZtpc5NgKO uh+u2gOmsLkk5DnChJEqCJS+p3hqH87j1yJkD9YjsxR9rbtd+PZePT7QF7b1Oxh+303D Uxpw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b=EMn3fVGT; 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 (ip-8-43-85-97.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id ne24-20020a1709077b9800b006fe4c66b711si1893031ejc.46.2022.08.04.11.47.41 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Aug 2022 11:47:42 -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=EMn3fVGT; 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 DABEE385AC32 for ; Thu, 4 Aug 2022 18:47:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org DABEE385AC32 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1659638860; bh=UuKD114KjLUy5l4qsKuwGsABJCN+1PGrP1rqKfubf7M=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=EMn3fVGTadvBgZWr2X15J2Uqr4Zh9tB5sik33idgrFmfSJ04YlPC9enDn9NUSZzrZ qZB7qypYCdcx597ikNnirsQ08FjagcnkJ/8F8tob6kdAii79aS04OZzDkvZhjWmcYK W/IvYYFbBl5X4EGAbgEBZmWfrxrNPtk0LIdFZyb8= 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 89B1138582AE for ; Thu, 4 Aug 2022 18:46:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 89B1138582AE Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-599-dtvXDiPdPrm4z58sbkvOYw-1; Thu, 04 Aug 2022 14:46:55 -0400 X-MC-Unique: dtvXDiPdPrm4z58sbkvOYw-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 835518037AC for ; Thu, 4 Aug 2022 18:46:55 +0000 (UTC) Received: from pdp-11.redhat.com (unknown [10.22.17.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6023F492C3B; Thu, 4 Aug 2022 18:46:55 +0000 (UTC) To: GCC Patches , Jason Merrill Subject: [PATCH] c++: Tweak for -Wpessimizing-move in templates [PR89780] Date: Thu, 4 Aug 2022 14:46:42 -0400 Message-Id: <20220804184642.106756-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.8 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 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 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?1740257479534091202?= X-GMAIL-MSGID: =?utf-8?q?1740257479534091202?= In my previous patches I've been extending our std::move warnings, but this tweak actually dials it down a little bit. As reported in bug 89780, it's questionable to warn about expressions in templates that were type-dependent, but aren't anymore because we're instantiating the template. As in, template Dest withMove() { T x; return std::move(x); } template Dest withMove(); // #1 template Dest withMove(); // #2 Saying that the std::move is pessimizing for #1 is not incorrect, but it's not useful, because removing the std::move would then pessimize #2. So the user can't really win. At the same time, disabling the warning just because we're in a template would be going too far, I still want to warn for template Dest withMove() { Dest x; return std::move(x); } because the std::move therein will be pessimizing for any instantiation. So I'm using the suppress_warning machinery to that effect. Problem: I had to add a new group to nowarn_spec_t, otherwise suppressing the -Wpessimizing-move warning would disable a whole bunch of other warnings, which we really don't want. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/89780 gcc/cp/ChangeLog: * pt.cc (tsubst_copy_and_build) : Maybe suppress -Wpessimizing-move. * typeck.cc (maybe_warn_pessimizing_move): Don't issue warnings if they are suppressed. (check_return_expr): Disable -Wpessimizing-move when returning a dependent expression. gcc/ChangeLog: * diagnostic-spec.cc (nowarn_spec_t::nowarn_spec_t): Handle OPT_Wpessimizing_move and OPT_Wredundant_move. * diagnostic-spec.h (nowarn_spec_t): Add NW_REDUNDANT enumerator. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/Wpessimizing-move3.C: Remove dg-warning. * g++.dg/cpp0x/Wpessimizing-move7.C: Likewise. * g++.dg/cpp0x/Wredundant-move2.C: Likewise. * g++.dg/cpp0x/Wpessimizing-move9.C: New test. --- gcc/cp/pt.cc | 3 + gcc/cp/typeck.cc | 20 +++-- gcc/diagnostic-spec.cc | 7 +- gcc/diagnostic-spec.h | 4 +- .../g++.dg/cpp0x/Wpessimizing-move3.C | 2 +- .../g++.dg/cpp0x/Wpessimizing-move7.C | 2 +- .../g++.dg/cpp0x/Wpessimizing-move9.C | 89 +++++++++++++++++++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C | 4 +- 8 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C base-commit: be58bf98e98bb431ed26ca8be84586075fe8be82 prerequisite-patch-id: f4862bc588ce5fed1d21016fecc4b61feb71eba5 prerequisite-patch-id: ce490c3c0b991fa7f27315387949c145c66223a4 diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 6c581fe0fb7..fe7e809fc2d 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -21215,6 +21215,9 @@ tsubst_copy_and_build (tree t, CALL_EXPR_ORDERED_ARGS (call) = ord; CALL_EXPR_REVERSE_ARGS (call) = rev; } + if (warning_suppressed_p (t, OPT_Wpessimizing_move)) + /* This also suppresses -Wredundant-move. */ + suppress_warning (ret, OPT_Wpessimizing_move); } RETURN (ret); diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 2650beb780e..70a5efc45de 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10430,9 +10430,10 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) if (can_do_rvo_p (arg, type)) { auto_diagnostic_group d; - if (warning_at (loc, OPT_Wpessimizing_move, - "moving a temporary object prevents copy " - "elision")) + 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 @@ -10443,14 +10444,16 @@ maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) else if (can_do_nrvo_p (arg, type)) { auto_diagnostic_group d; - if (warning_at (loc, OPT_Wpessimizing_move, - "moving a local object in a return statement " - "prevents copy elision")) + 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 @@ -10695,6 +10698,11 @@ check_return_expr (tree retval, bool *no_warning) /* We don't know if this is an lvalue or rvalue use, but either way we can mark it as read. */ mark_exp_read (retval); + /* Disable our std::move warnings when we're returning + a dependent expression (c++/89780). */ + if (retval && TREE_CODE (retval) == CALL_EXPR) + /* This also suppresses -Wredundant-move. */ + suppress_warning (retval, OPT_Wpessimizing_move); return retval; } diff --git a/gcc/diagnostic-spec.cc b/gcc/diagnostic-spec.cc index 4341ccfaae9..aece89619e7 100644 --- a/gcc/diagnostic-spec.cc +++ b/gcc/diagnostic-spec.cc @@ -96,7 +96,7 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt) case OPT_Winit_self: case OPT_Wuninitialized: case OPT_Wmaybe_uninitialized: - m_bits = NW_UNINIT; + m_bits = NW_UNINIT; break; case OPT_Wdangling_pointer_: @@ -105,6 +105,11 @@ nowarn_spec_t::nowarn_spec_t (opt_code opt) m_bits = NW_DANGLING; break; + case OPT_Wpessimizing_move: + case OPT_Wredundant_move: + m_bits = NW_REDUNDANT; + break; + default: /* A catchall group for everything else. */ m_bits = NW_OTHER; diff --git a/gcc/diagnostic-spec.h b/gcc/diagnostic-spec.h index 28e5e5ccc75..e5f1c127d4f 100644 --- a/gcc/diagnostic-spec.h +++ b/gcc/diagnostic-spec.h @@ -45,9 +45,11 @@ public: NW_DANGLING = 1 << 5, /* All other unclassified warnings. */ NW_OTHER = 1 << 6, + /* Warnings about redundant calls. */ + NW_REDUNDANT = 1 << 7, /* All groups of warnings. */ NW_ALL = (NW_ACCESS | NW_LEXICAL | NW_NONNULL - | NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_OTHER) + | NW_UNINIT | NW_VFLOW | NW_DANGLING | NW_REDUNDANT | NW_OTHER) }; nowarn_spec_t (): m_bits () { } diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C index a1af1230b68..c81f29a4ba6 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C @@ -39,7 +39,7 @@ Tp1 fn2 () { Tp2 t; - return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" } + return std::move (t); } template diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C index a17c7a87b7d..ebbcb2122a6 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C @@ -46,7 +46,7 @@ template T1 fn3 () { - return std::move (T2{}); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (T2{}); } void diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C new file mode 100644 index 00000000000..9519695eda7 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move9.C @@ -0,0 +1,89 @@ +// PR c++/89780 +// { dg-do compile { target c++11 } } +// { dg-options "-Wpessimizing-move -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 Dest { + Dest() = default; + Dest(Dest&&); + Dest(const Dest&); +}; +struct Source : Dest {}; + +template +Dest withMove() { + T x; + return std::move(x); +} + +template Dest withMove(); +template Dest withMove(); + +template +Dest bar () { + return std::move(T()); +} + +template Dest bar(); +template Dest bar(); + +template +Dest baz (T x) { + return std::move(x); +} + +void +call_baz () +{ + Dest d; + Source s; + baz (d); + baz (s); +} + +template +Dest foo () +{ + Dest d; + return std::move(d); // { dg-warning "moving a local object in a return statement prevents copy elision" } +} + +template Dest foo(); + +template +Dest qux () { + return std::move(Dest()); // { dg-warning "moving a temporary object prevents copy elision" } +} + +template Dest qux(); + +template +Dest qui (Dest x) { + return std::move(x); // { dg-warning "redundant move in return statement" } +} + +void +call_qui () +{ + Dest d; + qui (d); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C index f181afeeb84..6e0aa4b0277 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C @@ -37,14 +37,14 @@ template Tp1 fn2 (Tp2 t) { - return std::move (t); // { dg-warning "redundant move in return statement" } + return std::move (t); } template Tp1 fn3 (Tp2 t) { - return std::move (t); // { dg-warning "redundant move in return statement" } + return std::move (t); } int