Extend UnnecessaryCopyInitialization check to trigger on non-const copies that can be safely converted to const references.

[flx]

Summary

Move code shared between UnnecessaryCopyInitialization and ForRangeCopyCheck into utilities files.
Add more test cases for UnnecessaryCopyInitialization and disable fixes inside of macros.

[alexfh]

clang-tidy/performance/UnnecessaryCopyInitialization.cpp

69 ↗

(On Diff #48607)

nit: Clang-format doesn't reflow string literals
One way to fix this is to merge all the lines of this string literal and clang-format again.

clang-tidy/performance/UnnecessaryCopyInitialization.h

19 ↗

(On Diff #48607)

Maybe just "The check detects local variable declarations ...."?

Also, should the .rst file be updated as well?

clang-tidy/utils/DeclRefExprUtils.h

21 ↗

(On Diff #48607)

nit: There should be an empty line after the brief summary, IIRC. (But you can look at the doxygen output and see how this looks like.)

clang-tidy/utils/FixItHintUtils.cpp

23 ↗

(On Diff #48607)

Lexer::getLocForEndOfToken seems to be more suitable for this purpose.

clang-tidy/utils/FixItHintUtils.h

21 ↗

(On Diff #48607)

Since this is now a library, we should make names clear and unambiguous. I'd pull a part of the name's meaning to the namespace and expand the name with the needed details. Maybe something like this:

namespace clang {
namespace tidy {
namespace utils {
namespace create_fixit {

FixItHint changeVarDeclToReference(...);
FixItHint changeVarDeclToConst(...);

[flx]

clang-tidy/performance/UnnecessaryCopyInitialization.h

19 ↗

(On Diff #48607)

.rst file didn't exist. Added one now.

clang-tidy/utils/FixItHintUtils.h

21 ↗

(On Diff #48607)

Done, but put an extra underscore in the create_fix_it namespace to be consistent with the camelcase of the class.

[alexfh]

Looks good. Thanks!

[alexfh]

clang-tidy/utils/FixItHintUtils.cpp

2 ↗

(On Diff #49306)

Please fix the line break.

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/LexerUtils.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/FixItHintUtils.h"
 #include "../utils/TypeTraits.h"
-#include "clang/Lex/Lexer.h"
-#include "llvm/ADT/SmallPtrSet.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 using namespace ::clang::ast_matchers;
-using llvm::SmallPtrSet;
-
-namespace {
-
-template <typename S> bool isSetDifferenceEmpty(const S &S1, const S &S2) {
-  for (const auto &E : S1)
-    if (S2.count(E) == 0)
-      return false;
-  return true;
-}
-
-// Extracts all Nodes keyed by ID from Matches and inserts them into Nodes.
-template <typename Node>
-void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
-                        SmallPtrSet<const Node *, 16> &Nodes) {
-  for (const auto &Match : Matches)
-    Nodes.insert(Match.getNodeAs<Node>(ID));
-}
-
-// Finds all DeclRefExprs to VarDecl in Stmt.
-SmallPtrSet<const DeclRefExpr *, 16>
-declRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context) {
-  auto Matches = match(
-      findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef")),
-      Stmt, Context);
-  SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  return DeclRefs;
-}
-
-// Finds all DeclRefExprs where a const method is called on VarDecl or VarDecl
-// is the a const reference or value argument to a CallExpr or CXXConstructExpr.
-SmallPtrSet<const DeclRefExpr *, 16>
-constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
-                           ASTContext &Context) {
-  auto DeclRefToVar =
-      declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
-  auto ConstMethodCallee = callee(cxxMethodDecl(isConst()));
-  // Match method call expressions where the variable is referenced as the this
-  // implicit object argument and opertor call expression for member operators
-  // where the variable is the 0-th argument.
-  auto Matches = match(
-      findAll(expr(anyOf(cxxMemberCallExpr(ConstMethodCallee, on(DeclRefToVar)),
-                         cxxOperatorCallExpr(ConstMethodCallee,
-                                             hasArgument(0, DeclRefToVar))))),
-      Stmt, Context);
-  SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  auto ConstReferenceOrValue =
-      qualType(anyOf(referenceType(pointee(qualType(isConstQualified()))),
-                     unless(anyOf(referenceType(), pointerType()))));
-  auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
-      DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValue)));
-  Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  Matches =
-      match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
-  extractNodesByIdTo(Matches, "declRef", DeclRefs);
-  return DeclRefs;
-}
-
-// Modifies VarDecl to be a reference.
-FixItHint createAmpersandFix(const VarDecl &VarDecl, ASTContext &Context) {
-  SourceLocation AmpLocation = VarDecl.getLocation();
-  auto Token = lexer_utils::getPreviousNonCommentToken(Context, AmpLocation);
-  if (!Token.is(tok::unknown))
-    AmpLocation = Token.getLocation().getLocWithOffset(Token.getLength());
-  return FixItHint::CreateInsertion(AmpLocation, "&");
-}
-
-// Modifies VarDecl to be const.
-FixItHint createConstFix(const VarDecl &VarDecl) {
-  return FixItHint::CreateInsertion(VarDecl.getTypeSpecStartLoc(), "const ");
-}
-
-} // namespace
 
 ForRangeCopyCheck::ForRangeCopyCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
       WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)) {}
 
 void ForRangeCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnAllAutoCopies", WarnOnAllAutoCopies);
 }
bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
   llvm::Optional<bool> Expensive =
       type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
   if (!Expensive || !*Expensive)
     return false;
   auto Diagnostic =
       diag(LoopVar.getLocation(),
            "the loop variable's type is not a reference type; this creates a "
            "copy in each iteration; consider making this a reference")
-      << createAmpersandFix(LoopVar, Context);
+      << utils::create_fix_it::changeVarDeclToReference(LoopVar, Context);
   if (!LoopVar.getType().isConstQualified())
-    Diagnostic << createConstFix(LoopVar);
+    Diagnostic << utils::create_fix_it::changeVarDeclToConst(LoopVar);
   return true;
 }
 
 bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
     const VarDecl &LoopVar, const CXXForRangeStmt &ForRange,
     ASTContext &Context) {
   llvm::Optional<bool> Expensive =
       type_traits::isExpensiveToCopy(LoopVar.getType(), Context);
-  if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive) {
+  if (LoopVar.getType().isConstQualified() || !Expensive || !*Expensive)
     return false;
-  }
-  // Collect all DeclRefExprs to the loop variable and all CallExprs and
-  // CXXConstructExprs where the loop variable is used as argument to a const
-  // reference parameter.
-  // If the difference is empty it is safe for the loop variable to be a const
-  // reference.
-  auto AllDeclRefs = declRefExprs(LoopVar, *ForRange.getBody(), Context);
-  auto ConstReferenceDeclRefs =
-      constReferenceDeclRefExprs(LoopVar, *ForRange.getBody(), Context);
-  if (AllDeclRefs.empty() ||
-      !isSetDifferenceEmpty(AllDeclRefs, ConstReferenceDeclRefs))
+  if (!decl_ref_expr_utils::isOnlyUsedAsConst(LoopVar, *ForRange.getBody(), Context))
     return false;
   diag(LoopVar.getLocation(),
        "loop variable is copied but only used as const reference; consider "
        "making it a const reference")
-      << createConstFix(LoopVar) << createAmpersandFix(LoopVar, Context);
+      << utils::create_fix_it::changeVarDeclToConst(LoopVar)
+      << utils::create_fix_it::changeVarDeclToReference(LoopVar, Context);
   return true;
 }
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang

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

 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_UNNECESSARY_COPY_INITIALIZATION_H
 
 #include "../ClangTidy.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
