diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp index 5aa6cbeff53d8..44b1eb3fa169e 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp @@ -54,6 +54,18 @@ void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) { hasAncestor(expr(hasUnevaluatedContext()))))) .bind("move-call"); + // P1825R0: 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 StatementMatcher UsageMatcher = stmt( + anyOf(MoveCallMatcher, AllowImplicitMove ? ImplicitMoveReturnMatcher + : stmt(unless(anything())))); + Finder->addMatcher( parmVarDecl( hasType(type(rValueReferenceType())), parmVarDecl().bind("param"), @@ -68,10 +80,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 +120,15 @@ void RvalueReferenceParamNotMovedCheck::check( } const auto *MoveCall = Result.Nodes.getNodeAs("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("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 +138,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 +147,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..61740202fa3f6 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -290,9 +290,13 @@ Changes in existing checks detail. - Improved :doc:`cppcoreguidelines-rvalue-reference-param-not-moved - ` check - by fixing a false positive on implicitly generated functions such as - inherited constructors. + ` check: + + - Fixed a false positive on implicitly generated functions such as + inherited constructors. + + - Added `AllowImplicitMove` option to not warn when an rvalue reference + parameter is returned without an explicit ``std::move``. - Improved :doc:`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..ee60f6c0179a9 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 @@ -51,6 +51,17 @@ Options pair other = std::move(p); } +.. option:: AllowImplicitMove + + If set to `true`, the check recognizes implicit move ``return param;`` + where ``param`` is a rvalue reference parameter. Default is `false`. + + .. code-block:: c++ + + A f(A&& a) { + return a; // no warning with AllowImplicitMove = true + } + .. option:: IgnoreUnnamedParams If set to `true`, the check ignores unnamed rvalue reference parameters. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-allow-implicit.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-allow-implicit.cpp new file mode 100644 index 0000000000000..190a6e0222e86 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved-allow-implicit.cpp @@ -0,0 +1,89 @@ +// RUN: %check_clang_tidy -check-suffix=ALLOW -std=c++11-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=DEFAULT -std=c++11-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t + +#include + +struct S { + S(); + S(const S&); + S(S&&) noexcept; + S& operator=(const S&); + S& operator=(S&&) noexcept; +}; + +int intImplicitReturn(int&& x) { + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:29: warning: rvalue reference parameter 'x' is never moved from inside the function body + return x; +} + +S classImplicitReturn(S&& s) { + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:27: warning: rvalue reference parameter 's' is never moved from inside the function body + return s; +} + +S classImplicitReturnParens(S&& s) { + // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]: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: :[[@LINE-1]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]: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-DEFAULT: :[[@LINE-1]]: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: :[[@LINE-1]]:26: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]: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-DEFAULT: :[[@LINE-1]]:26: warning: rvalue reference parameter 'a' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]: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: :[[@LINE-1]]:36: warning: rvalue reference parameter 'b' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:29: warning: rvalue reference parameter 'a' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-3]]: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: :[[@LINE-1]]:32: warning: rvalue reference parameter 'param' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]: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: :[[@LINE-1]]:24: warning: rvalue reference parameter 's' is never moved from inside the function body + // CHECK-MESSAGES-DEFAULT: :[[@LINE-2]]:24: warning: rvalue reference parameter 's' is never moved from inside the function body + return S(s); +}