angelgarcia created this revision.
angelgarcia added a reviewer: alexfh.
angelgarcia added a subscriber: cfe-commits.
angelgarcia changed the visibility of this Differential Revision from "Public
(No Login Required)" to "All Users".
This is the first step for migrating cppmodernize to clang-tidy.
http://reviews.llvm.org/D11946
Files:
clang-tidy/CMakeLists.txt
clang-tidy/Makefile
clang-tidy/modernize/CMakeLists.txt
clang-tidy/modernize/Makefile
clang-tidy/modernize/ModernizeTidyModule.cpp
clang-tidy/modernize/PassByValueCheck.cpp
clang-tidy/modernize/PassByValueCheck.h
clang-tidy/tool/CMakeLists.txt
clang-tidy/tool/ClangTidyMain.cpp
clang-tidy/tool/Makefile
test/clang-tidy/modernize-pass-by-value.cpp
Index: test/clang-tidy/modernize-pass-by-value.cpp
===================================================================
--- test/clang-tidy/modernize-pass-by-value.cpp
+++ test/clang-tidy/modernize-pass-by-value.cpp
@@ -0,0 +1,185 @@
+// RUN: $(dirname %s)/check_clang_tidy.sh %s modernize-pass-by-value %t
+// REQUIRES: shell
+
+// CHECK: #include <utility>
+
+namespace {
+// POD types are trivially move constructible
+struct Movable {
+ int a, b, c;
+};
+
+struct NotMovable {
+ NotMovable() = default;
+ NotMovable(const NotMovable &) = default;
+ NotMovable(NotMovable &&) = delete;
+ int a, b, c;
+};
+}
+
+struct A {
+ A(const Movable &M) : M(M) {}
+ // CHECK: A(Movable M) : M(std::move(M)) {}
+ // SAFE_RISK: A(const Movable &M) : M(M) {}
+ Movable M;
+};
+
+// Test that we aren't modifying other things than a parameter
+Movable GlobalObj;
+struct B {
+ B(const Movable &M) : M(GlobalObj) {}
+ // CHECK: B(const Movable &M) : M(GlobalObj) {}
+ Movable M;
+};
+
+// Test that a parameter with more than one reference to it won't be changed.
+struct C {
+ // Tests extra-reference in body
+ C(const Movable &M) : M(M) { this->i = M.a; }
+ // CHECK: C(const Movable &M) : M(M) { this->i = M.a; }
+
+ // Tests extra-reference in init-list
+ C(const Movable &M, int) : M(M), i(M.a) {}
+ // CHECK: C(const Movable &M, int) : M(M), i(M.a) {}
+ Movable M;
+ int i;
+};
+
+// Test that both declaration and definition are updated
+struct D {
+ D(const Movable &M);
+ // CHECK: D(Movable M);
+ Movable M;
+};
+D::D(const Movable &M) : M(M) {}
+// CHECK: D::D(Movable M) : M(std::move(M)) {}
+
+// Test with default parameter
+struct E {
+ E(const Movable &M = Movable()) : M(M) {}
+ // CHECK: E(Movable M = Movable()) : M(std::move(M)) {}
+ Movable M;
+};
+
+// Test with object that can't be moved
+struct F {
+ F(const NotMovable &NM) : NM(NM) {}
+ // CHECK: F(const NotMovable &NM) : NM(NM) {}
+ NotMovable NM;
+};
+
+// Test unnamed parameter in declaration
+struct G {
+ G(const Movable &);
+ // CHECK: G(Movable );
+ Movable M;
+};
+G::G(const Movable &M) : M(M) {}
+// CHECK: G::G(Movable M) : M(std::move(M)) {}
+
+// Test parameter with and without qualifier
+namespace ns_H {
+typedef ::Movable HMovable;
+}
+struct H {
+ H(const ns_H::HMovable &M);
+ // CHECK: H(ns_H::HMovable M);
+ ns_H::HMovable M;
+};
+using namespace ns_H;
+H::H(const HMovable &M) : M(M) {}
+// CHECK: H(HMovable M) : M(std::move(M)) {}
+
+// Try messing up with macros
+#define MOVABLE_PARAM(Name) const Movable & Name
+// CHECK: #define MOVABLE_PARAM(Name) const Movable & Name
+struct I {
+ I(MOVABLE_PARAM(M)) : M(M) {}
+ // CHECK: I(MOVABLE_PARAM(M)) : M(M) {}
+ Movable M;
+};
+#undef MOVABLE_PARAM
+
+// Test that templates aren't modified
+template <typename T> struct J {
+ J(const T &M) : M(M) {}
+ // CHECK: J(const T &M) : M(M) {}
+ T M;
+};
+J<Movable> j1(Movable());
+J<NotMovable> j2(NotMovable());
+
+struct K_Movable {
+ K_Movable() = default;
+ K_Movable(const K_Movable &) = default;
+ K_Movable(K_Movable &&o) { dummy = o.dummy; }
+ int dummy;
+};
+
+// Test with movable type with an user defined move constructor.
+struct K {
+ K(const K_Movable &M) : M(M) {}
+ // CHECK: K(K_Movable M) : M(std::move(M)) {}
+ K_Movable M;
+};
+
+template <typename T> struct L {
+ L(const Movable &M) : M(M) {}
+ // CHECK: L(Movable M) : M(std::move(M)) {}
+ Movable M;
+};
+L<int> l(Movable());
+
+// Test with a non-instantiated template class
+template <typename T> struct N {
+ N(const Movable &M) : M(M) {}
+ // CHECK: N(Movable M) : M(std::move(M)) {}
+
+ Movable M;
+ T A;
+};
+
+// Test with value parameter
+struct O {
+ O(Movable M) : M(M) {}
+ // CHECK: O(Movable M) : M(std::move(M)) {}
+ Movable M;
+};
+
+// Test with a const-value parameter
+struct P {
+ P(const Movable M) : M(M) {}
+ // CHECK: P(const Movable M) : M(M) {}
+ Movable M;
+};
+
+// Test with multiples parameters where some need to be changed and some don't
+// need to.
+struct Q {
+ Q(const Movable &A, const Movable &B, const Movable &C, double D)
+ : A(A), B(B), C(C), D(D) {}
+ // CHECK: Q(const Movable &A, Movable B, Movable C, double D)
+ // CHECK-NEXT: : A(A), B(std::move(B)), C(std::move(C)), D(D) {}
+ const Movable &A;
+ Movable B;
+ Movable C;
+ double D;
+};
+
+// Test that value-parameters with a nested name specifier are left as-is
+namespace ns_R {
+typedef ::Movable RMovable;
+}
+struct R {
+ R(ns_R::RMovable M) : M(M) {}
+ // CHECK: R(ns_R::RMovable M) : M(std::move(M)) {}
+ ns_R::RMovable M;
+};
+
+// Test with rvalue parameter
+struct S {
+ S(Movable &&M) : M(M) {}
+ // CHECK: S(Movable &&M) : M(M) {}
+ Movable M;
+};
+
Index: clang-tidy/tool/Makefile
===================================================================
--- clang-tidy/tool/Makefile
+++ clang-tidy/tool/Makefile
@@ -17,9 +17,9 @@
include $(CLANG_LEVEL)/../../Makefile.config
LINK_COMPONENTS := $(TARGETS_TO_BUILD) asmparser bitreader support mc option
USEDLIBS = clangTidy.a clangTidyLLVMModule.a clangTidyGoogleModule.a \
- clangTidyMiscModule.a clangTidyReadability.a clangTidyUtils.a \
- clangStaticAnalyzerFrontend.a clangStaticAnalyzerCheckers.a \
- clangStaticAnalyzerCore.a \
+ clangTidyMiscModule.a clangTidyModernizeModule.a clangTidyReadability.a \
+ clangTidyUtils.a clangStaticAnalyzerFrontend.a \
+ clangStaticAnalyzerCheckers.a clangStaticAnalyzerCore.a \
clangFormat.a clangASTMatchers.a clangTooling.a clangToolingCore.a \
clangFrontend.a clangSerialization.a clangDriver.a clangParse.a \
clangSema.a clangAnalysis.a clangRewriteFrontend.a clangRewrite.a \
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -346,6 +346,10 @@
extern volatile int MiscModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED MiscModuleAnchorDestination = MiscModuleAnchorSource;
+// This anchor is used to force the linker to link the ModernizeModule.
+extern volatile int ModernizeModuleAnchorSource;
+static int ModernizeModuleAnchorDestination = ModernizeModuleAnchorSource;
+
// This anchor is used to force the linker to link the ReadabilityModule.
extern volatile int ReadabilityModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination = ReadabilityModuleAnchorSource;
Index: clang-tidy/tool/CMakeLists.txt
===================================================================
--- clang-tidy/tool/CMakeLists.txt
+++ clang-tidy/tool/CMakeLists.txt
@@ -13,6 +13,7 @@
clangTidyGoogleModule
clangTidyLLVMModule
clangTidyMiscModule
+ clangTidyModernizeModule
clangTidyReadabilityModule
clangTooling
)
Index: clang-tidy/modernize/PassByValueCheck.h
===================================================================
--- clang-tidy/modernize/PassByValueCheck.h
+++ clang-tidy/modernize/PassByValueCheck.h
@@ -0,0 +1,39 @@
+//===--- PassByValueCheck.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_MODERNIZE_PASS_BY_VALUE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_PASS_BY_VALUE_H
+
+#include "../ClangTidy.h"
+#include "../IncludeInserter.h"
+
+#include <memory>
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+class PassByValueCheck : public ClangTidyCheck {
+public:
+ PassByValueCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerPPCallbacks(clang::CompilerInstance &Compiler) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ std::unique_ptr<IncludeInserter> Inserter;
+};
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_PASS_BY_VALUE_H
+
Index: clang-tidy/modernize/PassByValueCheck.cpp
===================================================================
--- clang-tidy/modernize/PassByValueCheck.cpp
+++ clang-tidy/modernize/PassByValueCheck.cpp
@@ -0,0 +1,209 @@
+//===--- PassByValueCheck.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 "PassByValueCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+
+using namespace clang::ast_matchers;
+using namespace llvm;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+/// \brief Matches move constructible classes.
+///
+/// Given
+/// \code
+/// // POD types are trivially move constructible
+/// struct Foo { int a; };
+///
+/// struct Bar {
+/// Bar(Bar &&) = deleted;
+/// int a;
+/// };
+/// \endcode
+/// recordDecl(isMoveConstructible())
+/// matches "Foo".
+AST_MATCHER(CXXRecordDecl, isMoveConstructible) {
+ for (const CXXConstructorDecl* Ctor : Node.ctors()) {
+ if (Ctor->isMoveConstructor() && !Ctor->isDeleted())
+ return true;
+ }
+ return false;
+}
+
+/// \brief Matches non-deleted copy constructors.
+///
+/// Given
+/// \code
+/// struct Foo { Foo(const Foo &) = default; };
+/// struct Bar { Bar(const Bar &) = deleted; };
+/// \endcode
+/// constructorDecl(isNonDeletedCopyConstructor())
+/// matches "Foo(const Foo &)".
+AST_MATCHER(CXXConstructorDecl, isNonDeletedCopyConstructor) {
+ return Node.isCopyConstructor() && !Node.isDeleted();
+}
+
+static TypeMatcher constRefType() {
+ return lValueReferenceType(pointee(isConstQualified()));
+}
+
+static TypeMatcher nonConstValueType() {
+ return qualType(unless(anyOf(referenceType(), isConstQualified())));
+}
+
+/// \brief \c clang::RecursiveASTVisitor that checks that the given
+/// \c ParmVarDecl is used exactly one time.
+///
+/// \see ExactlyOneUsageVisitor::hasExactlyOneUsageIn()
+class ExactlyOneUsageVisitor
+ : public RecursiveASTVisitor<ExactlyOneUsageVisitor> {
+ friend class RecursiveASTVisitor<ExactlyOneUsageVisitor>;
+
+public:
+ ExactlyOneUsageVisitor(const ParmVarDecl *ParamDecl) : ParamDecl(ParamDecl) {}
+
+ /// \brief Whether or not the parameter variable is referred only once in the
+ /// given constructor.
+ bool hasExactlyOneUsageIn(const CXXConstructorDecl *Ctor) {
+ Count = 0;
+ TraverseDecl(const_cast<CXXConstructorDecl*>(Ctor));
+ return Count == 1;
+ }
+
+private:
+ /// \brief Counts the number of references to a variable.
+ ///
+ /// Stops the AST traversal if more than one usage is found.
+ bool VisitDeclRefExpr(DeclRefExpr *D) {
+ if (const ParmVarDecl *To = dyn_cast<ParmVarDecl>(D->getDecl()))
+ if (To == ParamDecl) {
+ ++Count;
+ if (Count > 1)
+ // no need to look further, used more than once
+ return false;
+ }
+ return true;
+ }
+
+ const ParmVarDecl *ParamDecl;
+ unsigned Count;
+};
+
+/// \brief Whether or not \p ParamDecl is used exactly one time in \p Ctor.
+///
+/// Checks both in the init-list and the body of the constructor.
+static bool paramReferredExactlyOnce(const CXXConstructorDecl *Ctor,
+ const ParmVarDecl *ParamDecl) {
+ ExactlyOneUsageVisitor Visitor(ParamDecl);
+ return Visitor.hasExactlyOneUsageIn(Ctor);
+}
+
+/// \brief Find all references to \p ParamDecl across all of the
+/// redeclarations of \p Ctor.
+static void collectParamDecls(const CXXConstructorDecl *Ctor,
+ const ParmVarDecl *ParamDecl,
+ SmallVectorImpl<const ParmVarDecl*> &Results) {
+ unsigned ParamIdx = ParamDecl->getFunctionScopeIndex();
+
+ for (const FunctionDecl* Redecl : Ctor->redecls())
+ Results.push_back(Redecl->getParamDecl(ParamIdx));
+}
+
+
+void PassByValueCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(constructorDecl(
+ forEachConstructorInitializer(ctorInitializer(
+ // Clang builds a CXXConstructExpr only whin it knows which
+ // constructor will be called. In dependent contexts a ParenListExpr
+ // is generated instead of a CXXConstructExpr, filtering out
+ // templates automatically for us.
+ withInitializer(constructExpr(
+ has(declRefExpr(to(
+ parmVarDecl(hasType(qualType(
+ // match only constref or a non-const value parameters,
+ // rvalues and const-values shouldn't be modified.
+ anyOf(constRefType(), nonConstValueType()))))
+ .bind("Param")))),
+ hasDeclaration(constructorDecl(
+ isNonDeletedCopyConstructor(),
+ hasDeclContext(recordDecl(isMoveConstructible())))))))
+ .bind("Initializer")))
+ .bind("Ctor"), this);
+}
+
+void PassByValueCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+ Inserter.reset(new IncludeInserter(Compiler.getSourceManager(),
+ Compiler.getLangOpts(),
+ IncludeSorter::IS_LLVM));
+ Compiler.getPreprocessor().addPPCallbacks(
+ Inserter->CreatePPCallbacks());
+}
+
+void PassByValueCheck::check(const MatchFinder::MatchResult &Result) {
+ const CXXConstructorDecl *Ctor =
+ Result.Nodes.getNodeAs<CXXConstructorDecl>("Ctor");
+ const ParmVarDecl *ParamDecl =
+ Result.Nodes.getNodeAs<ParmVarDecl>("Param");
+ const CXXCtorInitializer *Initializer =
+ Result.Nodes.getNodeAs<CXXCtorInitializer>("Initializer");
+ SourceManager &SM = *Result.SourceManager;
+
+ // If the parameter is used or anything other than the copy, do not apply
+ // the changes.
+ if (!paramReferredExactlyOnce(Ctor, ParamDecl))
+ return;
+
+
+ SmallVector<const ParmVarDecl*, 2> AllParamDecls;
+ collectParamDecls(Ctor, ParamDecl, AllParamDecls);
+
+ auto Diag = diag(ParamDecl->getLocStart(),
+ "Pass by value and use std::move");
+
+ // Iterate over all declarations of the constructor.
+ for (const ParmVarDecl* ParmDecl : AllParamDecls) {
+ TypeLoc ParamTL = ParmDecl->getTypeSourceInfo()->getTypeLoc();
+ ReferenceTypeLoc RefTL = ParamTL.getAs<ReferenceTypeLoc>();
+
+ // Do not replace if it is already a value, skip.
+ if (RefTL.isNull()) continue;
+
+ TypeLoc ValueTL = RefTL.getPointeeLoc();
+ auto TypeRange = CharSourceRange::getTokenRange(ParmDecl->getLocStart(),
+ ParamTL.getLocEnd());
+ auto ValueStr = Lexer::getSourceText(CharSourceRange::getTokenRange(
+ ValueTL.getSourceRange()), SM, Result.Context->getLangOpts()).str();
+ ValueStr += ' ';
+ Diag << FixItHint::CreateReplacement(TypeRange, ValueStr);
+ }
+
+ // Move the value in the initialization list.
+ Diag << FixItHint::CreateInsertion(Initializer->getRParenLoc(), ")")
+ << FixItHint::CreateInsertion(
+ Initializer->getLParenLoc().getLocWithOffset(1), "std::move(");
+
+ auto Insertion = Inserter->CreateIncludeInsertion(SM.getMainFileID(),
+ "utility",
+ /*IsAngled=*/true);
+ if (Insertion.hasValue()) Diag << Insertion.getValue();
+}
+
+} // namespace modernize
+} // namespace tidy
+} // namespace clang
+
Index: clang-tidy/modernize/ModernizeTidyModule.cpp
===================================================================
--- clang-tidy/modernize/ModernizeTidyModule.cpp
+++ clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -0,0 +1,50 @@
+//===--- GoogleTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "../readability/BracesAroundStatementsCheck.h"
+#include "../readability/FunctionSizeCheck.h"
+#include "../readability/NamespaceCommentCheck.h"
+#include "../readability/RedundantSmartptrGetCheck.h"
+#include "PassByValueCheck.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace modernize {
+
+class ModernizeModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<PassByValueCheck>(
+ "modernize-pass-by-value");
+ }
+
+ ClangTidyOptions getModuleOptions() override {
+ ClangTidyOptions Options;
+ auto &Opts = Options.CheckOptions;
+ return Options;
+ }
+};
+
+// Register the GoogleTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<ModernizeModule> X("modernize-module",
+ "Adds modernize checks.");
+
+} // namespace modernize
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the ModernizeModule.
+volatile int ModernizeModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/modernize/Makefile
===================================================================
--- clang-tidy/modernize/Makefile
+++ clang-tidy/modernize/Makefile
@@ -0,0 +1,12 @@
+##===- clang-tidy/google/Makefile --------------------------*- Makefile -*-===##
+#
+# The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+##===----------------------------------------------------------------------===##
+CLANG_LEVEL := ../../../..
+LIBRARYNAME := clangTidyModernizeModule
+
+include $(CLANG_LEVEL)/Makefile
Index: clang-tidy/modernize/CMakeLists.txt
===================================================================
--- clang-tidy/modernize/CMakeLists.txt
+++ clang-tidy/modernize/CMakeLists.txt
@@ -0,0 +1,14 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyModernizeModule
+ ModernizeTidyModule.cpp
+ PassByValueCheck.cpp
+
+ LINK_LIBS
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyReadabilityModule
+ )
Index: clang-tidy/Makefile
===================================================================
--- clang-tidy/Makefile
+++ clang-tidy/Makefile
@@ -11,6 +11,6 @@
LIBRARYNAME := clangTidy
include $(CLANG_LEVEL)/../../Makefile.config
-DIRS = utils readability llvm google misc tool
+DIRS = utils readability llvm google misc modernize tool
include $(CLANG_LEVEL)/Makefile
Index: clang-tidy/CMakeLists.txt
===================================================================
--- clang-tidy/CMakeLists.txt
+++ clang-tidy/CMakeLists.txt
@@ -30,5 +30,6 @@
add_subdirectory(llvm)
add_subdirectory(google)
add_subdirectory(misc)
+add_subdirectory(modernize)
add_subdirectory(readability)
add_subdirectory(utils)
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