-// A check that detects const local variable declarations that are copy
-// initialized with the const reference of a function call or the const
-// reference of a method call whose object is guaranteed to outlive the
-// variable's scope and suggests to use a const reference.
+// The check detects local variable declarations that are copy initialized with
+// the const reference of a function call or the const reference of a method
+// 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) {}

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/LexerUtils.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 
 namespace clang {
 namespace tidy {
 namespace performance {
 
 using namespace ::clang::ast_matchers;
 
-namespace {
-AST_MATCHER(QualType, isPointerType) { return Node->isPointerType(); }
-} // namespace
-
 void UnnecessaryCopyInitialization::registerMatchers(
     ast_matchers::MatchFinder *Finder) {
   auto ConstReference = referenceType(pointee(qualType(isConstQualified())));
   auto ConstOrConstReference =
       allOf(anyOf(ConstReference, isConstQualified()),
-            unless(allOf(isPointerType(), unless(pointerType(pointee(qualType(
-                                              isConstQualified())))))));
+            unless(allOf(pointerType(), unless(pointerType(pointee(
+                                            qualType(isConstQualified())))))));
   // Match method call expressions where the this argument is a const
   // type or const reference. 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 ConstRefReturningMethodCallOfConstParam = cxxMemberCallExpr(
       callee(cxxMethodDecl(returns(ConstReference))),
       on(declRefExpr(to(varDecl(hasType(qualType(ConstOrConstReference)))))));
   auto ConstRefReturningFunctionCall =
       callExpr(callee(functionDecl(returns(ConstReference))),
                unless(callee(cxxMethodDecl())));
   Finder->addMatcher(
+      compoundStmt(
+          forEachDescendant(
               varDecl(
-          hasLocalStorage(), hasType(isConstQualified()),
-          hasType(matchers::isExpensiveToCopy()),
+                  hasLocalStorage(), hasType(matchers::isExpensiveToCopy()),
                   hasInitializer(cxxConstructExpr(
                       hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
-              hasArgument(0, anyOf(ConstRefReturningFunctionCall,
+                      hasArgument(
+                          0, anyOf(ConstRefReturningFunctionCall,
                                    ConstRefReturningMethodCallOfConstParam)))))
-          .bind("varDecl"),
+                  .bind("varDecl"))).bind("blockStmt"),
       this);
 }
 
 void UnnecessaryCopyInitialization::check(
     const ast_matchers::MatchFinder::MatchResult &Result) {
   const auto *Var = Result.Nodes.getNodeAs<VarDecl>("varDecl");
-  SourceLocation AmpLocation = Var->getLocation();
-  auto Token = lexer_utils::getPreviousNonCommentToken(*Result.Context,
-                                                       Var->getLocation());
-  if (!Token.is(tok::unknown)) {
-    AmpLocation = Token.getLocation().getLocWithOffset(Token.getLength());
-  }
+  const auto *BlockStmt = Result.Nodes.getNodeAs<Stmt>("blockStmt");
+  bool IsConstQualified = Var->getType().isConstQualified();
+  if (!IsConstQualified &&
+      !decl_ref_expr_utils::isOnlyUsedAsConst(*Var, *BlockStmt,
+                                              *Result.Context))
+    return;
+  DiagnosticBuilder Diagnostic =
       diag(Var->getLocation(),
-       "the const qualified variable '%0' is copy-constructed from a "
-       "const reference; consider making it a const reference")
-      << Var->getName() << FixItHint::CreateInsertion(AmpLocation, "&");
+           IsConstQualified ? "the const qualified variable '%0' is "
+                              "copy-constructed from a const reference; "
+                              "consider making it a const reference"
+                            : "the variable '%0' is copy-constructed from a "
+                              "const reference but is only used as const "
+                              "reference; consider making it a const reference")
+      << Var->getName();
+  // Do not propose fixes in macros since we cannot place them correctly.
+  if (Var->getLocStart().isMacroID())
+    return;
+  Diagnostic << utils::create_fix_it::changeVarDeclToReference(*Var,
+                                                               *Result.Context);
+  if (!IsConstQualified)
+    Diagnostic << utils::create_fix_it::changeVarDeclToConst(*Var);
 }
 
 } // namespace performance
 } // namespace tidy
 } // namespace clang

