Index: clang-tidy/performance/CMakeLists.txt =================================================================== --- clang-tidy/performance/CMakeLists.txt +++ clang-tidy/performance/CMakeLists.txt @@ -1,6 +1,7 @@ set(LLVM_LINK_COMPONENTS support) add_clang_library(clangTidyPerformanceModule + EmplaceCheck.cpp FasterStringFindCheck.cpp ForRangeCopyCheck.cpp ImplicitCastInLoopCheck.cpp Index: clang-tidy/performance/EmplaceCheck.h =================================================================== --- /dev/null +++ clang-tidy/performance/EmplaceCheck.h @@ -0,0 +1,30 @@ +//===--- EmplaceCheck.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_PERFORMANCE_EMPLACE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_EMPLACE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace performance { + +class EmplaceCheck : public ClangTidyCheck { +public: + EmplaceCheck(StringRef Name, ClangTidyContext *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_EMPLACE_H Index: clang-tidy/performance/EmplaceCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/performance/EmplaceCheck.cpp @@ -0,0 +1,98 @@ +//===--- EmplaceCheck.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 "EmplaceCheck.h" +#include "clang/Lex/Lexer.h" + +namespace clang { +namespace tidy { +namespace performance { + +using namespace ast_matchers; + +EmplaceCheck::EmplaceCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + +void EmplaceCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + cxxMemberCallExpr( + on(expr(hasType(cxxRecordDecl(hasName("std::vector"))))), + callee(functionDecl(hasName("push_back"))), + hasArgument(0, cxxConstructExpr().bind("construct_expr"))) + .bind("call"), + this); +} + +namespace { + +/// \brief Get a StringRef representing a SourceRange. +StringRef getAsString(const SourceManager &SM, const LangOptions &LO, + SourceRange SR) { + if (SR.isInvalid()) + return {}; + return Lexer::getSourceText(Lexer::getAsCharRange(SR, SM, LO), SM, LO); +} + +/// \brief Return the precise end location for the given token. +/// +/// Use the offset to the last character in the token, instead of the offset +/// to the first character after the token. +SourceLocation getPreciseTokenEnd(const SourceManager &SM, + const LangOptions &LO, SourceLocation Loc) { + unsigned TokLen = Lexer::MeasureTokenLength(SM.getSpellingLoc(Loc), SM, LO); + return Loc.getLocWithOffset(TokLen - 1); +} + +/// \brief Return the range containing the given token. +SourceRange getPreciseTokenRange(const SourceManager &SM, const LangOptions &LO, + SourceLocation Loc) { + return {Loc, getPreciseTokenEnd(SM, LO, Loc)}; +} + +} // end anonymous namespace + +void EmplaceCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Call = Result.Nodes.getNodeAs("call"); + const auto *Cons = Result.Nodes.getNodeAs("construct_expr"); + + // We need this range to emit a fixit. + SourceRange ArgsR = Cons->getParenOrBraceRange(); + if (ArgsR.isInvalid()) + return; + + // Don't try to mess with callees that come from expansions. + SourceLocation CalleeStartLoc = Call->getExprLoc(); + if (CalleeStartLoc.isMacroID()) + return; + + // Don't try to forward initializer lists. + if (Cons->isListInitialization() || Cons->isStdInitListInitialization()) + return; + + const SourceManager &SM = *Result.SourceManager; + const LangOptions &LO = Result.Context->getLangOpts(); + + // Skip just past the left parenthesis in '(', ..., ')'. + ArgsR.setBegin(Lexer::getLocForEndOfToken(ArgsR.getBegin(), 0, SM, LO)); + + StringRef ArgsStr = getAsString(SM, LO, ArgsR); + if (!ArgsStr.data()) + return; + + SourceRange CalleeR = getPreciseTokenRange(SM, LO, CalleeStartLoc); + SourceRange ConstructR = Cons->getSourceRange(); + + diag(CalleeStartLoc, "Avoid a copy by using an emplace method") + << FixItHint::CreateReplacement(CalleeR, "emplace_back") + << FixItHint::CreateReplacement(ConstructR, ArgsStr); +} + +} // namespace performance +} // namespace tidy +} // namespace clang Index: clang-tidy/performance/PerformanceTidyModule.cpp =================================================================== --- clang-tidy/performance/PerformanceTidyModule.cpp +++ clang-tidy/performance/PerformanceTidyModule.cpp @@ -11,6 +11,7 @@ #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "EmplaceCheck.h" #include "FasterStringFindCheck.h" #include "ForRangeCopyCheck.h" #include "ImplicitCastInLoopCheck.h" @@ -24,6 +25,8 @@ class PerformanceModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "performance-emplace-into-containers"); CheckFactories.registerCheck( "performance-faster-string-find"); CheckFactories.registerCheck( Index: test/clang-tidy/performance-emplace-into-containers.cpp =================================================================== --- /dev/null +++ test/clang-tidy/performance-emplace-into-containers.cpp @@ -0,0 +1,57 @@ +// RUN: %check_clang_tidy %s performance-emplace-into-containers %t -- -- -std=c++11 + +namespace std { + +template +struct vector { + void push_back(const T &x); + void push_back(T &&x); + + template + void emplace_back(Args &&... args); +}; + +} // std + +struct X { + int x; + X() : x(0) {} + X(int x) : x(x) {} + X(int x, int y) : x(x + y) {} + X(const X &Obj) : x(Obj.x) {} +}; + +void f1() { + std::vector v1; + + v1.push_back(X()); + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: Avoid a copy {{.*}} + // CHECK-FIXES: v1.emplace_back() + + v1.push_back(X(/*a*/)); + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: Avoid a copy {{.*}} + // CHECK-FIXES: v1.emplace_back(/*a*/) + + v1.push_back(X(0, 0)); + // CHECK-MESSAGES: [[@LINE-1]]:6: warning: Avoid a copy {{.*}} + // CHECK-FIXES: v1.emplace_back(0, 0) + + v1/*a*/./*b*/push_back(/*c*/X(/*d*/0,/*e*/0/*f*/)/*g*/)/*h*/ ; + // CHECK-MESSAGES: [[@LINE-1]]:16: warning: Avoid a copy {{.*}} + // CHECK-FIXES: v1/*a*/./*b*/emplace_back(/*c*//*d*/0,/*e*/0/*f*//*g*/)/*h*/ ; + + // Do not try to deal with initializer lists. + v1.push_back({0, 0}); + + // Do not try to deal with functional casts. FIXME? + X x{0, 0}; + v1.push_back(X(x)); + v1.push_back(X(0)); + + // Do not try to deal with weird expansions. +#define M1 X(0, 0) +#define M2 push_back + v1.push_back(M1); + v1.M2(X(0, 0)); + v1.M2(M1); +}