[clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[baloghadamsoftware]

Summary

New option added to these three checks to be able to silence false positives on types that are intentionally passed by value or copied. Such types are e.g. intrusive reference counting pointer types like llvm::IntrusiveRefCntPtr. The new option is named WhiteListTypes and can contain a semicolon-separated list of names of these types. Regular expressions are allowed. Default is empty.

[lebedev.ri]

Thanks, that looks much better.
Please clang-format the patch.

This pattern is repeated at least 4 times now, i think some abstraction is needed.

clang-tidy/performance/ForRangeCopyCheck.cpp

49 ↗

(On Diff #167746)

I'm not sure what is auto here, please spell QualType.

50–53 ↗

(On Diff #167746)

llvm::any_of()

clang-tidy/performance/UnnecessaryCopyInitialization.cpp

96 ↗

(On Diff #167746)

Same

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

103 ↗

(On Diff #167746)

Same

[JonasToth]

clang-tidy/performance/ForRangeCopyCheck.cpp

49 ↗

(On Diff #167746)

And please elide const, as it is a value and values are not made const in llvm code (consistency for now)

50 ↗

(On Diff #167746)

This configuration should be used in the matchers. Please see cppcoreguidelines-no-malloc for an example on how to do it in the matchers. Having it there usually improves performance and is clearer.

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

110 ↗

(On Diff #167746)

please remove the const

docs/clang-tidy/checks/performance-for-range-copy.rst

31 ↗

(On Diff #167746)

Please give an example for regular expressions. There are many slightly different variations of them and not everyone might be familiar. I think one wildcard expression is enough.

The sentences miss some fill words i think. whitelisted types, The default is empty.

docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst

44 ↗

(On Diff #167746)

Same as other doc, same below

test/clang-tidy/performance-for-range-copy.cpp

1 ↗

(On Diff #167746)

I would prefer 2 test files. One with default configuration and one with the special whitelisting, same for the other checks

[baloghadamsoftware]

clang-tidy/performance/ForRangeCopyCheck.cpp

50 ↗

(On Diff #167746)

google-runtime-references has this in the check() function. It seems there is no common agreement where to put this. cppcoreguidelines-no-malloc is not a good example since it simple compares strings instead of matching regular expressions. I think here we should allow regular expressions. Then we would need a new matcher called matchesAnyName. Should I create it?

[JonasToth]

clang-tidy/performance/ForRangeCopyCheck.cpp

50 ↗

(On Diff #167746)

Yes, regex is not supported there, but the mechanism is the same. You can put the new matcher in this check for now, if it's generally usefully in other clang-tidy check we can easily migrate them later on.

My motiviation for wanting it in the matcher instead of the check is to reduce the amount of ineffective work. If the matcher already ignores these cases, we don't need to spend cycles doing the callbacks and all the machinery. Its not a strong opinion though, if you don't agree I am fine with it.

Do you want to provide many matchers? In principle one could make one big regex with (PATTERN1|PATTERN2|PATTERN3). What do you think?

[lebedev.ri]

clang-tidy/performance/ForRangeCopyCheck.cpp

50 ↗

(On Diff #167746)

You can put the new matcher in this check for now, if it's generally usefully in other clang-tidy check we can easily migrate them later on.

Since it's now used in 4 places, i think it should be clang-tidy/utils/Matchers.h

[JonasToth]

True

Am 02.10.2018 um 22:28 schrieb Roman Lebedev via Phabricator:

lebedev.ri added inline comments.

Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:50
+ const auto VarType = Var->getType();
+ if (std::find_if(WhiteListTypes.begin(), WhiteListTypes.end(),

+ [&](llvm::StringRef WhiteListType) {

JonasToth wrote:

baloghadamsoftware wrote:

JonasToth wrote:

lebedev.ri wrote:

llvm::any_of()

This configuration should be used in the matchers. Please see cppcoreguidelines-no-malloc for an example on how to do it in the matchers. Having it there usually improves performance and is clearer.

google-runtime-references has this in the check() function. It seems there is no common agreement where to put this. cppcoreguidelines-no-malloc is not a good example since it simple compares strings instead of matching regular expressions. I think here we should allow regular expressions. Then we would need a new matcher called matchesAnyName. Should I create it?

Yes, regex is not supported there, but the mechanism is the same. You can put the new matcher in this check for now, if it's generally usefully in other clang-tidy check we can easily migrate them later on.

My motiviation for wanting it in the matcher instead of the check is to reduce the amount of ineffective work. If the matcher already ignores these cases, we don't need to spend cycles doing the callbacks and all the machinery. Its not a strong opinion though, if you don't agree I am fine with it.

Do you want to provide many matchers? In principle one could make one big regex with (PATTERN1|PATTERN2|PATTERN3). What do you think?
You can put the new matcher in this check for now, if it's generally usefully in other clang-tidy check we can easily migrate them later on.

Since it's now used in 4 places, i think it should be clang-tidy/utils/Matchers.h

Repository:

rCTE Clang Tools Extra

https://reviews.llvm.org/D52727

[aaron.ballman]

clang-tidy/performance/ForRangeCopyCheck.h

43 ↗

(On Diff #167746)

Nit: can you name it AllowedTypes rather than WhiteListTypes? Use similar nomenclature ("allow" instead of "white") elsewhere as well.

[baloghadamsoftware]

Updated according to the comments.

[lebedev.ri]

Thanks looks even better, getting there.
There are some spurious whiteline changes.

clang-tidy/performance/ForRangeCopyCheck.cpp

53 ↗

(On Diff #168274)

Spurious newline

clang-tidy/utils/Matchers.cpp

20 ↗

(On Diff #168274)

if(NameList.empty())
  return false;

29 ↗

(On Diff #168274)

Why do you need to 'concatenate' all the regexes?
Why not simply match in a loop?

[baloghadamsoftware]

clang-tidy/utils/Matchers.cpp

20 ↗

(On Diff #168274)

false? But this functions returns Matcher<NamedDecl>.

29 ↗

(On Diff #168274)

This is what @JonasToth suggested. How to match them in a loop if the function returns a Matcher<NamedDecl>?

[lebedev.ri]

clang-tidy/utils/Matchers.cpp

20 ↗

(On Diff #168274)

Hm, then unless(anything()), or unless(anything()).matches(Node, Finder, Builder).

[lebedev.ri]

clang-tidy/utils/Matchers.cpp

18–19 ↗

(On Diff #168274)

Actually wait, what is this?
It should be something like

AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector<std::string> &, NameList) {
...

20 ↗

(On Diff #168274)

Are you *sure* it doesn't work? Have you tried?

29 ↗

(On Diff #168274)

Are you *sure* it doesn't work? Have you tried?

[lebedev.ri]

clang-tidy/utils/Matchers.cpp

1 ↗

(On Diff #168274)

Also, why is this not in a header?
I suspect that heavily pessimizes inlining.

[baloghadamsoftware]

clang-tidy/utils/Matchers.cpp

18–19 ↗

(On Diff #168274)

Taken from cppcoreguidelines-no-malloc as @JonasToth suggested.

[lebedev.ri]

clang-tidy/utils/Matchers.cpp

18–19 ↗

(On Diff #168274)

Ok, but as you can see in clang-tidy/utils/Matchers.h (and ASTMatchers.h), it's not really correct prototype.

[JonasToth]

clang-tidy/utils/Matchers.cpp

18–19 ↗

(On Diff #168274)

This is not a full matcher, but a utility function creating one. You can make a full matcher-class with the AST_MATCHER macros as well.
Given this now is a public (within clang-tidy) matcher, it should probably be a AST_MATCHER based matcher. The cppcoreguidelines-check can then be migrated.

[baloghadamsoftware]

Updated according to the comments.

[lebedev.ri]

That's it!
Hopefully last portion of nits.

clang-tidy/performance/UnnecessaryCopyInitialization.cpp

65–66 ↗

(On Diff #168646)

Does it matter whether we are calling matchers::isExpensiveToCopy() on the type, or on the canonical type?

clang-tidy/performance/UnnecessaryValueParamCheck.cpp

83–86 ↗

(On Diff #168646)

This is inconsistent with the rest of the matchers here.
I think you want to check isExpensiveToCopy() first, because that should be less expensive than regex.

clang-tidy/utils/Matchers.h

51 ↗

(On Diff #168646)

Please double-check that this does not result in a copy, and std::vector is passed as a reference.

53 ↗

(On Diff #168646)

for (const std::string &Name : NameList) {

53 ↗

(On Diff #168646)

Do we want to be explicit about early-return here?

[JonasToth]

clang-tidy/performance/UnnecessaryCopyInitialization.cpp

65–66 ↗

(On Diff #168646)

the canonical type does resolve all typedefs, which is what is desirable in this case.

clang-tidy/utils/Matchers.h

51 ↗

(On Diff #168646)

its const&, see clang/include/clang/ASTMatchers/ASTMatchersMacros.h line 139

53 ↗

(On Diff #168646)

it looks like a return llvm::any_of(NameList, []() { /* Match regex */});

[baloghadamsoftware]

Updated according to the comments.

[baloghadamsoftware]

clang-tidy/performance/UnnecessaryCopyInitialization.cpp

65–66 ↗

(On Diff #168646)

The real question is whether we want to match the canonical type to the list of allowed type names. I am not sure.

[JonasToth]

clang-tidy/performance/UnnecessaryCopyInitialization.cpp

65–66 ↗

(On Diff #168646)

Well, very long template names are impractical, even for something "simple" as std::string the name gets very long without the typedef. I would say the typedefs should be respected

[lebedev.ri]

LG other than the hasCanonicalType() vs hasType() question.

[baloghadamsoftware]

Matching of allowed types happens now on the top-level type, not the canonical one.

[lebedev.ri]

LG unless @JonasToth or @aaron.ballman has any further comments.

[JonasToth]

LG in principle, just the SmallVec thing could be done if you agree. I don't insist on it, but it looks like a performance benefit to me.

clang-tidy/performance/UnnecessaryCopyInitialization.h

41 ↗

(On Diff #168993)

I think these lists for the allowed types could even be SmallVectors, as I wouldn't expect so many entries.

[baloghadamsoftware]

In D52727#1261901, @JonasToth wrote:

LG in principle, just the SmallVec thing could be done if you agree. I don't insist on it, but it looks like a performance benefit to me.

I principally agree, but then I also have to duplicate the string list parser function in the options to make it work on SmallVector as well.

[JonasToth]

I see, probably not worth it. Its one-time effort anyway right?
LGTM

[baloghadamsoftware]

Maybe it should be done in a separate patch together for all checks using that utility function. So there would be no need for duplication but the original functions should be changed instead.

clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h

private:
   // Checks if the loop variable is a non-const value and whether only
   // const methods are invoked on it or whether it is only used as a const
   // reference argument. If so it suggests it be made a const reference.
   bool handleCopyIsOnlyConstReferenced(const VarDecl &LoopVar,
                                        const CXXForRangeStmt &ForRange,
                                        ASTContext &Context);
 
   const bool WarnOnAllAutoCopies;
+  const std::vector<std::string> AllowedTypes;
 };
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_FORRANGECOPYCHECK_H

clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp

 //===--- ForRangeCopyCheck.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 "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"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 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) {
   // Match loop variables that are not references or pointers or are already
   // 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"),
                      this);
 }
 
 void ForRangeCopyCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Var = Result.Nodes.getNodeAs<VarDecl>("loopVar");
+
   // Ignore code in macros since we can't place the fixes correctly.
   if (Var->getBeginLoc().isMacroID())
     return;
   if (handleConstValueCopy(*Var, *Result.Context))
     return;
   const auto *ForRange = Result.Nodes.getNodeAs<CXXForRangeStmt>("forRange");
   handleCopyIsOnlyConstReferenced(*Var, *ForRange, *Result.Context);
 }

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h

 // call whose object is guaranteed to outlive the variable's scope and suggests
 // to use a const reference.
 //
 // The check currently only understands a subset of variables that are
 // guaranteed to outlive the const reference returned, namely: const variables,
 // 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,
                                   bool IssueFix, const VarDecl *ObjectArg,
                                   ASTContext &Context);
   void handleCopyFromLocalVar(const VarDecl &NewVar, const VarDecl &OldVar,
                               const Stmt &BlockStmt, bool IssueFix,
                               ASTContext &Context);
+  const std::vector<std::string> AllowedTypes;
 };
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

 //===--- UnnecessaryCopyInitialization.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 "UnnecessaryCopyInitialization.h"
 
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 namespace {
 
 void recordFixes(const VarDecl &Var, ASTContext &Context,
                  DiagnosticBuilder &Diagnostic) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
   if (!Var.getType().isLocalConstQualified())
     Diagnostic << utils::fixit::changeVarDeclToConst(Var);
 }
 
 } // namespace
 
 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 =
       allOf(anyOf(ConstReference, isConstQualified()),
             unless(allOf(pointerType(), unless(pointerType(pointee(
                                             qualType(isConstQualified())))))));
 
   // Match method call expressions where the `this` argument is only used as
   // const, this will be checked in `check()` part. This returned const
   // reference is highly likely to outlive the local const reference of the
   // variable being declared. The assumption is that the const reference being
   // returned either points to a global static variable or to a member of the
   // called object.
   auto ConstRefReturningMethodCall =
       cxxMemberCallExpr(callee(cxxMethodDecl(returns(ConstReference))),
                         on(declRefExpr(to(varDecl().bind("objectArg")))));
   auto ConstRefReturningFunctionCall =
       callExpr(callee(functionDecl(returns(ConstReference))),
                unless(callee(cxxMethodDecl())));
 
-  auto localVarCopiedFrom = [](const internal::Matcher<Expr> &CopyCtorArg) {
+  auto localVarCopiedFrom = [this](const internal::Matcher<Expr> &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(
                                            hasDeclaration(cxxConstructorDecl(
                                                isCopyConstructor())),
                                            hasArgument(0, CopyCtorArg))
                                            .bind("ctorCall")))
                                .bind("newVarDecl")))

 
 void UnnecessaryCopyInitialization::check(
     const MatchFinder::MatchResult &Result) {
   const auto *NewVar = Result.Nodes.getNodeAs<VarDecl>("newVarDecl");
   const auto *OldVar = Result.Nodes.getNodeAs<VarDecl>("oldVarDecl");
   const auto *ObjectArg = Result.Nodes.getNodeAs<VarDecl>("objectArg");
   const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
   const auto *CtorCall = Result.Nodes.getNodeAs<CXXConstructExpr>("ctorCall");
+
   // Do not propose fixes if the DeclStmt has multiple VarDecls or in macros
   // since we cannot place them correctly.
   bool IssueFix =
       Result.Nodes.getNodeAs<DeclStmt>("declStmt")->isSingleDecl() &&
       !NewVar->getLocation().isMacroID();
 
   // A constructor that looks like T(const T& t, bool arg = false) counts as a
   // copy only when it is called with default arguments for the arguments after
void UnnecessaryCopyInitialization::handleCopyFromLocalVar(
   auto Diagnostic = diag(NewVar.getLocation(),
                          "local copy %0 of the variable %1 is never modified; "
                          "consider avoiding the copy")
                     << &NewVar << &OldVar;
   if (IssueFix)
     recordFixes(NewVar, Context, Diagnostic);
 }
 
+void UnnecessaryCopyInitialization::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowedTypes",
+                utils::options::serializeStringList(AllowedTypes));
+}
+
 } // namespace performance
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h

 private:
   void handleMoveFix(const ParmVarDecl &Var, const DeclRefExpr &CopyArgument,
                      const ASTContext &Context);
 
   llvm::DenseMap<const FunctionDecl *, FunctionParmMutationAnalyzer>
       MutationAnalyzers;
   std::unique_ptr<utils::IncludeInserter> Inserter;
   const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  const std::vector<std::string> AllowedTypes;
 };
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_VALUE_PARAM_H

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp

 //===--- UnnecessaryValueParamCheck.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 "UnnecessaryValueParamCheck.h"
 
 #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"
 #include "clang/Lex/Preprocessor.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {

 }
 
 } // namespace
 
 UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
     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)))))))),
   Finder->addMatcher(
       functionDecl(hasBody(stmt()), isDefinition(), unless(isImplicit()),
                    unless(cxxMethodDecl(anyOf(isOverride(), isFinal()))),
                    has(typeLoc(forEach(ExpensiveValueParamDecl))),
                    unless(isInstantiated()), decl().bind("functionDecl")),
       this);
 }
 
Inserter.reset(new utils::IncludeInserter(
       Compiler.getSourceManager(), Compiler.getLangOpts(), IncludeStyle));
   Compiler.getPreprocessor().addPPCallbacks(Inserter->CreatePPCallbacks());
 }
 
 void UnnecessaryValueParamCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IncludeStyle",
                 utils::IncludeSorter::toString(IncludeStyle));
+  Options.store(Opts, "AllowedTypes",
+                utils::options::serializeStringList(AllowedTypes));
 }
 
 void UnnecessaryValueParamCheck::onEndOfTranslationUnit() {
   MutationAnalyzers.clear();
 }
 
 void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
                                                const DeclRefExpr &CopyArgument,

clang-tools-extra/trunk/clang-tidy/utils/Matchers.h

 }
 
 // Returns QualType matcher for references to const.
 AST_MATCHER_FUNCTION(ast_matchers::TypeMatcher, isReferenceToConst) {
   using namespace ast_matchers;
   return referenceType(pointee(qualType(isConstQualified())));
 }
 
+AST_MATCHER_P(NamedDecl, matchesAnyListedName, std::vector<std::string>,
+              NameList) {
+  return llvm::any_of(NameList, [&Node](const std::string &Name) {
+      return llvm::Regex(Name).match(Node.getName());
+    });
+}
+
 } // namespace matchers
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_MATCHERS_H

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-for-range-copy.rst

 
 Options
 -------
 
 .. option:: WarnOnAllAutoCopies
 
    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.

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst

void Function(const Foo& foo) {
     // The warning will suggest making this a const reference.
     string UnnecessaryCopy1 = foo.name();
     UnnecessaryCopy1.find("bar");
 
     // The warning will suggest making this a const reference.
     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.

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst

 
 Options
 -------
 
 .. option:: IncludeStyle
 
    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.

clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp

+// 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 <typename T>
+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 <typename T>
+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 <typename T> struct SomeComplexTemplate {
+  ~SomeComplexTemplate();
+};
+
+typedef SomeComplexTemplate<int> NotTooComplexRef;
+
+void negativeSmartPointer() {
+  for (auto P : View<Iterator<SmartPointer>>()) {
+    auto P2 = P;
+  }
+}
+
+void negative_smart_pointer() {
+  for (auto p : View<Iterator<smart_pointer>>()) {
+    auto p2 = p;
+  }
+}
+
+void negativeSmartPtr() {
+  for (auto P : View<Iterator<SmartPtr>>()) {
+    auto P2 = P;
+  }
+}
+
+void negative_smart_ptr() {
+  for (auto p : View<Iterator<smart_ptr>>()) {
+    auto p2 = p;
+  }
+}
+
+void negativeSmartReference() {
+  for (auto R : View<Iterator<SmartReference>>()) {
+    auto R2 = R;
+  }
+}
+
+void negative_smart_reference() {
+  for (auto r : View<Iterator<smart_reference>>()) {
+    auto r2 = r;
+  }
+}
+
+void negativeSmartRef() {
+  for (auto R : View<Iterator<SmartRef>>()) {
+    auto R2 = R;
+  }
+}
+
+void negative_smart_ref() {
+  for (auto r : View<Iterator<smart_ref>>()) {
+    auto r2 = r;
+  }
+}
+
+void positiveOtherType() {
+  for (auto O : View<Iterator<OtherType>>()) {
+  // 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<Iterator<OtherType>>()) {
+    auto O2 = O;
+  }
+}
+
+void negativeNotTooComplexRef() {
+  for (NotTooComplexRef R : View<Iterator<NotTooComplexRef>>()) {
+    auto R2 = R;
+  }
+}

clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp

+// 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 <typename T> struct SomeComplexTemplate {
+  ~SomeComplexTemplate();
+};
+
+typedef SomeComplexTemplate<int> 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.
+}

clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp

+// 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 <typename T> struct SomeComplexTemplate {
+  ~SomeComplexTemplate();
+};
+
+typedef SomeComplexTemplate<int> 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) {
+}