clang-tools-extra/trunk/clang-tidy/utils/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyUtils
+  DeclRefExprUtils.cpp
+  FixItHintUtils.cpp
   HeaderGuard.cpp
   HeaderFileExtensionsUtils.cpp
   IncludeInserter.cpp
   IncludeSorter.cpp
   LexerUtils.cpp
   TypeTraits.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   )

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

+//===--- DeclRefExprUtils.h - clang-tidy-------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
+
+namespace clang {
+namespace tidy {
+namespace decl_ref_expr_utils {
+
+/// \brief Returns true if all DeclRefExpr to the variable within Stmt do not
+/// modify it.
+///
+/// Returns true if only const methods or operators are called on the variable
+/// or the variable is a const reference or value argument to a callExpr().
+bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
+                       ASTContext &Context);
+
+} // namespace decl_ref_expr_utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_DECLREFEXPRUTILS_H

clang-tools-extra/trunk/clang-tidy/utils/DeclRefExprUtils.cpp

+//===--- DeclRefExprUtils.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 "DeclRefExprUtils.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
+namespace clang {
+namespace tidy {
+namespace decl_ref_expr_utils {
+
+using namespace ::clang::ast_matchers;
+using llvm::SmallPtrSet;
+
+namespace {
+
+template <typename S> bool isSetDifferenceEmpty(const S &S1, const S &S2) {
+  for (const auto &E : S1)
+    if (S2.count(E) == 0)
+      return false;
+  return true;
+}
+
+// Extracts all Nodes keyed by ID from Matches and inserts them into Nodes.
+template <typename Node>
+void extractNodesByIdTo(ArrayRef<BoundNodes> Matches, StringRef ID,
+                        SmallPtrSet<const Node *, 16> &Nodes) {
+  for (const auto &Match : Matches)
+    Nodes.insert(Match.getNodeAs<Node>(ID));
+}
+
+// Finds all DeclRefExprs to VarDecl in Stmt.
+SmallPtrSet<const DeclRefExpr *, 16>
+declRefExprs(const VarDecl &VarDecl, const Stmt &Stmt, ASTContext &Context) {
+  auto Matches = match(
+      findAll(declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef")),
+      Stmt, Context);
+  SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  return DeclRefs;
+}
+
+// Finds all DeclRefExprs where a const method is called on VarDecl or VarDecl
+// is the a const reference or value argument to a CallExpr or CXXConstructExpr.
+SmallPtrSet<const DeclRefExpr *, 16>
+constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
+                           ASTContext &Context) {
+  auto DeclRefToVar =
+      declRefExpr(to(varDecl(equalsNode(&VarDecl)))).bind("declRef");
+  auto ConstMethodCallee = callee(cxxMethodDecl(isConst()));
+  // Match method call expressions where the variable is referenced as the this
+  // implicit object argument and opertor call expression for member operators
+  // where the variable is the 0-th argument.
+  auto Matches = match(
+      findAll(expr(anyOf(cxxMemberCallExpr(ConstMethodCallee, on(DeclRefToVar)),
+                         cxxOperatorCallExpr(ConstMethodCallee,
+                                             hasArgument(0, DeclRefToVar))))),
+      Stmt, Context);
+  SmallPtrSet<const DeclRefExpr *, 16> DeclRefs;
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  auto ConstReferenceOrValue =
+      qualType(anyOf(referenceType(pointee(qualType(isConstQualified()))),
+                     unless(anyOf(referenceType(), pointerType()))));
+  auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
+      DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValue)));
+  Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  Matches =
+      match(findAll(cxxConstructExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
+  extractNodesByIdTo(Matches, "declRef", DeclRefs);
+  return DeclRefs;
+}
+
+} // namespace
+
+bool isOnlyUsedAsConst(const VarDecl &Var, const Stmt &Stmt,
+                       ASTContext &Context) {
+  // Collect all DeclRefExprs to the loop variable and all CallExprs and
+  // CXXConstructExprs where the loop variable is used as argument to a const
+  // reference parameter.
+  // If the difference is empty it is safe for the loop variable to be a const
+  // reference.
+  auto AllDeclRefs = declRefExprs(Var, Stmt, Context);
+  auto ConstReferenceDeclRefs = constReferenceDeclRefExprs(Var, Stmt, Context);
+  return isSetDifferenceEmpty(AllDeclRefs, ConstReferenceDeclRefs);
+}
+
+} // namespace decl_ref_expr_utils
+} // namespace tidy
+} // namespace clang

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

