llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) <details> <summary>Changes</summary> Fixes https://github.com/llvm/llvm-project/issues/132419. --- Full diff: https://github.com/llvm/llvm-project/pull/190541.diff 5 Files Affected: - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp (+30-10) - (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h (+1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+8-3) - (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst (+12) - (added) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-cxx20.cpp (+103) ``````````diff diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp index 5aa6cbeff53d8..2934e97143d4e 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp @@ -54,6 +54,21 @@ void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) { hasAncestor(expr(hasUnevaluatedContext()))))) .bind("move-call"); + // P1825R0 (C++20): returning a named rvalue reference parameter by name + // performs an implicit move, which is equivalent to ``std::move(param)`` + const StatementMatcher ImplicitMoveReturnMatcher = traverse( + TK_IgnoreUnlessSpelledInSource, + returnStmt(hasReturnValue(ignoringParens( + declRefExpr(to(equalsBoundNode("param"))).bind("ref")))) + .bind("implicit-move-return")); + + const bool EnableImplicitMove = + AllowImplicitMove && getLangOpts().CPlusPlus20; + + const StatementMatcher UsageMatcher = stmt( + anyOf(MoveCallMatcher, EnableImplicitMove ? ImplicitMoveReturnMatcher + : stmt(unless(anything())))); + Finder->addMatcher( parmVarDecl( hasType(type(rValueReferenceType())), parmVarDecl().bind("param"), @@ -68,10 +83,10 @@ void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) { unless(cxxConstructorDecl(isMoveConstructor())), unless(cxxMethodDecl(isMoveAssignmentOperator())), ToParam, anyOf(cxxConstructorDecl( - optionally(hasDescendant(MoveCallMatcher))), - functionDecl(unless(cxxConstructorDecl()), - optionally(hasBody( - hasDescendant(MoveCallMatcher)))))) + optionally(hasDescendant(UsageMatcher))), + functionDecl( + unless(cxxConstructorDecl()), + optionally(hasBody(hasDescendant(UsageMatcher)))))) .bind("func"))), this); } @@ -108,12 +123,15 @@ void RvalueReferenceParamNotMovedCheck::check( } const auto *MoveCall = Result.Nodes.getNodeAs<CallExpr>("move-call"); - if (!MoveCall) { - diag(Param->getLocation(), - "rvalue reference parameter %0 is never moved from " - "inside the function body") - << Param; - } + const auto *ImplicitMoveReturn = + Result.Nodes.getNodeAs<ReturnStmt>("implicit-move-return"); + if (MoveCall || ImplicitMoveReturn) + return; + + diag(Param->getLocation(), + "rvalue reference parameter %0 is never moved from " + "inside the function body") + << Param; } RvalueReferenceParamNotMovedCheck::RvalueReferenceParamNotMovedCheck( @@ -123,6 +141,7 @@ RvalueReferenceParamNotMovedCheck::RvalueReferenceParamNotMovedCheck( IgnoreUnnamedParams(Options.get("IgnoreUnnamedParams", false)), IgnoreNonDeducedTemplateTypes( Options.get("IgnoreNonDeducedTemplateTypes", false)), + AllowImplicitMove(Options.get("AllowImplicitMove", false)), MoveFunction(Options.get("MoveFunction", "::std::move")) {} void RvalueReferenceParamNotMovedCheck::storeOptions( @@ -131,6 +150,7 @@ void RvalueReferenceParamNotMovedCheck::storeOptions( Options.store(Opts, "IgnoreUnnamedParams", IgnoreUnnamedParams); Options.store(Opts, "IgnoreNonDeducedTemplateTypes", IgnoreNonDeducedTemplateTypes); + Options.store(Opts, "AllowImplicitMove", AllowImplicitMove); Options.store(Opts, "MoveFunction", MoveFunction); } diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h index 9fec58fb86036..360b1fc76a830 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h @@ -32,6 +32,7 @@ class RvalueReferenceParamNotMovedCheck : public ClangTidyCheck { const bool AllowPartialMove; const bool IgnoreUnnamedParams; const bool IgnoreNonDeducedTemplateTypes; + const bool AllowImplicitMove; const StringRef MoveFunction; }; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 69dc5b9633398..4d76c6e04bc48 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -290,9 +290,14 @@ Changes in existing checks detail. - Improved :doc:`cppcoreguidelines-rvalue-reference-param-not-moved - <clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved>` check - by fixing a false positive on implicitly generated functions such as - inherited constructors. + <clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved>` check: + + - Fixed a false positive on implicitly generated functions such as + inherited constructors. + + - Added `AllowImplicitMove` option. When enabled and compiling as C++20 + or later, the check don't warn when an rvalue reference parameter is returned + without an explicit ``std::move``. - Improved :doc:`llvm-use-ranges <clang-tidy/checks/llvm/use-ranges>` check by adding support for the following diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst index 2fea9f16b3bb0..2698b0ac40c10 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst @@ -79,6 +79,18 @@ Options T other = std::forward<T>(t); } +.. option:: AllowImplicitMove + + If set to `true`, the check recognizes C++20 implicit move ``return param;`` + where ``param`` is a rvalue reference parameter. This option only + takes effect when compiled with C++20 or later. Default is `false`. + + .. code-block:: c++ + + A f(A&& a) { + return a; // no warning with AllowImplicitMove = true + } + .. option:: MoveFunction Specify the function used for moving. Default is `::std::move`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-cxx20.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-cxx20.cpp new file mode 100644 index 0000000000000..7b485887f9446 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-cxx20.cpp @@ -0,0 +1,103 @@ +// RUN: %check_clang_tidy -check-suffix=ALLOW-CXX20 -std=c++20-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \ +// RUN: -config="{CheckOptions: {cppcoreguidelines-rvalue-reference-param-not-moved.AllowImplicitMove: true}}" +// RUN: %check_clang_tidy -check-suffix=ALLOW-PRE-CXX20 -std=c++11,c++14,c++17 %s cppcoreguidelines-rvalue-reference-param-not-moved %t -- \ +// RUN: -config="{CheckOptions: {cppcoreguidelines-rvalue-reference-param-not-moved.AllowImplicitMove: true}}" +// RUN: %check_clang_tidy -check-suffix=DEFAULT -std=c++11-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t + +#include <utility> + +struct S { + S(); + S(const S&); + S(S&&) noexcept; + S& operator=(const S&); + S& operator=(S&&) noexcept; +}; + +int intImplicitReturn(int&& x) { + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-1]]:29: warning: rvalue reference parameter 'x' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:29: warning: rvalue reference parameter 'x' is never moved from inside the function body + return x; +} + +S classImplicitReturn(S&& s) { + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-1]]:27: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:27: warning: rvalue reference parameter 's' is never moved from inside the function body + return s; +} + +S classImplicitReturnParens(S&& s) { + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-1]]:33: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:33: warning: rvalue reference parameter 's' is never moved from inside the function body + return (s); +} + +S explicitMoveReturn(S&& s) { + return std::move(s); +} + +S notMovedOrReturned(S&& s) { + // CHECK-MESSAGES-ALLOW-CXX20: :[[@LINE-1]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-2]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-3]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + S copy = s; + return copy; +} + +S SomePathsReturnParam(S&& s, bool cond) { + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-1]]:28: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:28: warning: rvalue reference parameter 's' is never moved from inside the function body + if (cond) + return s; + return S(); +} + +S NoPathReturnsParam(S&& s, bool cond) { + // CHECK-MESSAGES-ALLOW-CXX20: :[[@LINE-1]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-2]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-3]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + if (cond) + return S(); + S copy = s; + return copy; +} + +S TwoParamsBothMoved(S&& a, S&& b, bool cond) { + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-1]]:26: warning: rvalue reference parameter 'a' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-2]]:33: warning: rvalue reference parameter 'b' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-3]]:26: warning: rvalue reference parameter 'a' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-4]]:33: warning: rvalue reference parameter 'b' is never moved from inside the function body + if (cond) + return a; + return b; +} + +S TwoParamsOnlyOneMoved(S&& a, S&& b) { + // CHECK-MESSAGES-ALLOW-CXX20: :[[@LINE-1]]:36: warning: rvalue reference parameter 'b' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-2]]:29: warning: rvalue reference parameter 'a' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-3]]:36: warning: rvalue reference parameter 'b' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-4]]:29: warning: rvalue reference parameter 'a' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-5]]:36: warning: rvalue reference parameter 'b' is never moved from inside the function body + (void)b; + return a; +} + +struct A { + A(); + A(const A&, int); + A(A&&) noexcept; +}; + +A explicitCtorWithParamArg(A&& param) { + // CHECK-MESSAGES-ALLOW-CXX20: :[[@LINE-1]]:32: warning: rvalue reference parameter 'param' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-2]]:32: warning: rvalue reference parameter 'param' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-3]]:32: warning: rvalue reference parameter 'param' is never moved from inside the function body + return A(param, 10); +} + +S explicitCtorCall(S&& s) { + // CHECK-MESSAGES-ALLOW-CXX20: :[[@LINE-1]]:24: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-ALLOW-PRE-CXX20: :[[@LINE-2]]:24: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-3]]:24: warning: rvalue reference parameter 's' is never moved from inside the function body + return S(s); +} `````````` </details> https://github.com/llvm/llvm-project/pull/190541 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
