flx updated this revision to Diff 35204.
flx marked 10 inline comments as done.
http://reviews.llvm.org/D12839
Files:
clang-tidy/misc/MoveConstructorInitCheck.cpp
clang-tidy/misc/MoveConstructorInitCheck.h
clang-tidy/utils/CMakeLists.txt
clang-tidy/utils/Matchers.h
clang-tidy/utils/TypeTraits.cpp
clang-tidy/utils/TypeTraits.h
docs/clang-tidy/checks/misc-move-constructor-init.rst
test/clang-tidy/misc-move-constructor-init.cpp
Index: test/clang-tidy/misc-move-constructor-init.cpp
===================================================================
--- test/clang-tidy/misc-move-constructor-init.cpp
+++ test/clang-tidy/misc-move-constructor-init.cpp
@@ -76,3 +76,59 @@
B Mem;
N(N &&RHS) : Mem(move(RHS.Mem)) {}
};
+
+struct Movable {
+ Movable(Movable &&) = default;
+ Movable(const Movable &) = default;
+ Movable &operator=(const Movable &) = default;
+ ~Movable() {}
+};
+
+struct TriviallyCopyable {
+ TriviallyCopyable() = default;
+ TriviallyCopyable(TriviallyCopyable &&) = default;
+ TriviallyCopyable(const TriviallyCopyable &) = default;
+};
+
+struct Positive {
+ Positive(Movable M) : M_(M) {}
+ // CHECK-MESSAGES: [[@LINE-1]]:28: warning: value argument can be moved to avoid copy [misc-move-constructor-init]
+ Movable M_;
+};
+
+struct NegativeMultipleInitializerReferences {
+ NegativeMultipleInitializerReferences(Movable M) : M_(M), n_(M) {}
+ Movable M_;
+ Movable n_;
+};
+
+struct NegativeReferencedInConstructorBody {
+ NegativeReferencedInConstructorBody(Movable M) : M_(M) { M_ = M; }
+ Movable M_;
+};
+
+struct NegativeParamTriviallyCopyable {
+ NegativeParamTriviallyCopyable(TriviallyCopyable T) : T_(T) {}
+ NegativeParamTriviallyCopyable(int I) : I_(I) {}
+ TriviallyCopyable T_;
+ int I_;
+};
+
+struct NegativeNotPassedByValue {
+ NegativeNotPassedByValue(const Movable &M) : M_(M) {}
+ NegativeNotPassedByValue(const Movable M) : M_(M) {}
+ NegativeNotPassedByValue(Movable &M) : M_(M) {}
+ NegativeNotPassedByValue(Movable *M) : M_(*M) {}
+ NegativeNotPassedByValue(const Movable *M) : M_(*M) {}
+ Movable M_;
+};
+
+struct Immovable {
+ Immovable(const Immovable &) = default;
+ Immovable(Immovable &&) = delete;
+};
+
+struct NegativeImmovableParameter {
+ NegativeImmovableParameter(Immovable I) : I_(I) {}
+ Immovable I_;
+};
Index: docs/clang-tidy/checks/misc-move-constructor-init.rst
===================================================================
--- docs/clang-tidy/checks/misc-move-constructor-init.rst
+++ docs/clang-tidy/checks/misc-move-constructor-init.rst
@@ -5,3 +5,6 @@
The check flags user-defined move constructors that have a ctor-initializer
initializing a member or base class through a copy constructor instead of a
move constructor.
+
+It also flags constructor arguments that are passed by value, have a non-deleted
+move-constructor and are assigned to a class field by copy construction.
Index: clang-tidy/utils/TypeTraits.h
===================================================================
--- clang-tidy/utils/TypeTraits.h
+++ clang-tidy/utils/TypeTraits.h
@@ -0,0 +1,31 @@
+//===--- TypeTraits.h - clang-tidy-------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
+
+namespace clang {
+namespace tidy {
+namespace type_traits {
+
+/// Returns true if \c Type is defined has no non-trivial copy-constructor or
+/// destructor.
+bool classHasTrivialCopyAndDestroy(QualType Type);
+
+// \brief Returns true If \c Type is expensive to copy.
+bool isExpensiveToCopy(QualType Type, ASTContext &Context);
+
+} // type_traits
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_TYPETRAITS_H
Index: clang-tidy/utils/TypeTraits.cpp
===================================================================
--- clang-tidy/utils/TypeTraits.cpp
+++ clang-tidy/utils/TypeTraits.cpp
@@ -0,0 +1,38 @@
+//===--- TypeTraits.cpp - clang-tidy---------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "TypeTraits.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+
+namespace clang {
+namespace tidy {
+namespace type_traits {
+
+bool classHasTrivialCopyAndDestroy(QualType Type) {
+ auto *Record = Type->getAsCXXRecordDecl();
+ return Record && Record->hasDefinition() &&
+ !Record->hasNonTrivialCopyConstructor() &&
+ !Record->hasNonTrivialDestructor();
+}
+
+bool isExpensiveToCopy(QualType Type, ASTContext &Context) {
+ // We can't reason about dependent types. Ignore them.
+ // If there's a template instantiation that results in what we consider
+ // excessive copying, we'll warn on them.
+ if (Type->isDependentType())
+ return false;
+ // Ignore trivially copyable types.
+ return !(Type->isScalarType() || Type.isTriviallyCopyableType(Context) ||
+ classHasTrivialCopyAndDestroy(Type));
+}
+
+} // type_traits
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/utils/Matchers.h
===================================================================
--- clang-tidy/utils/Matchers.h
+++ clang-tidy/utils/Matchers.h
@@ -0,0 +1,23 @@
+//===--- Matchers.h - clang-tidy-------------------------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "TypeTraits.h"
+
+namespace clang {
+namespace tidy {
+namespace matchers {
+
+AST_MATCHER(QualType, isExpensiveToCopy) {
+ return type_traits::isExpensiveToCopy(Node, Finder->getASTContext());
+}
+
+} // namespace matchers
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/utils/CMakeLists.txt
===================================================================
--- clang-tidy/utils/CMakeLists.txt
+++ clang-tidy/utils/CMakeLists.txt
@@ -4,6 +4,7 @@
HeaderGuard.cpp
IncludeInserter.cpp
IncludeSorter.cpp
+ TypeTraits.cpp
LINK_LIBS
clangAST
Index: clang-tidy/misc/MoveConstructorInitCheck.h
===================================================================
--- clang-tidy/misc/MoveConstructorInitCheck.h
+++ clang-tidy/misc/MoveConstructorInitCheck.h
@@ -18,12 +18,20 @@
/// The check flags user-defined move constructors that have a ctor-initializer
/// initializing a member or base class through a copy constructor instead of a
/// move constructor.
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-move-constructor-init.html
class MoveConstructorInitCheck : public ClangTidyCheck {
public:
MoveConstructorInitCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ void
+ handleMoveConstructor(const ast_matchers::MatchFinder::MatchResult &Result);
+ void
+ handleParamNotMoved(const ast_matchers::MatchFinder::MatchResult &Result);
};
} // namespace tidy
Index: clang-tidy/misc/MoveConstructorInitCheck.cpp
===================================================================
--- clang-tidy/misc/MoveConstructorInitCheck.cpp
+++ clang-tidy/misc/MoveConstructorInitCheck.cpp
@@ -8,14 +8,32 @@
//===----------------------------------------------------------------------===//
#include "MoveConstructorInitCheck.h"
+#include "../utils/Matchers.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
using namespace clang::ast_matchers;
namespace clang {
namespace tidy {
+namespace {
+
+unsigned int parmVarDeclRefExprOccurences(const ParmVarDecl &MovableParam,
+ const CXXConstructorDecl &ConstructorDecl,
+ ASTContext &Context) {
+ unsigned int Occurrences = 0;
+ auto AllDeclRefs =
+ findAll(declRefExpr(to(parmVarDecl(equalsNode(&MovableParam)))));
+ Occurrences += match(AllDeclRefs, *ConstructorDecl.getBody(), Context).size();
+ for (const auto *Initializer : ConstructorDecl.inits()) {
+ Occurrences += match(AllDeclRefs, *Initializer->getInit(), Context).size();
+ }
+ return Occurrences;
+}
+
+} // namespace
+
void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
// Only register the matchers for C++11; the functionality currently does not
// provide any benefit to other languages, despite being benign.
@@ -28,14 +46,57 @@
hasAnyConstructorInitializer(
ctorInitializer(withInitializer(constructExpr(hasDeclaration(
constructorDecl(isCopyConstructor()).bind("ctor")
- )))).bind("init")
+ )))).bind("move-init")
)
)), this);
+
+ auto NonConstValueMovableAndExpensiveToCopy =
+ qualType(allOf(unless(pointerType()), unless(isConstQualified()),
+ hasDeclaration(recordDecl(hasMethod(constructorDecl(
+ isMoveConstructor(), unless(isDeleted()))))),
+ matchers::isExpensiveToCopy()));
+ Finder->addMatcher(
+ constructorDecl(
+ allOf(
+ unless(isMoveConstructor()),
+ hasAnyConstructorInitializer(withInitializer(constructExpr(
+ hasDeclaration(constructorDecl(isCopyConstructor())),
+ hasArgument(
+ 0, declRefExpr(
+ to(parmVarDecl(
+ hasType(
+ NonConstValueMovableAndExpensiveToCopy))
+ .bind("movable-param")))
+ .bind("init-arg")))))))
+ .bind("ctor-decl"),
+ this);
}
void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
+ if (Result.Nodes.getNodeAs<CXXCtorInitializer>("move-init") != nullptr)
+ handleMoveConstructor(Result);
+ if (Result.Nodes.getNodeAs<ParmVarDecl>("movable-param") != nullptr)
+ handleParamNotMoved(Result);
+}
+
+void MoveConstructorInitCheck::handleParamNotMoved(
+ const MatchFinder::MatchResult &Result) {
+ const auto *MovableParam =
+ Result.Nodes.getNodeAs<ParmVarDecl>("movable-param");
+ const auto *ConstructorDecl =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor-decl");
+ const auto *InitArg = Result.Nodes.getNodeAs<DeclRefExpr>("init-arg");
+ // If the parameter is referenced more than once it is not safe to move it.
+ if (parmVarDeclRefExprOccurences(*MovableParam, *ConstructorDecl,
+ *Result.Context) > 1)
+ return;
+ diag(InitArg->getLocStart(), "value argument can be moved to avoid copy");
+}
+
+void MoveConstructorInitCheck::handleMoveConstructor(
+ const MatchFinder::MatchResult &Result) {
const auto *CopyCtor = Result.Nodes.getNodeAs<CXXConstructorDecl>("ctor");
- const auto *Initializer = Result.Nodes.getNodeAs<CXXCtorInitializer>("init");
+ const auto *Initializer = Result.Nodes.getNodeAs<CXXCtorInitializer>("move-init");
// Do not diagnose if the expression used to perform the initialization is a
// trivially-copyable type.
@@ -79,4 +140,3 @@
} // namespace tidy
} // namespace clang
-
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