+//===--- FixItHintUtils.h - clang-tidy---------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+namespace create_fix_it {
+
+/// \brief Creates fix to make VarDecl a reference by adding '&'.
+FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context);
+
+/// \brief Creates fix to make VarDecl const qualified.
+FixItHint changeVarDeclToConst(const VarDecl &Var);
+
+} // namespace create_fix_it
+} // utils
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FIXITHINTUTILS_H

clang-tools-extra/trunk/clang-tidy/utils/FixItHintUtils.cpp

+//===--- FixItHintUtils.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 "FixItHintUtils.h"
+#include "LexerUtils.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace utils {
+namespace create_fix_it {
+
+FixItHint changeVarDeclToReference(const VarDecl &Var, ASTContext &Context) {
+  SourceLocation AmpLocation = Var.getLocation();
+  auto Token = lexer_utils::getPreviousNonCommentToken(Context, AmpLocation);
+  if (!Token.is(tok::unknown))
+    AmpLocation = Lexer::getLocForEndOfToken(Token.getLocation(), 0,
+                                             Context.getSourceManager(),
+                                             Context.getLangOpts());
+  return FixItHint::CreateInsertion(AmpLocation, "&");
+}
+
+FixItHint changeVarDeclToConst(const VarDecl &Var) {
+  return FixItHint::CreateInsertion(Var.getTypeSpecStartLoc(), "const ");
+}
+
+} // namespace create_fix_it
+} // namespace utils
+} // namespace tidy
+} // namespace clang

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

