Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ clang-tidy/cert/CERTTidyModule.cpp @@ -11,11 +11,11 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" -#include "../misc/MoveConstructorInitCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" #include "../misc/NonCopyableObjects.h" #include "../misc/StaticAssertCheck.h" #include "../misc/ThrowByValueCatchByReferenceCheck.h" +#include "../performance/MoveConstructorInitCheck.h" #include "CommandProcessorCheck.h" #include "DontModifyStdNamespaceCheck.h" #include "FloatLoopCounter.h" @@ -46,7 +46,7 @@ CheckFactories.registerCheck( "cert-dcl59-cpp"); // OOP - CheckFactories.registerCheck( + CheckFactories.registerCheck( "cert-oop11-cpp"); // ERR CheckFactories.registerCheck( Index: clang-tidy/misc/CMakeLists.txt =================================================================== --- clang-tidy/misc/CMakeLists.txt +++ clang-tidy/misc/CMakeLists.txt @@ -7,13 +7,11 @@ UnconventionalAssignOperatorCheck.cpp DefinitionsInHeadersCheck.cpp IncorrectRoundings.cpp - InefficientAlgorithmCheck.cpp MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp MiscTidyModule.cpp MisplacedWideningCastCheck.cpp MoveConstantArgumentCheck.cpp - MoveConstructorInitCheck.cpp NewDeleteOverloadsCheck.cpp NoexceptMoveConstructorCheck.cpp NonCopyableObjects.cpp Index: clang-tidy/misc/InefficientAlgorithmCheck.h =================================================================== --- clang-tidy/misc/InefficientAlgorithmCheck.h +++ clang-tidy/misc/InefficientAlgorithmCheck.h @@ -1,36 +0,0 @@ -//===--- InefficientAlgorithmCheck.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_MISC_INEFFICIENTALGORITHMCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INEFFICIENTALGORITHMCHECK_H - -#include "../ClangTidy.h" - -namespace clang { -namespace tidy { -namespace misc { - -/// Warns on inefficient use of STL algorithms on associative containers. -/// -/// Associative containers implements some of the algorithms as methods which -/// should be preferred to the algorithms in the algorithm header. The methods -/// can take advanatage of the order of the elements. -class InefficientAlgorithmCheck : public ClangTidyCheck { -public: - InefficientAlgorithmCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} - void registerMatchers(ast_matchers::MatchFinder *Finder) override; - void check(const ast_matchers::MatchFinder::MatchResult &Result) override; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_INEFFICIENTALGORITHMCHECK_H Index: clang-tidy/misc/InefficientAlgorithmCheck.cpp =================================================================== --- clang-tidy/misc/InefficientAlgorithmCheck.cpp +++ clang-tidy/misc/InefficientAlgorithmCheck.cpp @@ -1,163 +0,0 @@ -//===--- InefficientAlgorithmCheck.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 "InefficientAlgorithmCheck.h" -#include "clang/AST/ASTContext.h" -#include "clang/ASTMatchers/ASTMatchFinder.h" -#include "clang/Lex/Lexer.h" - -using namespace clang::ast_matchers; - -namespace clang { -namespace tidy { -namespace misc { - -static bool areTypesCompatible(QualType Left, QualType Right) { - if (const auto *LeftRefType = Left->getAs()) - Left = LeftRefType->getPointeeType(); - if (const auto *RightRefType = Right->getAs()) - Right = RightRefType->getPointeeType(); - return Left->getCanonicalTypeUnqualified() == - Right->getCanonicalTypeUnqualified(); -} - -void InefficientAlgorithmCheck::registerMatchers(MatchFinder *Finder) { - // Only register the matchers for C++; the functionality currently does not - // provide any benefit to other languages, despite being benign. - if (!getLangOpts().CPlusPlus) - return; - - const auto Algorithms = - hasAnyName("::std::find", "::std::count", "::std::equal_range", - "::std::lower_bound", "::std::upper_bound"); - const auto ContainerMatcher = classTemplateSpecializationDecl(hasAnyName( - "::std::set", "::std::map", "::std::multiset", "::std::multimap", - "::std::unordered_set", "::std::unordered_map", - "::std::unordered_multiset", "::std::unordered_multimap")); - - const auto Matcher = - callExpr( - callee(functionDecl(Algorithms)), - hasArgument( - 0, cxxConstructExpr(has(ignoringParenImpCasts(cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("begin"))), - on(declRefExpr( - hasDeclaration(decl().bind("IneffContObj")), - anyOf(hasType(ContainerMatcher.bind("IneffCont")), - hasType(pointsTo( - ContainerMatcher.bind("IneffContPtr"))))) - .bind("IneffContExpr"))))))), - hasArgument( - 1, cxxConstructExpr(has(ignoringParenImpCasts(cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("end"))), - on(declRefExpr( - hasDeclaration(equalsBoundNode("IneffContObj"))))))))), - hasArgument(2, expr().bind("AlgParam")), - unless(isInTemplateInstantiation())) - .bind("IneffAlg"); - - Finder->addMatcher(Matcher, this); -} - -void InefficientAlgorithmCheck::check(const MatchFinder::MatchResult &Result) { - const auto *AlgCall = Result.Nodes.getNodeAs("IneffAlg"); - const auto *IneffCont = - Result.Nodes.getNodeAs("IneffCont"); - bool PtrToContainer = false; - if (!IneffCont) { - IneffCont = - Result.Nodes.getNodeAs("IneffContPtr"); - PtrToContainer = true; - } - const llvm::StringRef IneffContName = IneffCont->getName(); - const bool Unordered = - IneffContName.find("unordered") != llvm::StringRef::npos; - const bool Maplike = IneffContName.find("map") != llvm::StringRef::npos; - - // Store if the key type of the container is compatible with the value - // that is searched for. - QualType ValueType = AlgCall->getArg(2)->getType(); - QualType KeyType = - IneffCont->getTemplateArgs()[0].getAsType().getCanonicalType(); - const bool CompatibleTypes = areTypesCompatible(KeyType, ValueType); - - // Check if the comparison type for the algorithm and the container matches. - if (AlgCall->getNumArgs() == 4 && !Unordered) { - const Expr *Arg = AlgCall->getArg(3); - const QualType AlgCmp = - Arg->getType().getUnqualifiedType().getCanonicalType(); - const unsigned CmpPosition = - (IneffContName.find("map") == llvm::StringRef::npos) ? 1 : 2; - const QualType ContainerCmp = IneffCont->getTemplateArgs()[CmpPosition] - .getAsType() - .getUnqualifiedType() - .getCanonicalType(); - if (AlgCmp != ContainerCmp) { - diag(Arg->getLocStart(), - "different comparers used in the algorithm and the container"); - return; - } - } - - const auto *AlgDecl = AlgCall->getDirectCallee(); - if (!AlgDecl) - return; - - if (Unordered && AlgDecl->getName().find("bound") != llvm::StringRef::npos) - return; - - const auto *AlgParam = Result.Nodes.getNodeAs("AlgParam"); - const auto *IneffContExpr = Result.Nodes.getNodeAs("IneffContExpr"); - FixItHint Hint; - - SourceManager &SM = *Result.SourceManager; - LangOptions LangOpts = getLangOpts(); - - CharSourceRange CallRange = - CharSourceRange::getTokenRange(AlgCall->getSourceRange()); - - // FIXME: Create a common utility to extract a file range that the given token - // sequence is exactly spelled at (without macro argument expansions etc.). - // We can't use Lexer::makeFileCharRange here, because for - // - // #define F(x) x - // x(a b c); - // - // it will return "x(a b c)", when given the range "a"-"c". It makes sense for - // removals, but not for replacements. - // - // This code is over-simplified, but works for many real cases. - if (SM.isMacroArgExpansion(CallRange.getBegin()) && - SM.isMacroArgExpansion(CallRange.getEnd())) { - CallRange.setBegin(SM.getSpellingLoc(CallRange.getBegin())); - CallRange.setEnd(SM.getSpellingLoc(CallRange.getEnd())); - } - - if (!CallRange.getBegin().isMacroID() && !Maplike && CompatibleTypes) { - StringRef ContainerText = Lexer::getSourceText( - CharSourceRange::getTokenRange(IneffContExpr->getSourceRange()), SM, - LangOpts); - StringRef ParamText = Lexer::getSourceText( - CharSourceRange::getTokenRange(AlgParam->getSourceRange()), SM, - LangOpts); - std::string ReplacementText = - (llvm::Twine(ContainerText) + (PtrToContainer ? "->" : ".") + - AlgDecl->getName() + "(" + ParamText + ")") - .str(); - Hint = FixItHint::CreateReplacement(CallRange, ReplacementText); - } - - diag(AlgCall->getLocStart(), - "this STL algorithm call should be replaced with a container method") - << Hint; -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/misc/MiscTidyModule.cpp =================================================================== --- clang-tidy/misc/MiscTidyModule.cpp +++ clang-tidy/misc/MiscTidyModule.cpp @@ -13,14 +13,12 @@ #include "DefinitionsInHeadersCheck.h" #include "ForwardingReferenceOverloadCheck.h" #include "IncorrectRoundings.h" -#include "InefficientAlgorithmCheck.h" #include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" #include "MisplacedConstCheck.h" #include "MisplacedWideningCastCheck.h" #include "MoveConstantArgumentCheck.h" -#include "MoveConstructorInitCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NoexceptMoveConstructorCheck.h" #include "NonCopyableObjects.h" @@ -63,8 +61,6 @@ "misc-definitions-in-headers"); CheckFactories.registerCheck( "misc-incorrect-roundings"); - CheckFactories.registerCheck( - "misc-inefficient-algorithm"); CheckFactories.registerCheck( "misc-macro-parentheses"); CheckFactories.registerCheck( @@ -73,8 +69,6 @@ "misc-misplaced-widening-cast"); CheckFactories.registerCheck( "misc-move-const-arg"); - CheckFactories.registerCheck( - "misc-move-constructor-init"); CheckFactories.registerCheck( "misc-new-delete-overloads"); CheckFactories.registerCheck( Index: clang-tidy/misc/MoveConstructorInitCheck.h =================================================================== --- clang-tidy/misc/MoveConstructorInitCheck.h +++ clang-tidy/misc/MoveConstructorInitCheck.h @@ -1,44 +0,0 @@ -//===--- MoveConstructorInitCheck.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_MISC_MOVECONSTRUCTORINITCHECK_H -#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H - -#include "../ClangTidy.h" -#include "../utils/IncludeInserter.h" - -#include - -namespace clang { -namespace tidy { -namespace misc { - -/// 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); - 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: - std::unique_ptr Inserter; - const utils::IncludeSorter::IncludeStyle IncludeStyle; -}; - -} // namespace misc -} // namespace tidy -} // namespace clang - -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTRUCTORINITCHECK_H Index: clang-tidy/misc/MoveConstructorInitCheck.cpp =================================================================== --- clang-tidy/misc/MoveConstructorInitCheck.cpp +++ clang-tidy/misc/MoveConstructorInitCheck.cpp @@ -1,110 +0,0 @@ -//===--- MoveConstructorInitCheck.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 "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 misc { - -MoveConstructorInitCheck::MoveConstructorInitCheck(StringRef Name, - ClangTidyContext *Context) - : ClangTidyCheck(Name, Context), - IncludeStyle(utils::IncludeSorter::parseIncludeStyle( - Options.getLocalOrGlobal("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. - if (!getLangOpts().CPlusPlus11) - return; - - Finder->addMatcher( - cxxConstructorDecl( - unless(isImplicit()), - allOf(isMoveConstructor(), - hasAnyConstructorInitializer( - cxxCtorInitializer( - withInitializer(cxxConstructExpr(hasDeclaration( - cxxConstructorDecl(isCopyConstructor()) - .bind("ctor"))))) - .bind("move-init")))), - this); -} - -void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) { - const auto *CopyCtor = Result.Nodes.getNodeAs("ctor"); - const auto *Initializer = - Result.Nodes.getNodeAs("move-init"); - - // Do not diagnose if the expression used to perform the initialization is a - // trivially-copyable type. - QualType QT = Initializer->getInit()->getType(); - if (QT.isTriviallyCopyableType(*Result.Context)) - return; - - if (QT.isConstQualified()) - return; - - const auto *RD = QT->getAsCXXRecordDecl(); - if (RD && RD->isTriviallyCopyable()) - return; - - // Diagnose when the class type has a move constructor available, but the - // ctor-initializer uses the copy constructor instead. - const CXXConstructorDecl *Candidate = nullptr; - for (const auto *Ctor : CopyCtor->getParent()->ctors()) { - if (Ctor->isMoveConstructor() && Ctor->getAccess() <= AS_protected && - !Ctor->isDeleted()) { - // The type has a move constructor that is at least accessible to the - // initializer. - // - // FIXME: Determine whether the move constructor is a viable candidate - // for the ctor-initializer, perhaps provide a fixit that suggests - // using std::move(). - Candidate = Ctor; - break; - } - } - - if (Candidate) { - // There's a move constructor candidate that the caller probably intended - // to call instead. - diag(Initializer->getSourceLocation(), - "move constructor initializes %0 by calling a copy constructor") - << (Initializer->isBaseInitializer() ? "base class" : "class member"); - diag(CopyCtor->getLocation(), "copy constructor being called", - DiagnosticIDs::Note); - diag(Candidate->getLocation(), "candidate move constructor here", - DiagnosticIDs::Note); - } -} - -void MoveConstructorInitCheck::registerPPCallbacks(CompilerInstance &Compiler) { - Inserter.reset(new utils::IncludeInserter( - Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle)); - Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); -} - -void MoveConstructorInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { - Options.store(Opts, "IncludeStyle", - utils::IncludeSorter::toString(IncludeStyle)); -} - -} // namespace misc -} // namespace tidy -} // namespace clang Index: clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tidy/performance/CMakeLists.txt +++ clang-tidy/performance/CMakeLists.txt @@ -4,8 +4,10 @@ FasterStringFindCheck.cpp ForRangeCopyCheck.cpp ImplicitConversionInLoopCheck.cpp + InefficientAlgorithmCheck.cpp InefficientStringConcatenationCheck.cpp InefficientVectorOperationCheck.cpp + MoveConstructorInitCheck.cpp PerformanceTidyModule.cpp TypePromotionInMathFnCheck.cpp UnnecessaryCopyInitialization.cpp Index: clang-tidy/performance/InefficientAlgorithmCheck.h =================================================================== --- clang-tidy/performance/InefficientAlgorithmCheck.h +++ clang-tidy/performance/InefficientAlgorithmCheck.h @@ -0,0 +1,36 @@ +//===--- InefficientAlgorithmCheck.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_PERFORMANCE_INEFFICIENTALGORITHMCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTALGORITHMCHECK_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace performance { + +/// Warns on inefficient use of STL algorithms on associative containers. +/// +/// Associative containers implements some of the algorithms as methods which +/// should be preferred to the algorithms in the algorithm header. The methods +/// can take advanatage of the order of the elements. +class InefficientAlgorithmCheck : public ClangTidyCheck { +public: + InefficientAlgorithmCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_INEFFICIENTALGORITHMCHECK_H Index: clang-tidy/performance/InefficientAlgorithmCheck.cpp =================================================================== --- clang-tidy/performance/InefficientAlgorithmCheck.cpp +++ clang-tidy/performance/InefficientAlgorithmCheck.cpp @@ -0,0 +1,163 @@ +//===--- InefficientAlgorithmCheck.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 "InefficientAlgorithmCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace performance { + +static bool areTypesCompatible(QualType Left, QualType Right) { + if (const auto *LeftRefType = Left->getAs()) + Left = LeftRefType->getPointeeType(); + if (const auto *RightRefType = Right->getAs()) + Right = RightRefType->getPointeeType(); + return Left->getCanonicalTypeUnqualified() == + Right->getCanonicalTypeUnqualified(); +} + +void InefficientAlgorithmCheck::registerMatchers(MatchFinder *Finder) { + // Only register the matchers for C++; the functionality currently does not + // provide any benefit to other languages, despite being benign. + if (!getLangOpts().CPlusPlus) + return; + + const auto Algorithms = + hasAnyName("::std::find", "::std::count", "::std::equal_range", + "::std::lower_bound", "::std::upper_bound"); + const auto ContainerMatcher = classTemplateSpecializationDecl(hasAnyName( + "::std::set", "::std::map", "::std::multiset", "::std::multimap", + "::std::unordered_set", "::std::unordered_map", + "::std::unordered_multiset", "::std::unordered_multimap")); + + const auto Matcher = + callExpr( + callee(functionDecl(Algorithms)), + hasArgument( + 0, cxxConstructExpr(has(ignoringParenImpCasts(cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("begin"))), + on(declRefExpr( + hasDeclaration(decl().bind("IneffContObj")), + anyOf(hasType(ContainerMatcher.bind("IneffCont")), + hasType(pointsTo( + ContainerMatcher.bind("IneffContPtr"))))) + .bind("IneffContExpr"))))))), + hasArgument( + 1, cxxConstructExpr(has(ignoringParenImpCasts(cxxMemberCallExpr( + callee(cxxMethodDecl(hasName("end"))), + on(declRefExpr( + hasDeclaration(equalsBoundNode("IneffContObj"))))))))), + hasArgument(2, expr().bind("AlgParam")), + unless(isInTemplateInstantiation())) + .bind("IneffAlg"); + + Finder->addMatcher(Matcher, this); +} + +void InefficientAlgorithmCheck::check(const MatchFinder::MatchResult &Result) { + const auto *AlgCall = Result.Nodes.getNodeAs("IneffAlg"); + const auto *IneffCont = + Result.Nodes.getNodeAs("IneffCont"); + bool PtrToContainer = false; + if (!IneffCont) { + IneffCont = + Result.Nodes.getNodeAs("IneffContPtr"); + PtrToContainer = true; + } + const llvm::StringRef IneffContName = IneffCont->getName(); + const bool Unordered = + IneffContName.find("unordered") != llvm::StringRef::npos; + const bool Maplike = IneffContName.find("map") != llvm::StringRef::npos; + + // Store if the key type of the container is compatible with the value + // that is searched for. + QualType ValueType = AlgCall->getArg(2)->getType(); + QualType KeyType = + IneffCont->getTemplateArgs()[0].getAsType().getCanonicalType(); + const bool CompatibleTypes = areTypesCompatible(KeyType, ValueType); + + // Check if the comparison type for the algorithm and the container matches. + if (AlgCall->getNumArgs() == 4 && !Unordered) { + const Expr *Arg = AlgCall->getArg(3); + const QualType AlgCmp = + Arg->getType().getUnqualifiedType().getCanonicalType(); + const unsigned CmpPosition = + (IneffContName.find("map") == llvm::StringRef::npos) ? 1 : 2; + const QualType ContainerCmp = IneffCont->getTemplateArgs()[CmpPosition] + .getAsType() + .getUnqualifiedType() + .getCanonicalType(); + if (AlgCmp != ContainerCmp) { + diag(Arg->getLocStart(), + "different comparers used in the algorithm and the container"); + return; + } + } + + const auto *AlgDecl = AlgCall->getDirectCallee(); + if (!AlgDecl) + return; + + if (Unordered && AlgDecl->getName().find("bound") != llvm::StringRef::npos) + return; + + const auto *AlgParam = Result.Nodes.getNodeAs("AlgParam"); + const auto *IneffContExpr = Result.Nodes.getNodeAs("IneffContExpr"); + FixItHint Hint; + + SourceManager &SM = *Result.SourceManager; + LangOptions LangOpts = getLangOpts(); + + CharSourceRange CallRange = + CharSourceRange::getTokenRange(AlgCall->getSourceRange()); + + // FIXME: Create a common utility to extract a file range that the given token + // sequence is exactly spelled at (without macro argument expansions etc.). + // We can't use Lexer::makeFileCharRange here, because for + // + // #define F(x) x + // x(a b c); + // + // it will return "x(a b c)", when given the range "a"-"c". It makes sense for + // removals, but not for replacements. + // + // This code is over-simplified, but works for many real cases. + if (SM.isMacroArgExpansion(CallRange.getBegin()) && + SM.isMacroArgExpansion(CallRange.getEnd())) { + CallRange.setBegin(SM.getSpellingLoc(CallRange.getBegin())); + CallRange.setEnd(SM.getSpellingLoc(CallRange.getEnd())); + } + + if (!CallRange.getBegin().isMacroID() && !Maplike && CompatibleTypes) { + StringRef ContainerText = Lexer::getSourceText( + CharSourceRange::getTokenRange(IneffContExpr->getSourceRange()), SM, + LangOpts); + StringRef ParamText = Lexer::getSourceText( + CharSourceRange::getTokenRange(AlgParam->getSourceRange()), SM, + LangOpts); + std::string ReplacementText = + (llvm::Twine(ContainerText) + (PtrToContainer ? "->" : ".") + + AlgDecl->getName() + "(" + ParamText + ")") + .str(); + Hint = FixItHint::CreateReplacement(CallRange, ReplacementText); + } + + diag(AlgCall->getLocStart(), + "this STL algorithm call should be replaced with a container method") + << Hint; +} + +} // namespace performance +} // namespace tidy +} // namespace clang Index: clang-tidy/performance/MoveConstructorInitCheck.h =================================================================== --- clang-tidy/performance/MoveConstructorInitCheck.h +++ clang-tidy/performance/MoveConstructorInitCheck.h @@ -0,0 +1,44 @@ +//===--- MoveConstructorInitCheck.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_PERFORMANCE_MOVECONSTRUCTORINITCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_MOVECONSTRUCTORINITCHECK_H + +#include "../ClangTidy.h" +#include "../utils/IncludeInserter.h" + +#include + +namespace clang { +namespace tidy { +namespace performance { + +/// 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/performance-move-constructor-init.html +class MoveConstructorInitCheck : public ClangTidyCheck { +public: + 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: + std::unique_ptr Inserter; + const utils::IncludeSorter::IncludeStyle IncludeStyle; +}; + +} // namespace performance +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_MOVECONSTRUCTORINITCHECK_H Index: clang-tidy/performance/MoveConstructorInitCheck.cpp =================================================================== --- clang-tidy/performance/MoveConstructorInitCheck.cpp +++ clang-tidy/performance/MoveConstructorInitCheck.cpp @@ -0,0 +1,110 @@ +//===--- MoveConstructorInitCheck.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 "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 performance { + +MoveConstructorInitCheck::MoveConstructorInitCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeStyle(utils::IncludeSorter::parseIncludeStyle( + Options.getLocalOrGlobal("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. + if (!getLangOpts().CPlusPlus11) + return; + + Finder->addMatcher( + cxxConstructorDecl( + unless(isImplicit()), + allOf(isMoveConstructor(), + hasAnyConstructorInitializer( + cxxCtorInitializer( + withInitializer(cxxConstructExpr(hasDeclaration( + cxxConstructorDecl(isCopyConstructor()) + .bind("ctor"))))) + .bind("move-init")))), + this); +} + +void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) { + const auto *CopyCtor = Result.Nodes.getNodeAs("ctor"); + const auto *Initializer = + Result.Nodes.getNodeAs("move-init"); + + // Do not diagnose if the expression used to perform the initialization is a + // trivially-copyable type. + QualType QT = Initializer->getInit()->getType(); + if (QT.isTriviallyCopyableType(*Result.Context)) + return; + + if (QT.isConstQualified()) + return; + + const auto *RD = QT->getAsCXXRecordDecl(); + if (RD && RD->isTriviallyCopyable()) + return; + + // Diagnose when the class type has a move constructor available, but the + // ctor-initializer uses the copy constructor instead. + const CXXConstructorDecl *Candidate = nullptr; + for (const auto *Ctor : CopyCtor->getParent()->ctors()) { + if (Ctor->isMoveConstructor() && Ctor->getAccess() <= AS_protected && + !Ctor->isDeleted()) { + // The type has a move constructor that is at least accessible to the + // initializer. + // + // FIXME: Determine whether the move constructor is a viable candidate + // for the ctor-initializer, perhaps provide a fixit that suggests + // using std::move(). + Candidate = Ctor; + break; + } + } + + if (Candidate) { + // There's a move constructor candidate that the caller probably intended + // to call instead. + diag(Initializer->getSourceLocation(), + "move constructor initializes %0 by calling a copy constructor") + << (Initializer->isBaseInitializer() ? "base class" : "class member"); + diag(CopyCtor->getLocation(), "copy constructor being called", + DiagnosticIDs::Note); + diag(Candidate->getLocation(), "candidate move constructor here", + DiagnosticIDs::Note); + } +} + +void MoveConstructorInitCheck::registerPPCallbacks(CompilerInstance &Compiler) { + Inserter.reset(new utils::IncludeInserter( + Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle)); + Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks()); +} + +void MoveConstructorInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", + utils::IncludeSorter::toString(IncludeStyle)); +} + +} // namespace performance +} // namespace tidy +} // namespace clang Index: clang-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tidy/performance/PerformanceTidyModule.cpp +++ clang-tidy/performance/PerformanceTidyModule.cpp @@ -13,8 +13,10 @@ #include "FasterStringFindCheck.h" #include "ForRangeCopyCheck.h" #include "ImplicitConversionInLoopCheck.h" +#include "InefficientAlgorithmCheck.h" #include "InefficientStringConcatenationCheck.h" #include "InefficientVectorOperationCheck.h" +#include "MoveConstructorInitCheck.h" #include "TypePromotionInMathFnCheck.h" #include "UnnecessaryCopyInitialization.h" #include "UnnecessaryValueParamCheck.h" @@ -32,10 +34,14 @@ "performance-for-range-copy"); CheckFactories.registerCheck( "performance-implicit-conversion-in-loop"); + CheckFactories.registerCheck( + "performance-inefficient-algorithm"); CheckFactories.registerCheck( "performance-inefficient-string-concatenation"); CheckFactories.registerCheck( "performance-inefficient-vector-operation"); + CheckFactories.registerCheck( + "performance-move-constructor-init"); CheckFactories.registerCheck( "performance-type-promotion-in-math-fn"); CheckFactories.registerCheck( Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,6 +57,12 @@ Improvements to clang-tidy -------------------------- +- The 'misc-move-constructor-init' check was renamed to `performance-move-constructor-init + `_ + +- The 'misc-inefficient-algorithm' check was renamed to `performance-inefficient-algorithm + `_ + - The 'misc-virtual-near-miss' check was renamed to `bugprone-virtual-near-miss `_ Index: docs/clang-tidy/checks/cert-oop11-cpp.rst =================================================================== --- docs/clang-tidy/checks/cert-oop11-cpp.rst +++ docs/clang-tidy/checks/cert-oop11-cpp.rst @@ -1,10 +1,10 @@ -.. title:: clang-tidy - cert-oop11-cpp -.. meta:: - :http-equiv=refresh: 5;URL=misc-move-constructor-init.html - -cert-oop11-cpp -============== - -The cert-oop11-cpp check is an alias, please see -`misc-move-constructor-init `_ for more -information. +.. title:: clang-tidy - cert-oop11-cpp +.. meta:: + :http-equiv=refresh: 5;URL=performance-move-constructor-init.html + +cert-oop11-cpp +============== + +The cert-oop11-cpp check is an alias, please see +`performance-move-constructor-init `_ +for more information. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -51,7 +51,7 @@ cert-flp30-c cert-msc30-c (redirects to cert-msc50-cpp) cert-msc50-cpp - cert-oop11-cpp (redirects to misc-move-constructor-init) + cert-oop11-cpp (redirects to performance-move-constructor-init) cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) cppcoreguidelines-interfaces-global-init cppcoreguidelines-no-malloc @@ -119,14 +119,12 @@ misc-definitions-in-headers misc-forwarding-reference-overload misc-incorrect-roundings - misc-inefficient-algorithm misc-lambda-function-name misc-macro-parentheses misc-macro-repeated-side-effects misc-misplaced-const misc-misplaced-widening-cast misc-move-const-arg - misc-move-constructor-init misc-new-delete-overloads misc-noexcept-move-constructor misc-non-copyable-objects @@ -181,8 +179,10 @@ performance-faster-string-find performance-for-range-copy performance-implicit-conversion-in-loop + performance-inefficient-algorithm performance-inefficient-string-concatenation performance-inefficient-vector-operation + performance-move-constructor-init performance-type-promotion-in-math-fn performance-unnecessary-copy-initialization performance-unnecessary-value-param Index: docs/clang-tidy/checks/misc-inefficient-algorithm.rst =================================================================== --- docs/clang-tidy/checks/misc-inefficient-algorithm.rst +++ docs/clang-tidy/checks/misc-inefficient-algorithm.rst @@ -1,29 +0,0 @@ -.. title:: clang-tidy - misc-inefficient-algorithm - -misc-inefficient-algorithm -========================== - - -Warns on inefficient use of STL algorithms on associative containers. - -Associative containers implements some of the algorithms as methods which -should be preferred to the algorithms in the algorithm header. The methods -can take advanatage of the order of the elements. - -.. code-block:: c++ - - std::set s; - auto it = std::find(s.begin(), s.end(), 43); - - // becomes - - auto it = s.find(43); - -.. code-block:: c++ - - std::set s; - auto c = std::count(s.begin(), s.end(), 43); - - // becomes - - auto c = s.count(43); 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 @@ -1,18 +0,0 @@ -.. title:: clang-tidy - misc-move-constructor-init - -misc-move-constructor-init -========================== - -"cert-oop11-cpp" redirects here as an alias for this check. - -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. - -Options -------- - -.. option:: IncludeStyle - - A string specifying which include-style is used, `llvm` or `google`. Default - is `llvm`. Index: docs/clang-tidy/checks/performance-inefficient-algorithm.rst =================================================================== --- docs/clang-tidy/checks/performance-inefficient-algorithm.rst +++ docs/clang-tidy/checks/performance-inefficient-algorithm.rst @@ -0,0 +1,29 @@ +.. title:: clang-tidy - performance-inefficient-algorithm + +performance-inefficient-algorithm +================================= + + +Warns on inefficient use of STL algorithms on associative containers. + +Associative containers implements some of the algorithms as methods which +should be preferred to the algorithms in the algorithm header. The methods +can take advanatage of the order of the elements. + +.. code-block:: c++ + + std::set s; + auto it = std::find(s.begin(), s.end(), 43); + + // becomes + + auto it = s.find(43); + +.. code-block:: c++ + + std::set s; + auto c = std::count(s.begin(), s.end(), 43); + + // becomes + + auto c = s.count(43); Index: docs/clang-tidy/checks/performance-move-constructor-init.rst =================================================================== --- docs/clang-tidy/checks/performance-move-constructor-init.rst +++ docs/clang-tidy/checks/performance-move-constructor-init.rst @@ -0,0 +1,18 @@ +.. title:: clang-tidy - performance-move-constructor-init + +performance-move-constructor-init +================================= + +"cert-oop11-cpp" redirects here as an alias for this check. + +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. + +Options +------- + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or `google`. Default + is `llvm`. Index: test/clang-tidy/cert-oop11-cpp.cpp =================================================================== --- test/clang-tidy/cert-oop11-cpp.cpp +++ test/clang-tidy/cert-oop11-cpp.cpp @@ -16,6 +16,6 @@ // This should not produce a diagnostic because it is not covered under // the CERT guideline for OOP11-CPP. However, this will produce a diagnostic - // under misc-move-constructor-init. + // under performance-move-constructor-init. D(B b) : b(b) {} }; Index: test/clang-tidy/misc-inefficient-algorithm.cpp =================================================================== --- test/clang-tidy/misc-inefficient-algorithm.cpp +++ test/clang-tidy/misc-inefficient-algorithm.cpp @@ -1,166 +0,0 @@ -// RUN: %check_clang_tidy %s misc-inefficient-algorithm %t - -namespace std { -template struct less { - bool operator()(const T &lhs, const T &rhs) { return lhs < rhs; } -}; - -template struct greater { - bool operator()(const T &lhs, const T &rhs) { return lhs > rhs; } -}; - -struct iterator_type {}; - -template > struct set { - typedef iterator_type iterator; - iterator find(const K &k); - unsigned count(const K &k); - - iterator begin(); - iterator end(); - iterator begin() const; - iterator end() const; -}; - -struct other_iterator_type {}; - -template > struct map { - typedef other_iterator_type iterator; - iterator find(const K &k); - unsigned count(const K &k); - - iterator begin(); - iterator end(); - iterator begin() const; - iterator end() const; -}; - -template struct multimap : map {}; -template struct unordered_set : set {}; -template struct unordered_map : map {}; -template struct unordered_multiset : set {}; -template struct unordered_multimap : map {}; - -template > struct multiset : set {}; - -template -FwIt find(FwIt, FwIt end, const K &) { return end; } - -template -FwIt find(FwIt, FwIt end, const K &, Cmp) { return end; } - -template -FwIt find_if(FwIt, FwIt end, Pred) { return end; } - -template -unsigned count(FwIt, FwIt, const K &) { return 0; } - -template -FwIt lower_bound(FwIt, FwIt end, const K &) { return end; } - -template -FwIt lower_bound(FwIt, FwIt end, const K &, Ord) { return end; } -} - -#define FIND_IN_SET(x) find(x.begin(), x.end(), 10) -// CHECK-FIXES: #define FIND_IN_SET(x) find(x.begin(), x.end(), 10) - -template void f(const T &t) { - std::set s; - find(s.begin(), s.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}s.find(46);{{$}} - - find(t.begin(), t.end(), 46); - // CHECK-FIXES: {{^ }}find(t.begin(), t.end(), 46);{{$}} -} - -int main() { - std::set s; - auto it = std::find(s.begin(), s.end(), 43); - // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: this STL algorithm call should be replaced with a container method [misc-inefficient-algorithm] - // CHECK-FIXES: {{^ }}auto it = s.find(43);{{$}} - auto c = count(s.begin(), s.end(), 43); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}auto c = s.count(43);{{$}} - -#define SECOND(x, y, z) y - SECOND(q,std::count(s.begin(), s.end(), 22),w); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}SECOND(q,s.count(22),w);{{$}} - - it = find_if(s.begin(), s.end(), [](int) { return false; }); - - std::multiset ms; - find(ms.begin(), ms.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}ms.find(46);{{$}} - - const std::multiset &msref = ms; - find(msref.begin(), msref.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}msref.find(46);{{$}} - - std::multiset *msptr = &ms; - find(msptr->begin(), msptr->end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}msptr->find(46);{{$}} - - it = std::find(s.begin(), s.end(), 43, std::greater()); - // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: different comparers used in the algorithm and the container [misc-inefficient-algorithm] - - FIND_IN_SET(s); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}FIND_IN_SET(s);{{$}} - - f(s); - - std::unordered_set us; - lower_bound(us.begin(), us.end(), 10); - // CHECK-FIXES: {{^ }}lower_bound(us.begin(), us.end(), 10);{{$}} - find(us.begin(), us.end(), 10); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}us.find(10);{{$}} - - std::unordered_multiset ums; - find(ums.begin(), ums.end(), 10); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}ums.find(10);{{$}} - - std::map intmap; - find(intmap.begin(), intmap.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}find(intmap.begin(), intmap.end(), 46);{{$}} - - std::multimap intmmap; - find(intmmap.begin(), intmmap.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}find(intmmap.begin(), intmmap.end(), 46);{{$}} - - std::unordered_map umap; - find(umap.begin(), umap.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}find(umap.begin(), umap.end(), 46);{{$}} - - std::unordered_multimap ummap; - find(ummap.begin(), ummap.end(), 46); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}find(ummap.begin(), ummap.end(), 46);{{$}} -} - -struct Value { - int value; -}; - -struct Ordering { - bool operator()(const Value &lhs, const Value &rhs) const { - return lhs.value < rhs.value; - } - bool operator()(int lhs, const Value &rhs) const { return lhs < rhs.value; } -}; - -void g(std::set container, int value) { - lower_bound(container.begin(), container.end(), value, Ordering()); - // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be - // CHECK-FIXES: {{^ }}lower_bound(container.begin(), container.end(), value, Ordering());{{$}} -} 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 @@ -1,154 +0,0 @@ -// RUN: %check_clang_tidy %s misc-move-constructor-init,modernize-pass-by-value %t -- \ -// RUN: -config='{CheckOptions: \ -// RUN: [{key: modernize-pass-by-value.ValuesOnly, value: 1}]}' \ -// RUN: -- -std=c++11 -isystem %S/Inputs/Headers - -#include - -// CHECK-FIXES: #include - -template struct remove_reference {typedef T type;}; -template struct remove_reference {typedef T type;}; -template struct remove_reference {typedef T type;}; - -template -typename remove_reference::type&& move(T&& arg) { - return static_cast::type&&>(arg); -} - -struct C { - C() = default; - C(const C&) = default; -}; - -struct B { - B() {} - B(const B&) {} - B(B &&) {} -}; - -struct D : B { - D() : B() {} - D(const D &RHS) : B(RHS) {} - // CHECK-MESSAGES: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [misc-move-constructor-init] - // CHECK-MESSAGES: 26:3: note: copy constructor being called - // CHECK-MESSAGES: 27:3: note: candidate move constructor here - D(D &&RHS) : B(RHS) {} -}; - -struct E : B { - E() : B() {} - E(const E &RHS) : B(RHS) {} - E(E &&RHS) : B(move(RHS)) {} // ok -}; - -struct F { - C M; - - F(F &&) : M(C()) {} // ok -}; - -struct G { - G() = default; - G(const G&) = default; - G(G&&) = delete; -}; - -struct H : G { - H() = default; - H(const H&) = default; - H(H &&RHS) : G(RHS) {} // ok -}; - -struct I { - I(const I &) = default; // suppresses move constructor creation -}; - -struct J : I { - J(J &&RHS) : I(RHS) {} // ok -}; - -struct K {}; // Has implicit copy and move constructors, is trivially copyable -struct L : K { - L(L &&RHS) : K(RHS) {} // ok -}; - -struct M { - B Mem; - // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [misc-move-constructor-init] - M(M &&RHS) : Mem(RHS.Mem) {} -}; - -struct N { - B Mem; - N(N &&RHS) : Mem(move(RHS.Mem)) {} -}; - -struct O { - O(O&& other) : b(other.b) {} // ok - const B b; -}; - -struct P { - P(O&& other) : b(other.b) {} // ok - B b; -}; - -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]]:12: warning: pass by value and use std::move [modernize-pass-by-value] - // CHECK-FIXES: Positive(Movable M) : M_(std::move(M)) {} - 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 { - // This const ref constructor isn't warned about because the ValuesOnly option is set. - 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: test/clang-tidy/performance-inefficient-algorithm.cpp =================================================================== --- test/clang-tidy/performance-inefficient-algorithm.cpp +++ test/clang-tidy/performance-inefficient-algorithm.cpp @@ -0,0 +1,166 @@ +// RUN: %check_clang_tidy %s performance-inefficient-algorithm %t + +namespace std { +template struct less { + bool operator()(const T &lhs, const T &rhs) { return lhs < rhs; } +}; + +template struct greater { + bool operator()(const T &lhs, const T &rhs) { return lhs > rhs; } +}; + +struct iterator_type {}; + +template > struct set { + typedef iterator_type iterator; + iterator find(const K &k); + unsigned count(const K &k); + + iterator begin(); + iterator end(); + iterator begin() const; + iterator end() const; +}; + +struct other_iterator_type {}; + +template > struct map { + typedef other_iterator_type iterator; + iterator find(const K &k); + unsigned count(const K &k); + + iterator begin(); + iterator end(); + iterator begin() const; + iterator end() const; +}; + +template struct multimap : map {}; +template struct unordered_set : set {}; +template struct unordered_map : map {}; +template struct unordered_multiset : set {}; +template struct unordered_multimap : map {}; + +template > struct multiset : set {}; + +template +FwIt find(FwIt, FwIt end, const K &) { return end; } + +template +FwIt find(FwIt, FwIt end, const K &, Cmp) { return end; } + +template +FwIt find_if(FwIt, FwIt end, Pred) { return end; } + +template +unsigned count(FwIt, FwIt, const K &) { return 0; } + +template +FwIt lower_bound(FwIt, FwIt end, const K &) { return end; } + +template +FwIt lower_bound(FwIt, FwIt end, const K &, Ord) { return end; } +} + +#define FIND_IN_SET(x) find(x.begin(), x.end(), 10) +// CHECK-FIXES: #define FIND_IN_SET(x) find(x.begin(), x.end(), 10) + +template void f(const T &t) { + std::set s; + find(s.begin(), s.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}s.find(46);{{$}} + + find(t.begin(), t.end(), 46); + // CHECK-FIXES: {{^ }}find(t.begin(), t.end(), 46);{{$}} +} + +int main() { + std::set s; + auto it = std::find(s.begin(), s.end(), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: this STL algorithm call should be replaced with a container method [performance-inefficient-algorithm] + // CHECK-FIXES: {{^ }}auto it = s.find(43);{{$}} + auto c = count(s.begin(), s.end(), 43); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}auto c = s.count(43);{{$}} + +#define SECOND(x, y, z) y + SECOND(q,std::count(s.begin(), s.end(), 22),w); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}SECOND(q,s.count(22),w);{{$}} + + it = find_if(s.begin(), s.end(), [](int) { return false; }); + + std::multiset ms; + find(ms.begin(), ms.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}ms.find(46);{{$}} + + const std::multiset &msref = ms; + find(msref.begin(), msref.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}msref.find(46);{{$}} + + std::multiset *msptr = &ms; + find(msptr->begin(), msptr->end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}msptr->find(46);{{$}} + + it = std::find(s.begin(), s.end(), 43, std::greater()); + // CHECK-MESSAGES: :[[@LINE-1]]:42: warning: different comparers used in the algorithm and the container [performance-inefficient-algorithm] + + FIND_IN_SET(s); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}FIND_IN_SET(s);{{$}} + + f(s); + + std::unordered_set us; + lower_bound(us.begin(), us.end(), 10); + // CHECK-FIXES: {{^ }}lower_bound(us.begin(), us.end(), 10);{{$}} + find(us.begin(), us.end(), 10); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}us.find(10);{{$}} + + std::unordered_multiset ums; + find(ums.begin(), ums.end(), 10); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}ums.find(10);{{$}} + + std::map intmap; + find(intmap.begin(), intmap.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}find(intmap.begin(), intmap.end(), 46);{{$}} + + std::multimap intmmap; + find(intmmap.begin(), intmmap.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}find(intmmap.begin(), intmmap.end(), 46);{{$}} + + std::unordered_map umap; + find(umap.begin(), umap.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}find(umap.begin(), umap.end(), 46);{{$}} + + std::unordered_multimap ummap; + find(ummap.begin(), ummap.end(), 46); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}find(ummap.begin(), ummap.end(), 46);{{$}} +} + +struct Value { + int value; +}; + +struct Ordering { + bool operator()(const Value &lhs, const Value &rhs) const { + return lhs.value < rhs.value; + } + bool operator()(int lhs, const Value &rhs) const { return lhs < rhs.value; } +}; + +void g(std::set container, int value) { + lower_bound(container.begin(), container.end(), value, Ordering()); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this STL algorithm call should be + // CHECK-FIXES: {{^ }}lower_bound(container.begin(), container.end(), value, Ordering());{{$}} +} Index: test/clang-tidy/performance-move-constructor-init.cpp =================================================================== --- test/clang-tidy/performance-move-constructor-init.cpp +++ test/clang-tidy/performance-move-constructor-init.cpp @@ -0,0 +1,154 @@ +// RUN: %check_clang_tidy %s performance-move-constructor-init,modernize-pass-by-value %t -- \ +// RUN: -config='{CheckOptions: \ +// RUN: [{key: modernize-pass-by-value.ValuesOnly, value: 1}]}' \ +// RUN: -- -std=c++11 -isystem %S/Inputs/Headers + +#include + +// CHECK-FIXES: #include + +template struct remove_reference {typedef T type;}; +template struct remove_reference {typedef T type;}; +template struct remove_reference {typedef T type;}; + +template +typename remove_reference::type&& move(T&& arg) { + return static_cast::type&&>(arg); +} + +struct C { + C() = default; + C(const C&) = default; +}; + +struct B { + B() {} + B(const B&) {} + B(B &&) {} +}; + +struct D : B { + D() : B() {} + D(const D &RHS) : B(RHS) {} + // CHECK-MESSAGES: :[[@LINE+3]]:16: warning: move constructor initializes base class by calling a copy constructor [performance-move-constructor-init] + // CHECK-MESSAGES: 26:3: note: copy constructor being called + // CHECK-MESSAGES: 27:3: note: candidate move constructor here + D(D &&RHS) : B(RHS) {} +}; + +struct E : B { + E() : B() {} + E(const E &RHS) : B(RHS) {} + E(E &&RHS) : B(move(RHS)) {} // ok +}; + +struct F { + C M; + + F(F &&) : M(C()) {} // ok +}; + +struct G { + G() = default; + G(const G&) = default; + G(G&&) = delete; +}; + +struct H : G { + H() = default; + H(const H&) = default; + H(H &&RHS) : G(RHS) {} // ok +}; + +struct I { + I(const I &) = default; // suppresses move constructor creation +}; + +struct J : I { + J(J &&RHS) : I(RHS) {} // ok +}; + +struct K {}; // Has implicit copy and move constructors, is trivially copyable +struct L : K { + L(L &&RHS) : K(RHS) {} // ok +}; + +struct M { + B Mem; + // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: move constructor initializes class member by calling a copy constructor [performance-move-constructor-init] + M(M &&RHS) : Mem(RHS.Mem) {} +}; + +struct N { + B Mem; + N(N &&RHS) : Mem(move(RHS.Mem)) {} +}; + +struct O { + O(O&& other) : b(other.b) {} // ok + const B b; +}; + +struct P { + P(O&& other) : b(other.b) {} // ok + B b; +}; + +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]]:12: warning: pass by value and use std::move [modernize-pass-by-value] + // CHECK-FIXES: Positive(Movable M) : M_(std::move(M)) {} + 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 { + // This const ref constructor isn't warned about because the ValuesOnly option is set. + 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_; +};