From patchwork Tue Aug 2 23:04:47 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 363 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6a10:b5d6:b0:2b9:3548:2db5 with SMTP id v22csp3266916pxt; Tue, 2 Aug 2022 16:05:52 -0700 (PDT) X-Google-Smtp-Source: AGRyM1s8t8EROPrQ/EfPGfKtVq7vSTboFvhLsXxwwktWqWUBfpExZ/1pnU+UITcFD0jNpiYapkIY X-Received: by 2002:a17:907:2854:b0:72b:7daf:cc0d with SMTP id el20-20020a170907285400b0072b7dafcc0dmr17860290ejc.524.1659481542530; Tue, 02 Aug 2022 16:05:42 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1659481542; cv=none; d=google.com; s=arc-20160816; b=DDv2N72CMNrBEcZq2gglPdoP43DXjGVsWsFwR+Y88xhF40sm/vBE3pKVbU9SWwqAz9 +pcwZ1xdAz7a2qJqkt9A8G+UOdt2ipXFQeZPB2Aj69E8gU2edc0wWKgoCUgXixgUCkZo 61DLGRjDw7NWmT3jBIf2A3Kd6zM6KxkbT6cgxsyxFcPUrj5+VG9XRC9M8zG8yxUe4pYO ZXYNGJjoZAem1PFC/tYO6CvkVRfqttcCspZt6sdRjMtTi6bKn19CItkGiqaTiy3Zd6Ps Bvk6oG9D2K1ETfSgKiXK9yfTFCPrq6fqr5Ht2DU9/+9U/vv8iskfWPIyh1fPojdwV9SU qZKw== 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=nifALtKOrNXg+SppUQxC0ccUO/hSBCyvBEpss0UOIIE=; b=Y+umAEuSTnNLexPupDt71VBPNUPwnTWBLtPwOGS55TO0mT399oLZ/Fxd3pxD+QkQmi S0kJLzr+LFPfbKVutZ+bl4ZeHF9nnV4vI0JjKAWYoyPs0NYTNMpCC8CVTmpQflygU2lL wXm/7rswxjtgJ77JIQUMHt+oT0ThJfPpP1gX/HFhVxxPV/3UmCxf2azP55koBlGPrO8J rmaaFxWThXkBwceQRL8s/TRIgxb+Z5NSqal+Kvh88rZ4FYxeU8QkVzP/oUsft4Wt9Q1E kjVM87Owhk9Ic21gE4Sb1V7I73uVRs+xP1+iGZtLbVwY3O6PFe54X8S1BCrC8f191G8Y hULQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gcc.gnu.org header.s=default header.b="m11PqV/O"; 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 wg3-20020a17090705c300b0072aef2d3af1si16552227ejb.53.2022.08.02.16.05.41 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Aug 2022 16:05: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="m11PqV/O"; 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 1A8B43857C6D for ; Tue, 2 Aug 2022 23:05:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1A8B43857C6D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1659481541; bh=nifALtKOrNXg+SppUQxC0ccUO/hSBCyvBEpss0UOIIE=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=m11PqV/OXkaVqUYP7yU1+vqCVpAxill/QUh7RXzM59K/01byvPCSU7+LlfLLQEATV q9NphBiXPXAiT1W/juyoGT2htkxUaN25/G70bK4j0SrQLS6xSDyOv1LuTSvQJy6X35 rxSE8+aXUPvonY+P3JftL3H8rXEofHGyTuhbTYCg= 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.133.124]) by sourceware.org (Postfix) with ESMTPS id 50D913858C27 for ; Tue, 2 Aug 2022 23:04:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 50D913858C27 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-310-DdgnyJUgMOGUkd8nnwjOGQ-1; Tue, 02 Aug 2022 19:04:55 -0400 X-MC-Unique: DdgnyJUgMOGUkd8nnwjOGQ-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 90B71811E75 for ; Tue, 2 Aug 2022 23:04:55 +0000 (UTC) Received: from pdp-11.redhat.com (unknown [10.22.33.121]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6C3A3492C3B; Tue, 2 Aug 2022 23:04:55 +0000 (UTC) To: GCC Patches , Jason Merrill Subject: [PATCH] c++: Extend -Wpessimizing-move to other contexts Date: Tue, 2 Aug 2022 19:04:47 -0400 Message-Id: <20220802230447.721163-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.5 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 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?1740092517543074146?= X-GMAIL-MSGID: =?utf-8?q?1740092517543074146?= In my recent patch which enhanced -Wpessimizing-move so that it warns about class prvalues too I said that I'd like to extend it so that it warns in more contexts where a std::move can prevent copy elision, such as: T t = std::move(T()); T t(std::move(T())); T t{std::move(T())}; T t = {std::move(T())}; void foo (T); foo (std::move(T())); This patch does that by adding two maybe_warn_pessimizing_move calls. These must happen before we've converted the initializers otherwise the std::move will be buried in a TARGET_EXPR. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/106276 gcc/cp/ChangeLog: * call.cc (build_over_call): Call maybe_warn_pessimizing_move. * cp-tree.h (maybe_warn_pessimizing_move): Declare. * decl.cc (build_aggr_init_full_exprs): Call maybe_warn_pessimizing_move. * typeck.cc (maybe_warn_pessimizing_move): Handle TREE_LIST and CONSTRUCTOR. Add a bool parameter and use it. Adjust a diagnostic message. (check_return_expr): Adjust the call to maybe_warn_pessimizing_move. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/Wpessimizing-move7.C: Add dg-warning. * g++.dg/cpp0x/Wpessimizing-move8.C: New test. --- gcc/cp/call.cc | 5 +- gcc/cp/cp-tree.h | 1 + gcc/cp/decl.cc | 3 +- gcc/cp/typeck.cc | 58 ++++++++++++----- .../g++.dg/cpp0x/Wpessimizing-move7.C | 16 ++--- .../g++.dg/cpp0x/Wpessimizing-move8.C | 65 +++++++++++++++++++ 6 files changed, 120 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C base-commit: 502605a277d36cee1b0570982a16d97a43eace67 prerequisite-patch-id: f4862bc588ce5fed1d21016fecc4b61feb71eba5 diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 01a7be10077..370137ebd6d 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -9627,10 +9627,13 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) if (!conversion_warning) arg_complain &= ~tf_warning; + if (arg_complain & tf_warning) + maybe_warn_pessimizing_move (arg, type, /*return_p*/false); + val = convert_like_with_context (conv, arg, fn, i - is_method, arg_complain); val = convert_for_arg_passing (type, val, arg_complain); - + if (val == error_mark_node) return error_mark_node; else diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 3278b4114bd..5a8af22b509 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -8101,6 +8101,7 @@ extern tree finish_right_unary_fold_expr (tree, int); extern tree finish_binary_fold_expr (tree, tree, int); extern tree treat_lvalue_as_rvalue_p (tree, bool); extern bool decl_in_std_namespace_p (tree); +extern void maybe_warn_pessimizing_move (tree, tree, bool); /* in typeck2.cc */ extern void require_complete_eh_spec_types (tree, tree); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 70ad681467e..dc6853a7de1 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -7220,9 +7220,10 @@ check_array_initializer (tree decl, tree type, tree init) static tree build_aggr_init_full_exprs (tree decl, tree init, int flags) - { gcc_assert (stmts_are_full_exprs_p ()); + if (init) + maybe_warn_pessimizing_move (init, TREE_TYPE (decl), /*return_p*/false); return build_aggr_init (decl, init, flags, tf_warning_or_error); } diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 9500c4e2fe8..2650beb780e 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10368,17 +10368,17 @@ treat_lvalue_as_rvalue_p (tree expr, bool return_p) } } -/* Warn about wrong usage of std::move in a return statement. RETVAL - is the expression we are returning; FUNCTYPE is the type the function - is declared to return. */ +/* Warn about dubious usage of std::move (in a return statement, if RETURN_P + is true). EXPR is the std::move expression; TYPE is the type of the object + being initialized. */ -static void -maybe_warn_pessimizing_move (tree retval, tree functype) +void +maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) { if (!(warn_pessimizing_move || warn_redundant_move)) return; - location_t loc = cp_expr_loc_or_input_loc (retval); + const location_t loc = cp_expr_loc_or_input_loc (expr); /* C++98 doesn't know move. */ if (cxx_dialect < cxx11) @@ -10390,14 +10390,32 @@ maybe_warn_pessimizing_move (tree retval, tree functype) return; /* This is only interesting for class types. */ - if (!CLASS_TYPE_P (functype)) + if (!CLASS_TYPE_P (type)) return; + /* A a = std::move (A()); */ + if (TREE_CODE (expr) == TREE_LIST) + { + if (list_length (expr) == 1) + expr = TREE_VALUE (expr); + else + return; + } + /* A a = {std::move (A())}; + A a{std::move (A())}; */ + else if (TREE_CODE (expr) == CONSTRUCTOR) + { + if (CONSTRUCTOR_NELTS (expr) == 1) + expr = CONSTRUCTOR_ELT (expr, 0)->value; + else + return; + } + /* We're looking for *std::move ((T &) &arg). */ - if (REFERENCE_REF_P (retval) - && TREE_CODE (TREE_OPERAND (retval, 0)) == CALL_EXPR) + if (REFERENCE_REF_P (expr) + && TREE_CODE (TREE_OPERAND (expr, 0)) == CALL_EXPR) { - tree fn = TREE_OPERAND (retval, 0); + tree fn = TREE_OPERAND (expr, 0); if (is_std_move_p (fn)) { tree arg = CALL_EXPR_ARG (fn, 0); @@ -10409,20 +10427,24 @@ maybe_warn_pessimizing_move (tree retval, tree functype) return; arg = TREE_OPERAND (arg, 0); arg = convert_from_reference (arg); - /* Warn if we could do copy elision were it not for the move. */ - if (can_do_nrvo_p (arg, functype)) + if (can_do_rvo_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")) + "moving a temporary object prevents copy " + "elision")) inform (loc, "remove % call"); } - else if (can_do_rvo_p (arg, functype)) + /* 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)) { auto_diagnostic_group d; if (warning_at (loc, OPT_Wpessimizing_move, - "moving a temporary object in a return statement " + "moving a local object in a return statement " "prevents copy elision")) inform (loc, "remove % call"); } @@ -10433,7 +10455,7 @@ maybe_warn_pessimizing_move (tree retval, tree functype) { /* Make sure that overload resolution would actually succeed if we removed the std::move call. */ - tree t = convert_for_initialization (NULL_TREE, functype, + tree t = convert_for_initialization (NULL_TREE, type, moved, (LOOKUP_NORMAL | LOOKUP_ONLYCONVERTING @@ -10718,7 +10740,7 @@ check_return_expr (tree retval, bool *no_warning) return NULL_TREE; if (!named_return_value_okay_p) - maybe_warn_pessimizing_move (retval, functype); + maybe_warn_pessimizing_move (retval, functype, /*return_p*/true); /* Do any required conversions. */ if (bare_retval == result || DECL_CONSTRUCTOR_P (current_function_decl)) diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C index cd4eaa09aae..a17c7a87b7d 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C @@ -30,23 +30,23 @@ static A foo (); A fn1 () { - return std::move (A{}); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } - return std::move (A()); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } - return std::move (foo ()); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } + return std::move (A{}); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (foo ()); // { dg-warning "moving a temporary object prevents copy elision" } } B fn2 () { - return std::move (A()); - return std::move (A{}); - return std::move (foo ()); + return std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (A{}); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (foo ()); // { dg-warning "moving a temporary object prevents copy elision" } } template T1 fn3 () { - return std::move (T2{}); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } + return std::move (T2{}); // { dg-warning "moving a temporary object prevents copy elision" } } void @@ -58,6 +58,6 @@ do_fn3 () char take_buffer; struct label_text { - label_text take() { return std::move(label_text(&take_buffer)); } // { dg-warning "moving a temporary object in a return statement prevents copy elision" } + label_text take() { return std::move(label_text(&take_buffer)); } // { dg-warning "moving a temporary object prevents copy elision" } label_text(char *); }; diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C new file mode 100644 index 00000000000..51406c8f97f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C @@ -0,0 +1,65 @@ +// PR c++/106276 +// { dg-do compile { target c++11 } } +// { dg-options "-Wpessimizing-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 A { A(); A(const A&) = delete; A(A&&); }; +struct B { B(A); }; +struct X { }; + +void foo (A); +void bar (X); + +void +fn1 () +{ + A a1 = std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + A a2 = std::move (A{}); // { dg-warning "moving a temporary object prevents copy elision" } + A a3(std::move (A())); // { dg-warning "moving a temporary object prevents copy elision" } + A a4(std::move (A{})); // { dg-warning "moving a temporary object prevents copy elision" } + A a5{std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + A a6{std::move (A{})}; // { dg-warning "moving a temporary object prevents copy elision" } + A a7 = {std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + A a8 = {std::move (A{})}; // { dg-warning "moving a temporary object prevents copy elision" } + + B b1 = std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + B b2(std::move (A())); // { dg-warning "moving a temporary object prevents copy elision" } + B b3{std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + B b4 = {std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + + X x1 = std::move (X()); // { dg-warning "moving a temporary object prevents copy elision" } + X x2 = std::move (X{}); // { dg-warning "moving a temporary object prevents copy elision" } + X x3(std::move (X())); // { dg-warning "moving a temporary object prevents copy elision" } + X x4(std::move (X{})); // { dg-warning "moving a temporary object prevents copy elision" } + X x5{std::move (X())}; // { dg-warning "moving a temporary object prevents copy elision" } + X x6{std::move (X{})}; // { dg-warning "moving a temporary object prevents copy elision" } + X x7 = {std::move (X())}; // { dg-warning "moving a temporary object prevents copy elision" } + X x8 = {std::move (X{})}; // { dg-warning "moving a temporary object prevents copy elision" } + + foo (std::move (A())); // { dg-warning "moving a temporary object prevents copy elision" } + foo (std::move (A{})); // { dg-warning "moving a temporary object prevents copy elision" } + bar (std::move (X())); // { dg-warning "moving a temporary object prevents copy elision" } + bar (std::move (X{})); // { dg-warning "moving a temporary object prevents copy elision" } + + foo (std::move (a1)); + bar (std::move (x1)); +}