ccotter updated this revision to Diff 489174.
ccotter added a comment.
- Match unresolved calls to move
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141569/new/
https://reviews.llvm.org/D141569
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/rvalue-reference-param-not-moved.cpp
@@ -0,0 +1,230 @@
+// RUN: %check_clang_tidy -std=c++14-or-later %s cppcoreguidelines-rvalue-reference-param-not-moved %t
+
+// NOLINTBEGIN
+namespace std {
+template <typename>
+struct remove_reference;
+
+template <typename _Tp>
+struct remove_reference {
+ typedef _Tp type;
+};
+
+template <typename _Tp>
+constexpr typename std::remove_reference<_Tp>::type &&move(_Tp &&__t) noexcept;
+
+template <typename _Tp>
+constexpr _Tp &&
+forward(typename remove_reference<_Tp>::type &__t) noexcept;
+
+}
+// NOLINTEND
+
+struct Obj {
+ Obj();
+ Obj(const Obj&);
+ Obj& operator=(const Obj&);
+ Obj(Obj&&);
+ Obj& operator=(Obj&&);
+ void member() const;
+};
+
+void consumes_object(Obj);
+
+void never_moves_param(Obj&& o) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+ o.member();
+}
+
+void copies_object(Obj&& o) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+ Obj copy = o;
+}
+
+template <typename T>
+void never_moves_param_template(Obj&& o, T t) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+ o.member();
+}
+
+void never_moves_params(Obj&& o1, Obj&& o2) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+ // CHECK-MESSAGES: :[[@LINE-2]]:35: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void never_moves_some_params(Obj&& o1, Obj&& o2) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+ Obj other{std::move(o2)};
+}
+
+void never_moves_mixed(Obj o1, Obj&& o2) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value(Obj&& o) {
+ auto f = [o]() {
+ consumes_object(std::move(o));
+ };
+ // CHECK-MESSAGES: :[[@LINE-4]]:41: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+void lambda_captures_parameter_as_value_nested(Obj&& o) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:48: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+ auto f = [&o]() {
+ auto f_nested = [o]() {
+ consumes_object(std::move(o));
+ };
+ };
+ auto f2 = [o]() {
+ auto f_nested = [&o]() {
+ consumes_object(std::move(o));
+ };
+ };
+ auto f3 = [o]() {
+ auto f_nested = [&o]() {
+ auto f_nested_inner = [&o]() {
+ consumes_object(std::move(o));
+ };
+ };
+ };
+ auto f4 = [&o]() {
+ auto f_nested = [&o]() {
+ auto f_nested_inner = [o]() {
+ consumes_object(std::move(o));
+ };
+ };
+ };
+}
+
+void misc_lambda_checks() {
+ auto never_moves = [](Obj&& o1) {
+ Obj other{o1};
+ };
+ // CHECK-MESSAGES: :[[@LINE-3]]:25: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+
+ auto never_moves_with_auto_param = [](Obj&& o1, auto& v) {
+ Obj other{o1};
+ };
+ // CHECK-MESSAGES: :[[@LINE-3]]:41: warning: rvalue reference parameter is never moved from inside the function body [cppcoreguidelines-rvalue-reference-param-not-moved]
+}
+
+// Negative tests - below functions do not violate the guideline
+
+template <typename T>
+void forwarding_ref(T&& t) {
+ t.member();
+}
+
+template <typename T>
+void forwarding_ref_forwarded(T&& t) {
+ forwarding_ref(std::forward<T>(t));
+}
+
+template <typename... Ts>
+void type_pack(Ts&&... ts) {
+ forwarding_ref(std::forward<Ts>(ts)...);
+}
+
+void good1(Obj&& o) {
+ Obj moved = std::move(o);
+ moved.member();
+}
+
+void good2(Obj& o) {
+ o.member();
+}
+
+void good3(Obj o) {
+ o.member();
+}
+
+void good4(const Obj& o) {
+ o.member();
+}
+
+void lambda_captures_parameter_as_reference(Obj&& o) {
+ auto f = [&o]() {
+ consumes_object(std::move(o));
+ };
+}
+
+void lambda_captures_parameter_as_reference_nested(Obj&& o) {
+ auto f = [&o]() {
+ auto f_nested = [&o]() {
+ auto f_nested2 = [&o]() {
+ consumes_object(std::move(o));
+ };
+ };
+ };
+}
+
+void lambda_captures_parameter_generalized(Obj&& o) {
+ auto f = [o = std::move(o)]() {
+ consumes_object(std::move(o));
+ };
+}
+
+void negative_lambda_checks() {
+ auto never_moves_nested = [](Obj&& o1) {
+ auto nested = [&]() {
+ Obj other{std::move(o1)};
+ };
+ };
+
+ auto auto_lvalue_ref_param = [](auto& o1) {
+ Obj other{o1};
+ };
+
+ auto auto_forwarding_ref_param = [](auto&& o1) {
+ Obj other{o1};
+ };
+
+ auto does_move = [](Obj&& o1) {
+ Obj other{std::move(o1)};
+ };
+
+ auto does_move_auto_rvalue_ref_param = [](auto&& o1) {
+ Obj other{std::forward(o1)};
+ };
+
+ auto not_rvalue_ref = [](Obj& o1) {
+ Obj other{std::move(o1)};
+ };
+
+ Obj local;
+ auto captures = [local]() {
+ };
+}
+
+void useless_move(Obj&& o) {
+ // FIXME - The object is not actually moved from - this should probably be
+ // flagged by *some* check. Which one?
+ std::move(o);
+}
+
+template <typename>
+class TemplatedClass;
+
+template <typename T>
+void unresolved_lookup(TemplatedClass<T>&& o) {
+ TemplatedClass<T> moved = std::move(o);
+}
+
+struct DefinesMove {
+ DefinesMove(DefinesMove&& rhs) : o(std::move(rhs.o)) {
+ }
+ DefinesMove& operator=(DefinesMove&& rhs) {
+ if (this != &rhs) {
+ o = std::move(rhs.o);
+ }
+ return *this;
+ }
+ Obj o;
+};
+
+struct AnotherObj {
+ AnotherObj(Obj&& o) : o(std::move(o)) {}
+ AnotherObj(Obj&& o, int) { o = std::move(o); }
+ Obj o;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -200,6 +200,7 @@
`cppcoreguidelines-pro-type-static-cast-downcast <cppcoreguidelines/pro-type-static-cast-downcast.html>`_, "Yes"
`cppcoreguidelines-pro-type-union-access <cppcoreguidelines/pro-type-union-access.html>`_,
`cppcoreguidelines-pro-type-vararg <cppcoreguidelines/pro-type-vararg.html>`_,
+ `cppcoreguidelines-rvalue-reference-param-not-moved <cppcoreguidelines/rvalue-reference-param-not-moved.html>`_,
`cppcoreguidelines-slicing <cppcoreguidelines/slicing.html>`_,
`cppcoreguidelines-special-member-functions <cppcoreguidelines/special-member-functions.html>`_,
`cppcoreguidelines-virtual-class-destructor <cppcoreguidelines/virtual-class-destructor.html>`_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - cppcoreguidelines-rvalue-reference-param-not-moved
+
+cppcoreguidelines-rvalue-reference-param-not-moved
+==================================================
+
+Warns when an rvalue reference function parameter is never moved within
+the function body.
+
+Rvalue reference parameters indicate a parameter that should be moved with
+``std::move`` from within the function body. Any such parameter that is
+never moved is confusing and potentially indicative of a buggy program.
+
+Examples:
+
+.. code-block:: c++
+
+ void logic(std::string&& Input) {
+ std::string Copy(Input); // Oops - forgot to std::move
+ }
+
+This check implements
+`CppCoreGuideline F.18 <http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f18-for-will-move-from-parameters-pass-by-x-and-stdmove-the-parameter>`_.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -126,6 +126,12 @@
Warns on coroutines that accept reference parameters.
+- New :doc:`cppcoreguidelines-rvalue-reference-param-not-moved
+ <clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved>` check.
+
+ Warns when an rvalue reference function parameter is never moved within
+ the function body.
+
- New :doc:`misc-use-anonymous-namespace
<clang-tidy/checks/misc/use-anonymous-namespace>` check.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.h
@@ -0,0 +1,38 @@
+//===--- RvalueReferenceParamNotMovedCheck.h - clang-tidy -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// Warns when an rvalue reference function parameter is never moved within
+/// the function body. This check implements CppCoreGuideline F.18.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/rvalue-reference-param-not-moved.html
+class RvalueReferenceParamNotMovedCheck : public ClangTidyCheck {
+public:
+ RvalueReferenceParamNotMovedCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus11;
+ }
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_RVALUEREFERENCEPARAMNOTMOVEDCHECK_H
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/RvalueReferenceParamNotMovedCheck.cpp
@@ -0,0 +1,146 @@
+//===--- RvalueReferenceParamNotMovedCheck.cpp - clang-tidy ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "RvalueReferenceParamNotMovedCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+#include <queue>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void RvalueReferenceParamNotMovedCheck::registerMatchers(MatchFinder *Finder) {
+ auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
+ Finder->addMatcher(
+ parmVarDecl(allOf(
+ parmVarDecl(hasType(type(rValueReferenceType()))).bind("param"),
+ parmVarDecl(
+ equalsBoundNode("param"),
+ unless(hasType(
+ qualType(references(templateTypeParmType(
+ hasDeclaration(templateTypeParmDecl()))),
+ unless(references(qualType(isConstQualified())))))),
+ anyOf(hasAncestor(compoundStmt(hasParent(
+ lambdaExpr(has(cxxRecordDecl(has(cxxMethodDecl(
+ ToParam, hasName("operator()"))))))
+ .bind("containing-lambda")))),
+ hasAncestor(cxxConstructorDecl(isDefinition(), ToParam,
+ unless(isMoveConstructor()),
+ isDefinition(), ToParam)
+ .bind("containing-ctor")),
+ hasAncestor(
+ functionDecl(
+ isDefinition(), ToParam,
+ unless(cxxConstructorDecl(isMoveConstructor())),
+ unless(cxxMethodDecl(isMoveAssignmentOperator())))
+ .bind("containing-func")))))),
+ this);
+}
+
+static bool isValueCapturedByAnyLambda(ASTContext &Context,
+ DynTypedNode ContainingNode,
+ const Expr *StartingExpr,
+ const Decl *Param) {
+ std::queue<DynTypedNode> ToVisit;
+ ToVisit.push(DynTypedNode::create(*StartingExpr));
+ while (!ToVisit.empty()) {
+ DynTypedNode At = ToVisit.front();
+ ToVisit.pop();
+
+ if (At == ContainingNode) {
+ return false;
+ }
+ if (const auto *Lambda = At.get<LambdaExpr>()) {
+ bool ParamIsValueCaptured =
+ std::find_if(Lambda->capture_begin(), Lambda->capture_end(),
+ [&](const LambdaCapture &Capture) {
+ return Capture.getCapturedVar() == Param &&
+ Capture.capturesVariable() &&
+ Capture.getCaptureKind() == LCK_ByCopy;
+ }) != Lambda->capture_end();
+ if (ParamIsValueCaptured) {
+ return true;
+ }
+ }
+
+ DynTypedNodeList Parents = Context.getParents(At);
+ std::for_each(Parents.begin(), Parents.end(),
+ [&](const DynTypedNode &Node) { ToVisit.push(Node); });
+ if (Parents.empty()) {
+ return false;
+ }
+ }
+ return false;
+}
+
+void RvalueReferenceParamNotMovedCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ const auto *Param = Result.Nodes.getNodeAs<ParmVarDecl>("param");
+ const auto *ContainingLambda =
+ Result.Nodes.getNodeAs<LambdaExpr>("containing-lambda");
+ const auto *ContainingCtor =
+ Result.Nodes.getNodeAs<FunctionDecl>("containing-ctor");
+ const auto *ContainingFunc =
+ Result.Nodes.getNodeAs<FunctionDecl>("containing-func");
+
+ StatementMatcher MoveCallMatcher =
+ callExpr(anyOf(callee(functionDecl(hasName("::std::move"))),
+ callee(unresolvedLookupExpr(hasAnyDeclaration(namedDecl(
+ hasUnderlyingDecl(hasName("::std::move"))))))),
+ argumentCountIs(1),
+ hasArgument(0, declRefExpr(to(equalsNode(Param))).bind("ref")));
+
+ DynTypedNode ContainingNode;
+
+ SmallVector<BoundNodes, 1> Matches;
+ if (ContainingLambda) {
+ if (!ContainingLambda->getBody())
+ return;
+ ContainingNode = DynTypedNode::create(*ContainingLambda);
+ Matches = match(findAll(MoveCallMatcher), *ContainingLambda->getBody(),
+ *Result.Context);
+ } else if (ContainingCtor) {
+ ContainingNode = DynTypedNode::create(*ContainingCtor);
+ Matches = match(findAll(cxxConstructorDecl(hasDescendant(MoveCallMatcher))),
+ *ContainingCtor, *Result.Context);
+ } else if (ContainingFunc) {
+ if (!ContainingFunc->getBody())
+ return;
+ ContainingNode = DynTypedNode::create(*ContainingFunc);
+ Matches = match(findAll(MoveCallMatcher), *ContainingFunc->getBody(),
+ *Result.Context);
+ } else {
+ return;
+ }
+
+ int MoveExprs = Matches.size();
+ for (const BoundNodes &Match : Matches) {
+ // The DeclRefExprs of non-initializer value captured variables refer to
+ // the original variable declaration in the AST. In such cases, we exclude
+ // those DeclRefExprs since they are not actually moving the original
+ // variable.
+ if (isValueCapturedByAnyLambda(*Result.Context, ContainingNode,
+ Match.getNodeAs<DeclRefExpr>("ref"),
+ Param)) {
+ --MoveExprs;
+ }
+ }
+
+ if (MoveExprs == 0) {
+ diag(Param->getBeginLoc(), "rvalue reference parameter is never moved from "
+ "inside the function body");
+ }
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -36,6 +36,7 @@
#include "ProTypeStaticCastDowncastCheck.h"
#include "ProTypeUnionAccessCheck.h"
#include "ProTypeVarargCheck.h"
+#include "RvalueReferenceParamNotMovedCheck.h"
#include "SlicingCheck.h"
#include "SpecialMemberFunctionsCheck.h"
#include "VirtualClassDestructorCheck.h"
@@ -99,6 +100,8 @@
"cppcoreguidelines-pro-type-union-access");
CheckFactories.registerCheck<ProTypeVarargCheck>(
"cppcoreguidelines-pro-type-vararg");
+ CheckFactories.registerCheck<RvalueReferenceParamNotMovedCheck>(
+ "cppcoreguidelines-rvalue-reference-param-not-moved");
CheckFactories.registerCheck<SpecialMemberFunctionsCheck>(
"cppcoreguidelines-special-member-functions");
CheckFactories.registerCheck<SlicingCheck>("cppcoreguidelines-slicing");
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
@@ -27,6 +27,7 @@
ProTypeStaticCastDowncastCheck.cpp
ProTypeUnionAccessCheck.cpp
ProTypeVarargCheck.cpp
+ RvalueReferenceParamNotMovedCheck.cpp
SlicingCheck.cpp
SpecialMemberFunctionsCheck.cpp
VirtualClassDestructorCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