+.. title:: clang-tidy - performance-unnecessary-copy-initialization
+
+performance-unnecessary-copy-initialization
+===========================================
+
+Finds local variable declarations that are initialized using the copy
+constructor of a non-trivially-copyable type but it would suffice to obtain a
+const reference.
+
+The check is only applied if it is safe to replace the copy by a const
+reference. This is the case when the variable is const qualified or when it is
+only used as a const, i.e. only const methods or operators are invoked on it, or
+it is used as const reference or value argument in constructors or function
+calls.
+
+Example:
+
+.. code-block:: c++
+
+  const string& constReference() {
+  }
+  void function() {
+    // The warning will suggest making this a const reference.
+    const string UnnecessaryCopy = constReference();
+  }
+
+  struct Foo {
+    const string& name() const;
+  }
+  void Function(const Foo& foo) {
+    // The warning will suggest making this a const reference.
+    string UnnecessaryCopy = foo.name();
+    UnnecessaryCopy.find("bar");
+  }

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

 // RUN: %check_clang_tidy %s performance-unnecessary-copy-initialization %t
 
 struct ExpensiveToCopyType {
   ExpensiveToCopyType() {}
   virtual ~ExpensiveToCopyType() {}
   const ExpensiveToCopyType &reference() const { return *this; }
+  void nonConstMethod() {}
 };
 
 struct TrivialToCopyType {
   const TrivialToCopyType &reference() const { return *this; }
 };
 
 const ExpensiveToCopyType &ExpensiveTypeReference() {
   static const ExpensiveToCopyType *Type = new ExpensiveToCopyType();
   return *Type;
 }
 
 const TrivialToCopyType &TrivialTypeReference() {
   static const TrivialToCopyType *Type = new TrivialToCopyType();
   return *Type;
 }
 
