Index: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h +++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h @@ -40,6 +40,7 @@ ASTContext &Context); const bool WarnOnAllAutoCopies; + const std::vector AllowedTypes; }; } // namespace performance Index: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp @@ -10,6 +10,8 @@ #include "ForRangeCopyCheck.h" #include "../utils/DeclRefExprUtils.h" #include "../utils/FixItHintUtils.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" #include "../utils/TypeTraits.h" #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h" @@ -21,10 +23,14 @@ ForRangeCopyCheck::ForRangeCopyCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)) {} + WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)), + AllowedTypes( + utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} void ForRangeCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "WarnOnAllAutoCopies", WarnOnAllAutoCopies); + Options.store(Opts, "AllowedTypes", + utils::options::serializeStringList(AllowedTypes)); } void ForRangeCopyCheck::registerMatchers(MatchFinder *Finder) { @@ -32,7 +38,10 @@ // initialized through MaterializeTemporaryExpr which indicates a type // conversion. auto LoopVar = varDecl( - hasType(hasCanonicalType(unless(anyOf(referenceType(), pointerType())))), + hasType(qualType( + unless(anyOf(hasCanonicalType(anyOf(referenceType(), pointerType())), + hasDeclaration(namedDecl( + matchers::matchesAnyListedName(AllowedTypes))))))), unless(hasInitializer(expr(hasDescendant(materializeTemporaryExpr()))))); Finder->addMatcher(cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar"))) .bind("forRange"), @@ -41,6 +50,7 @@ void ForRangeCopyCheck::check(const MatchFinder::MatchResult &Result) { const auto *Var = Result.Nodes.getNodeAs("loopVar"); + // Ignore code in macros since we can't place the fixes correctly. if (Var->getBeginLoc().isMacroID()) return; Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h @@ -26,10 +26,10 @@ // const references, and const pointers to const. class UnnecessaryCopyInitialization : public ClangTidyCheck { public: - UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + UnnecessaryCopyInitialization(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: void handleCopyFromMethodReturn(const VarDecl &Var, const Stmt &BlockStmt, @@ -38,6 +38,7 @@ void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar, const Stmt &BlockStmt, bool IssueFix, ASTContext &Context); + const std::vector AllowedTypes; }; } // namespace performance Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp @@ -12,6 +12,7 @@ #include "../utils/DeclRefExprUtils.h" #include "../utils/FixItHintUtils.h" #include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" namespace clang { namespace tidy { @@ -30,6 +31,12 @@ using namespace ::clang::ast_matchers; using utils::decl_ref_expr::isOnlyUsedAsConst; +UnnecessaryCopyInitialization::UnnecessaryCopyInitialization( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + AllowedTypes( + utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} + void UnnecessaryCopyInitialization::registerMatchers(MatchFinder *Finder) { auto ConstReference = referenceType(pointee(qualType(isConstQualified()))); auto ConstOrConstReference = @@ -50,12 +57,17 @@ callExpr(callee(functionDecl(returns(ConstReference))), unless(callee(cxxMethodDecl()))); - auto localVarCopiedFrom = [](const internal::Matcher &CopyCtorArg) { + auto localVarCopiedFrom = [this](const internal::Matcher &CopyCtorArg) { return compoundStmt( forEachDescendant( declStmt( has(varDecl(hasLocalStorage(), - hasType(matchers::isExpensiveToCopy()), + hasType(qualType( + allOf(hasCanonicalType( + matchers::isExpensiveToCopy()), + unless(hasDeclaration(namedDecl( + matchers::matchesAnyListedName( + AllowedTypes))))))), unless(isImplicit()), hasInitializer( cxxConstructExpr( @@ -84,6 +96,7 @@ const auto *ObjectArg = Result.Nodes.getNodeAs("objectArg"); const auto *BlockStmt = Result.Nodes.getNodeAs("blockStmt"); const auto *CtorCall = Result.Nodes.getNodeAs("ctorCall"); + // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros // since we cannot place them correctly. bool IssueFix = @@ -144,6 +157,12 @@ recordFixes(NewVar, Context, Diagnostic); } +void UnnecessaryCopyInitialization::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "AllowedTypes", + utils::options::serializeStringList(AllowedTypes)); +} + } // namespace performance } // namespace tidy } // namespace clang Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h @@ -40,6 +40,7 @@ MutationAnalyzers; std::unique_ptr Inserter; const utils::IncludeSorter::IncludeStyle IncludeStyle; + const std::vector AllowedTypes; }; } // namespace performance Index: clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp @@ -12,6 +12,7 @@ #include "../utils/DeclRefExprUtils.h" #include "../utils/FixItHintUtils.h" #include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" #include "../utils/TypeTraits.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Lex/Lexer.h" @@ -68,17 +69,22 @@ StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), IncludeStyle(utils::IncludeSorter::parseIncludeStyle( - Options.getLocalOrGlobal("IncludeStyle", "llvm"))) {} + Options.getLocalOrGlobal("IncludeStyle", "llvm"))), + AllowedTypes( + utils::options::parseStringList(Options.get("AllowedTypes", ""))) {} void UnnecessaryValueParamCheck::registerMatchers(MatchFinder *Finder) { // This check is specific to C++ and doesn't apply to languages like // Objective-C. if (!getLangOpts().CPlusPlus) return; - const auto ExpensiveValueParamDecl = - parmVarDecl(hasType(hasCanonicalType(allOf( - unless(referenceType()), matchers::isExpensiveToCopy()))), - decl().bind("param")); + const auto ExpensiveValueParamDecl = parmVarDecl( + hasType(qualType(allOf( + hasCanonicalType(matchers::isExpensiveToCopy()), + unless(anyOf(hasCanonicalType(referenceType()), + hasDeclaration(namedDecl( + matchers::matchesAnyListedName(AllowedTypes)))))))), + decl().bind("param")); Finder->addMatcher( functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()), unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))), @@ -173,6 +179,8 @@ ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IncludeStyle", utils::IncludeSorter::toString(IncludeStyle)); + Options.store(Opts, "AllowedTypes", + utils::options::serializeStringList(AllowedTypes)); } void UnnecessaryValueParamCheck::onEndOfTranslationUnit() { Index: clang-tools-extra/trunk/clang-tidy/utils/Matchers.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/utils/Matchers.h +++ clang-tools-extra/trunk/clang-tidy/utils/Matchers.h @@ -48,6 +48,13 @@ return referenceType(pointee(qualType(isConstQualified()))); } +AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector, + NameList) { + return llvm::any_of(NameList, [&Node](const std::string &Name) { + return llvm::Regex(Name).match(Node.getName()); + }); +} + } // namespace matchers } // namespace tidy } // namespace clang Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-for-range-copy.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-for-range-copy.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-for-range-copy.rst @@ -25,3 +25,10 @@ When non-zero, warns on any use of `auto` as the type of the range-based for loop variable. Default is `0`. + +.. option:: AllowedTypes + + A semicolon-separated list of names of types allowed to be copied in each + iteration. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches + every type with suffix `Ref`, `ref`, `Reference` and `reference`. The default + is empty. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst @@ -35,3 +35,13 @@ string UnnecessaryCopy2 = UnnecessaryCopy1; UnnecessaryCopy2.find("bar"); } + +Options +------- + +.. option:: AllowedTypes + + A semicolon-separated list of names of types allowed to be initialized by + copying. Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches + every type with suffix `Ref`, `ref`, `Reference` and `reference`. The + default is empty. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst @@ -61,3 +61,9 @@ A string specifying which include-style is used, `llvm` or `google`. Default is `llvm`. + +.. option:: AllowedTypes + + A semicolon-separated list of names of types allowed to be passed by value. + Regular expressions are accepted, e.g. `[Rr]ef(erence)?$` matches every type + with suffix `Ref`, `ref`, `Reference` and `reference`. The default is empty. Index: clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp @@ -0,0 +1,124 @@ +// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" -- -std=c++11 -fno-delayed-template-parsing + +template +struct Iterator { + void operator++() {} + const T& operator*() { + static T* TT = new T(); + return *TT; + } + bool operator!=(const Iterator &) { return false; } + typedef const T& const_reference; +}; +template +struct View { + T begin() { return T(); } + T begin() const { return T(); } + T end() { return T(); } + T end() const { return T(); } + typedef typename T::const_reference const_reference; +}; + +struct SmartPointer { + ~SmartPointer(); +}; + +struct smart_pointer { + ~smart_pointer(); +}; + +struct SmartPtr { + ~SmartPtr(); +}; + +struct smart_ptr { + ~smart_ptr(); +}; + +struct SmartReference { + ~SmartReference(); +}; + +struct smart_reference { + ~smart_reference(); +}; + +struct SmartRef { + ~SmartRef(); +}; + +struct smart_ref { + ~smart_ref(); +}; + +struct OtherType { + ~OtherType(); +}; + +template struct SomeComplexTemplate { + ~SomeComplexTemplate(); +}; + +typedef SomeComplexTemplate NotTooComplexRef; + +void negativeSmartPointer() { + for (auto P : View>()) { + auto P2 = P; + } +} + +void negative_smart_pointer() { + for (auto p : View>()) { + auto p2 = p; + } +} + +void negativeSmartPtr() { + for (auto P : View>()) { + auto P2 = P; + } +} + +void negative_smart_ptr() { + for (auto p : View>()) { + auto p2 = p; + } +} + +void negativeSmartReference() { + for (auto R : View>()) { + auto R2 = R; + } +} + +void negative_smart_reference() { + for (auto r : View>()) { + auto r2 = r; + } +} + +void negativeSmartRef() { + for (auto R : View>()) { + auto R2 = R; + } +} + +void negative_smart_ref() { + for (auto r : View>()) { + auto r2 = r; + } +} + +void positiveOtherType() { + for (auto O : View>()) { + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy] + // CHECK-FIXES: for (const auto& O : View>()) { + auto O2 = O; + } +} + +void negativeNotTooComplexRef() { + for (NotTooComplexRef R : View>()) { + auto R2 = R; + } +} Index: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp @@ -0,0 +1,98 @@ +// RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t -- -config="{CheckOptions: [{key: performance-unnecessary-copy-initialization.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" -- + +struct SmartPointer { + ~SmartPointer(); +}; + +struct smart_pointer { + ~smart_pointer(); +}; + +struct SmartPtr { + ~SmartPtr(); +}; + +struct smart_ptr { + ~smart_ptr(); +}; + +struct SmartReference { + ~SmartReference(); +}; + +struct smart_reference { + ~smart_reference(); +}; + +struct SmartRef { + ~SmartRef(); +}; + +struct smart_ref { + ~smart_ref(); +}; + +struct OtherType { + ~OtherType(); +}; + +template struct SomeComplexTemplate { + ~SomeComplexTemplate(); +}; + +typedef SomeComplexTemplate NotTooComplexRef; + +const SmartPointer &getSmartPointer(); +const smart_pointer &get_smart_pointer(); +const SmartPtr &getSmartPtr(); +const smart_ptr &get_smart_ptr(); +const SmartReference &getSmartReference(); +const smart_reference &get_smart_reference(); +const SmartRef &getSmartRef(); +const smart_ref &get_smart_ref(); +const OtherType &getOtherType(); +const NotTooComplexRef &getNotTooComplexRef(); + +void negativeSmartPointer() { + const auto P = getSmartPointer(); +} + +void negative_smart_pointer() { + const auto p = get_smart_pointer(); +} + +void negativeSmartPtr() { + const auto P = getSmartPtr(); +} + +void negative_smart_ptr() { + const auto p = get_smart_ptr(); +} + +void negativeSmartReference() { + const auto R = getSmartReference(); +} + +void negative_smart_reference() { + const auto r = get_smart_reference(); +} + +void negativeSmartRef() { + const auto R = getSmartRef(); +} + +void negative_smart_ref() { + const auto r = get_smart_ref(); +} + +void positiveOtherType() { + const auto O = getOtherType(); + // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'O' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization] + // CHECK-FIXES: const auto& O = getOtherType(); +} + +void negativeNotTooComplexRef() { + const NotTooComplexRef R = getNotTooComplexRef(); + // Using `auto` here would result in the "canonical" type which does not match + // the pattern. +} Index: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp +++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp @@ -0,0 +1,75 @@ +// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" -- + +struct SmartPointer { + ~SmartPointer(); +}; + +struct smart_pointer { + ~smart_pointer(); +}; + +struct SmartPtr { + ~SmartPtr(); +}; + +struct smart_ptr { + ~smart_ptr(); +}; + +struct SmartReference { + ~SmartReference(); +}; + +struct smart_reference { + ~smart_reference(); +}; + +struct SmartRef { + ~SmartRef(); +}; + +struct smart_ref { + ~smart_ref(); +}; + +struct OtherType { + ~OtherType(); +}; + +template struct SomeComplexTemplate { + ~SomeComplexTemplate(); +}; + +typedef SomeComplexTemplate NotTooComplexRef; + +void negativeSmartPointer(SmartPointer P) { +} + +void negative_smart_pointer(smart_pointer p) { +} + +void negativeSmartPtr(SmartPtr P) { +} + +void negative_smart_ptr(smart_ptr p) { +} + +void negativeSmartReference(SmartReference R) { +} + +void negative_smart_reference(smart_reference r) { +} + +void negativeSmartRef(SmartRef R) { +} + +void negative_smart_ref(smart_ref r) { +} + +void positiveOtherType(OtherType O) { + // CHECK-MESSAGES: [[@LINE-1]]:34: warning: the parameter 'O' is copied for each invocation but only used as a const reference; consider making it a const reference [performance-unnecessary-value-param] + // CHECK-FIXES: void positiveOtherType(const OtherType& O) { +} + +void negativeNotTooComplexRef(NotTooComplexRef R) { +}