diff options
Diffstat (limited to 'clang-tools-extra/clang-tidy')
10 files changed, 220 insertions, 14 deletions
diff --git a/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.cpp b/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.cpp index 0ba0f165114..ba182f3962e 100644 --- a/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.cpp @@ -8,14 +8,42 @@ //===----------------------------------------------------------------------===// #include "MoveConstructorInitCheck.h" +#include "../utils/Matchers.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.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 + +MoveConstructorInitCheck::MoveConstructorInitCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeStyle(IncludeSorter::parseIncludeStyle( + Options.get("IncludeStyle", "llvm"))) {} + 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. @@ -31,13 +59,65 @@ void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) { withInitializer(cxxConstructExpr(hasDeclaration( cxxConstructorDecl(isCopyConstructor()) .bind("ctor"))))) - .bind("init")))), + .bind("move-init")))), + this); + + auto NonConstValueMovableAndExpensiveToCopy = + qualType(allOf(unless(pointerType()), unless(isConstQualified()), + hasDeclaration(cxxRecordDecl(hasMethod(cxxConstructorDecl( + isMoveConstructor(), unless(isDeleted()))))), + matchers::isExpensiveToCopy())); + Finder->addMatcher( + cxxConstructorDecl( + allOf( + unless(isMoveConstructor()), + hasAnyConstructorInitializer(withInitializer(cxxConstructExpr( + hasDeclaration(cxxConstructorDecl(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; + auto DiagOut = + diag(InitArg->getLocStart(), "value argument can be moved to avoid copy"); + DiagOut << FixItHint::CreateReplacement( + InitArg->getSourceRange(), + (Twine("std::move(") + MovableParam->getName() + ")").str()); + if (auto IncludeFixit = Inserter->CreateIncludeInsertion( + Result.SourceManager->getFileID(InitArg->getLocStart()), "utility", + /*IsAngled=*/true)) { + DiagOut << *IncludeFixit; + } +} + +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,6 +159,15 @@ void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) { } } +void MoveConstructorInitCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Inserter.reset(new IncludeInserter(Compiler.getSourceManager(), + Compiler.getLangOpts(), IncludeStyle)); + Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); +} + +void MoveConstructorInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", IncludeSorter::toString(IncludeStyle)); +} + } // namespace tidy } // namespace clang - diff --git a/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.h b/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.h index 27fe5282d24..8898f2e3121 100644 --- a/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.h +++ b/clang-tools-extra/clang-tidy/misc/MoveConstructorInitCheck.h @@ -11,6 +11,9 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H #include "../ClangTidy.h" +#include "../utils/IncludeInserter.h" + +#include <memory> namespace clang { namespace tidy { @@ -18,12 +21,24 @@ namespace tidy { /// 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) {} + MoveConstructorInitCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(clang::CompilerInstance &Compiler) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + +private: + void + handleMoveConstructor(const ast_matchers::MatchFinder::MatchResult &Result); + void + handleParamNotMoved(const ast_matchers::MatchFinder::MatchResult &Result); + + std::unique_ptr<IncludeInserter> Inserter; + const IncludeSorter::IncludeStyle IncludeStyle; }; } // namespace tidy diff --git a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp index 3b12c594a63..02ec4b2d1c7 100644 --- a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp @@ -118,12 +118,11 @@ collectParamDecls(const CXXConstructorDecl *Ctor, PassByValueCheck::PassByValueCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - IncludeStyle(Options.get("IncludeStyle", "llvm") == "llvm" ? - IncludeSorter::IS_LLVM : IncludeSorter::IS_Google) {} + IncludeStyle(IncludeSorter::parseIncludeStyle( + Options.get("IncludeStyle", "llvm"))) {} void PassByValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "IncludeStyle", - IncludeStyle == IncludeSorter::IS_LLVM ? "llvm" : "google"); + Options.store(Opts, "IncludeStyle", IncludeSorter::toString(IncludeStyle)); } void PassByValueCheck::registerMatchers(MatchFinder *Finder) { diff --git a/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp index c6d6fcc2e2c..e41a3aec602 100644 --- a/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp @@ -189,13 +189,11 @@ static bool checkTokenIsAutoPtr(SourceLocation TokenStart, ReplaceAutoPtrCheck::ReplaceAutoPtrCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - IncludeStyle(Options.get("IncludeStyle", "llvm") == "llvm" - ? IncludeSorter::IS_LLVM - : IncludeSorter::IS_Google) {} + IncludeStyle(IncludeSorter::parseIncludeStyle( + Options.get("IncludeStyle", "llvm"))) {} void ReplaceAutoPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "IncludeStyle", - IncludeStyle == IncludeSorter::IS_LLVM ? "llvm" : "google"); + Options.store(Opts, "IncludeStyle", IncludeSorter::toString(IncludeStyle)); } void ReplaceAutoPtrCheck::registerMatchers(MatchFinder *Finder) { diff --git a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt index 06ae47e572e..5c64cc90801 100644 --- a/clang-tools-extra/clang-tidy/utils/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/utils/CMakeLists.txt @@ -4,6 +4,7 @@ add_clang_library(clangTidyUtils HeaderGuard.cpp IncludeInserter.cpp IncludeSorter.cpp + TypeTraits.cpp LINK_LIBS clangAST diff --git a/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp b/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp index 2432f072dac..d818dbbc647 100644 --- a/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp +++ b/clang-tools-extra/clang-tidy/utils/IncludeSorter.cpp @@ -285,5 +285,14 @@ FixItHint IncludeSorter::CreateFixIt(SourceRange EditRange, return Fix; } +IncludeSorter::IncludeStyle +IncludeSorter::parseIncludeStyle(const std::string &Value) { + return Value == "llvm" ? IS_LLVM : IS_Google; +} + +StringRef IncludeSorter::toString(IncludeStyle Style) { + return Style == IS_LLVM ? "llvm" : "google"; +} + } // namespace tidy } // namespace clang diff --git a/clang-tools-extra/clang-tidy/utils/IncludeSorter.h b/clang-tools-extra/clang-tidy/utils/IncludeSorter.h index b1fe316fd48..a5f71db3892 100644 --- a/clang-tools-extra/clang-tidy/utils/IncludeSorter.h +++ b/clang-tools-extra/clang-tidy/utils/IncludeSorter.h @@ -25,6 +25,12 @@ public: // Supported include styles. enum IncludeStyle { IS_LLVM = 0, IS_Google = 1 }; + // Converts "llvm" to IS_LLVM, otherwise returns IS_Google. + static IncludeStyle parseIncludeStyle(const std::string &Value); + + // Converts IncludeStyle to string representation. + static StringRef toString(IncludeStyle Style); + // The classifications of inclusions, in the order they should be sorted. enum IncludeKinds { IK_MainTUInclude = 0, // e.g. #include "foo.h" when editing foo.cc diff --git a/clang-tools-extra/clang-tidy/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h new file mode 100644 index 00000000000..d68740a4bd9 --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/Matchers.h @@ -0,0 +1,28 @@ +//===--- 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. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H + +#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 + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp new file mode 100644 index 00000000000..0de9677c8df --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.cpp @@ -0,0 +1,34 @@ +//===--- 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 { + +namespace { +bool classHasTrivialCopyAndDestroy(QualType Type) { + auto *Record = Type->getAsCXXRecordDecl(); + return Record && Record->hasDefinition() && + !Record->hasNonTrivialCopyConstructor() && + !Record->hasNonTrivialDestructor(); +} +} // namespace + +bool isExpensiveToCopy(QualType Type, ASTContext &Context) { + return !Type.isTriviallyCopyableType(Context) && + !classHasTrivialCopyAndDestroy(Type); +} + +} // type_traits +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/utils/TypeTraits.h b/clang-tools-extra/clang-tidy/utils/TypeTraits.h new file mode 100644 index 00000000000..33482b86e3e --- /dev/null +++ b/clang-tools-extra/clang-tidy/utils/TypeTraits.h @@ -0,0 +1,27 @@ +//===--- 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 { + +// \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 |