From patchwork Tue Aug 9 16:37:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 56627 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B7AED3856DFB for ; Tue, 9 Aug 2022 16:37:55 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B7AED3856DFB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1660063075; bh=55n8grzhtBVz0W4BMzXmS9ycZ1yV7a94ABp7Ee3HGf8=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=oGdciixwpWY876+DrxX37g2ARVMmHCPxJPnH0vGEGrFgOzHMUF0ipD32OUOoR6SJw EKObpVjL9NBOAzYyKhDGaSElVxOAj4H1tSjx+jkptBYnSS6Z/jtvTHvQZcutKNFzz2 6ZaQXKFiC0HPhwD+s/KHclqwwVNXNCytjxtEqBto= 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 ECC2D3856DC6 for ; Tue, 9 Aug 2022 16:37:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org ECC2D3856DC6 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-484-57_Tm83eOPyfkU5mwKtiVA-1; Tue, 09 Aug 2022 12:37:25 -0400 X-MC-Unique: 57_Tm83eOPyfkU5mwKtiVA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E15EA1019C8D for ; Tue, 9 Aug 2022 16:37:24 +0000 (UTC) Received: from pdp-11.redhat.com (unknown [10.22.9.34]) by smtp.corp.redhat.com (Postfix) with ESMTP id C2B081415116; Tue, 9 Aug 2022 16:37:23 +0000 (UTC) To: GCC Patches , Jason Merrill Subject: [PATCH] c++: Implement -Wself-move warning [PR81159] Date: Tue, 9 Aug 2022 12:37:19 -0400 Message-Id: <20220809163719.381319-1-polacek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-24.8 required=5.0 tests=BAYES_00, DKIM_INVALID, DKIM_SIGNED, GIT_PATCH_0, KAM_DMARC_NONE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_NONE, 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 Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" About 5 years ago we got a request to implement -Wself-move, which warns about useless moves like this: int x; x = std::move (x); This patch implements that warning. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/81159 gcc/c-family/ChangeLog: * c.opt (Wself-move): New option. gcc/cp/ChangeLog: * typeck.cc (maybe_warn_self_move): New. (cp_build_modify_expr): Call maybe_warn_self_move. gcc/ChangeLog: * doc/invoke.texi: Document -Wself-move. gcc/testsuite/ChangeLog: * g++.dg/warn/Wself-move1.C: New test. --- gcc/c-family/c.opt | 4 ++ gcc/cp/typeck.cc | 48 +++++++++++++- gcc/doc/invoke.texi | 23 ++++++- gcc/testsuite/g++.dg/warn/Wself-move1.C | 87 +++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/warn/Wself-move1.C base-commit: 9385cd9c74cf6662f43038aafe4d2467899f322e diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 44e1a60ce24..a098ae1830d 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1229,6 +1229,10 @@ Wselector ObjC ObjC++ Var(warn_selector) Warning Warn if a selector has multiple methods. +Wself-move +C++ ObjC++ Var(warn_self_move) Warning LangEnabledBy(C++ ObjC++, Wall) +Warn when a value is moved to itself with std::move. + Wsequence-point C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about possible violations of sequence point rules. diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 6e4f23af982..f05913c0fac 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -8897,7 +8897,51 @@ cp_build_c_cast (location_t loc, tree type, tree expr, return error_mark_node; } - + +/* Warn when a value is moved to itself with std::move. LHS is the target, + RHS may be the std::move call, and LOC is the location of the whole + assignment. */ + +static void +maybe_warn_self_move (location_t loc, tree lhs, tree rhs) +{ + if (!warn_self_move) + return; + + /* C++98 doesn't know move. */ + if (cxx_dialect < cxx11) + return; + + if (processing_template_decl) + return; + + /* We're looking for *std::move ((T &) &arg), or + *std::move ((T &) (T *) r) if the argument it a reference. */ + if (!REFERENCE_REF_P (rhs) + || TREE_CODE (TREE_OPERAND (rhs, 0)) != CALL_EXPR) + return; + tree fn = TREE_OPERAND (rhs, 0); + if (!is_std_move_p (fn)) + return; + tree arg = CALL_EXPR_ARG (fn, 0); + if (TREE_CODE (arg) != NOP_EXPR) + return; + /* Strip the (T &). */ + arg = TREE_OPERAND (arg, 0); + /* Strip the (T *) or &. */ + arg = TREE_OPERAND (arg, 0); + arg = convert_from_reference (arg); + /* So that we catch (i) = std::move (i);. */ + lhs = maybe_undo_parenthesized_ref (lhs); + STRIP_ANY_LOCATION_WRAPPER (lhs); + if (cp_tree_equal (lhs, arg)) + { + auto_diagnostic_group d; + if (warning_at (loc, OPT_Wself_move, "moving a variable to itself")) + inform (loc, "remove % call"); + } +} + /* For use from the C common bits. */ tree build_modify_expr (location_t location, @@ -9101,6 +9145,8 @@ cp_build_modify_expr (location_t loc, tree lhs, enum tree_code modifycode, if (modifycode == NOP_EXPR) { + maybe_warn_self_move (loc, lhs, rhs); + if (c_dialect_objc ()) { result = objc_maybe_build_modify_expr (lhs, rhs); diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index f3e9429b2ca..28cf36b94c6 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -264,7 +264,7 @@ in the following sections. -Wreorder -Wregister @gol -Wstrict-null-sentinel -Wno-subobject-linkage -Wtemplates @gol -Wno-non-template-friend -Wold-style-cast @gol --Woverloaded-virtual -Wno-pmf-conversions -Wsign-promo @gol +-Woverloaded-virtual -Wno-pmf-conversions -Wself-move -Wsign-promo @gol -Wsized-deallocation -Wsuggest-final-methods @gol -Wsuggest-final-types -Wsuggest-override @gol -Wno-terminate -Wuseless-cast -Wno-vexing-parse @gol @@ -5841,6 +5841,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}. -Wreorder @gol -Wrestrict @gol -Wreturn-type @gol +-Wself-move @r{(only for C++)} @gol -Wsequence-point @gol -Wsign-compare @r{(only in C++)} @gol -Wsizeof-array-div @gol @@ -6826,6 +6827,26 @@ of a declaration: This warning is enabled by @option{-Wall}. +@item -Wno-self-move @r{(C++ and Objective-C++ only)} +@opindex Wself-move +@opindex Wno-self-move +This warning warns when a value is moved to itself with @code{std::move}. +Such a @code{std::move} has no effect. + +@smallexample +struct T @{ +@dots{} +@}; +void fn() +@{ + T t; + @dots{} + t = std::move (t); +@} +@end smallexample + +This warning is enabled by @option{-Wall}. + @item -Wsequence-point @opindex Wsequence-point @opindex Wno-sequence-point diff --git a/gcc/testsuite/g++.dg/warn/Wself-move1.C b/gcc/testsuite/g++.dg/warn/Wself-move1.C new file mode 100644 index 00000000000..4a6dbad4533 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wself-move1.C @@ -0,0 +1,87 @@ +// PR c++/81159 +// { dg-do compile { target c++11 } } +// { dg-options "-Wself-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); } +} + +int g; + +struct S { + int x; + S(S&& o) { + x = std::move (x); // { dg-warning "moving a variable to itself" } + x = std::move (o.x); + o.x = std::move (x); + o.x = std::move (o.x); // { dg-warning "moving a variable to itself" } + } + void foo (int x) { + x = std::move (x); // { dg-warning "moving a variable to itself" } + } +}; + +struct X { + int x; + X(int x) : x(std::move (x)) { } +}; + +struct A {}; +struct B { A a; }; +struct C { C(); ~C(); }; +struct D { D(); D(const D&); D(D&&); D& operator=(const D&); }; + +void +test () +{ + int i = 42; + i = std::move (i); // { dg-warning "moving a variable to itself" } + (i) = std::move (i); // { dg-warning "moving a variable to itself" } + + g = std::move (g); // { dg-warning "moving a variable to itself" } + (g) = std::move (g); // { dg-warning "moving a variable to itself" } + + A a; + a = std::move (a); // { dg-warning "moving a variable to itself" } + + B b; + b = std::move (b); // { dg-warning "moving a variable to itself" } + b.a = std::move (b.a); // { dg-warning "moving a variable to itself" } + + C c; + c = std::move (c); // { dg-warning "moving a variable to itself" } + D d; + d = std::move (d); // { dg-warning "moving a variable to itself" } +} + +template +void ttest () +{ + T t; + t = std::move (t); // { dg-warning "moving a variable to itself" } +} + +template void ttest(); + +void +testref (int &r, int &&rr) +{ + r = std::move (r); // { dg-warning "moving a variable to itself" } + rr = std::move (rr); // { dg-warning "moving a variable to itself" } +}