+void mutate(ExpensiveToCopyType &);
+void mutate(ExpensiveToCopyType *);
+void useAsConstReference(const ExpensiveToCopyType &);
+void useByValue(ExpensiveToCopyType);
+
 void PositiveFunctionCall() {
   const auto AutoAssigned = ExpensiveTypeReference();
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable 'AutoAssigned' is copy-constructed from a const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
   // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
   const auto AutoCopyConstructed(ExpensiveTypeReference());
   // CHECK-MESSAGES: [[@LINE-1]]:14: warning: the const qualified variable
   // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference());
   const ExpensiveToCopyType VarAssigned = ExpensiveTypeReference();
void NegativeFunctionCallTrivialType() {
   const TrivialToCopyType VarAssigned = TrivialTypeReference();
   const TrivialToCopyType VarCopyConstructed(TrivialTypeReference());
 }
 
 void NegativeStaticLocalVar(const ExpensiveToCopyType &Obj) {
   static const auto StaticVar = Obj.reference();
 }
 
-void NegativeFunctionCallExpensiveTypeNonConstVariable() {
+void PositiveFunctionCallExpensiveTypeNonConstVariable() {
   auto AutoAssigned = ExpensiveTypeReference();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'AutoAssigned' is copy-constructed from a const reference but is only used as const reference; consider making it a const reference [performance-unnecessary-copy-initialization]
+  // CHECK-FIXES: const auto& AutoAssigned = ExpensiveTypeReference();
   auto AutoCopyConstructed(ExpensiveTypeReference());
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable
+  // CHECK-FIXES: const auto& AutoCopyConstructed(ExpensiveTypeReference());
   ExpensiveToCopyType VarAssigned = ExpensiveTypeReference();
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable
+  // CHECK-FIXES: const ExpensiveToCopyType& VarAssigned = ExpensiveTypeReference();
   ExpensiveToCopyType VarCopyConstructed(ExpensiveTypeReference());
+  // CHECK-MESSAGES: [[@LINE-1]]:23: warning: the variable
+  // CHECK-FIXES: const ExpensiveToCopyType& VarCopyConstructed(ExpensiveTypeReference());
+}
+
+void positiveNonConstVarInCodeBlock(const ExpensiveToCopyType &Obj) {
+  {
+    auto Assigned = Obj.reference();
+    // CHECK-MESSAGES: [[@LINE-1]]:10: warning: the variable
+    // CHECK-FIXES: const auto& Assigned = Obj.reference();
+    Assigned.reference();
+    useAsConstReference(Assigned);
+    useByValue(Assigned);
+  }
+}
+
+void negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType &Obj) {
+  {
+    auto NonConstInvoked = Obj.reference();
+    // CHECK-FIXES: auto NonConstInvoked = Obj.reference();
+    NonConstInvoked.nonConstMethod();
+  }
+  {
+    auto Reassigned = Obj.reference();
+    // CHECK-FIXES: auto Reassigned = Obj.reference();
+    Reassigned = ExpensiveToCopyType();
+  }
+  {
+    auto MutatedByReference = Obj.reference();
+    // CHECK-FIXES: auto MutatedByReference = Obj.reference();
+    mutate(MutatedByReference);
+  }
+  {
+    auto MutatedByPointer = Obj.reference();
+    // CHECK-FIXES: auto MutatedByPointer = Obj.reference();
+    mutate(&MutatedByPointer);
+  }
 }
 
 void NegativeMethodCallNonConstRef(ExpensiveToCopyType &Obj) {
   const auto AutoAssigned = Obj.reference();
   const auto AutoCopyConstructed(Obj.reference());
   const ExpensiveToCopyType VarAssigned = Obj.reference();
   const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
 }
void NegativeMethodCallNonConstPointer(ExpensiveToCopyType *const Obj) {
   const ExpensiveToCopyType VarAssigned = Obj->reference();
   const ExpensiveToCopyType VarCopyConstructed(Obj->reference());
 }
 
 void NegativeObjIsNotParam() {
   ExpensiveToCopyType Obj;
   const auto AutoAssigned = Obj.reference();
   const auto AutoCopyConstructed(Obj.reference());
-  const ExpensiveToCopyType VarAssigned = Obj.reference();
-  const ExpensiveToCopyType VarCopyConstructed(Obj.reference());
+  ExpensiveToCopyType VarAssigned = Obj.reference();
+  ExpensiveToCopyType VarCopyConstructed(Obj.reference());
 }
 
 struct NegativeConstructor {
   NegativeConstructor(const ExpensiveToCopyType &Obj) : Obj(Obj) {}
   ExpensiveToCopyType Obj;
 };
+
+#define UNNECESSARY_COPY_INIT_IN_MACRO_BODY(TYPE) \
+  void functionWith##TYPE(const TYPE& T) {    \
+    auto AssignedInMacro = T.reference();   \
+  }             \
+// Ensure fix is not applied.
+// CHECK-FIXES: auto AssignedInMacro = T.reference();
+
+
+UNNECESSARY_COPY_INIT_IN_MACRO_BODY(ExpensiveToCopyType)
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: the variable 'AssignedInMacro' is copy-constructed
+
+#define UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(ARGUMENT) \
+  ARGUMENT
+
+void PositiveMacroArgument(const ExpensiveToCopyType &Obj) {
+  UNNECESSARY_COPY_INIT_IN_MACRO_ARGUMENT(auto CopyInMacroArg = Obj.reference());
+  // CHECK-MESSAGES: [[@LINE-1]]:48: warning: the variable 'CopyInMacroArg' is copy-constructed
+  // Ensure fix is not applied.
+  // CHECK-FIXES: auto CopyInMacroArg = Obj.reference()
+}