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

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

Reviewers: alexfh

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D17488

git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@262781 91177308-0d34-0410-b5e6-96231b3b80d8

10 files changed, 353 insertions, 125 deletions

diff --git a/clang-tidy/performance/ForRangeCopyCheck.cpp b/clang-tidy/performance/ForRangeCopyCheck.cpp
index a293cfee..abdcdf49 100644
--- a/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ b/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -8,92 +8,15 @@
 //===----------------------------------------------------------------------===//
 
 #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),
@@ -143,9 +66,9 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl &LoopVar,
       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;
 }
 
@@ -154,24 +77,15 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
     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;
 }
 
diff --git a/clang-tidy/performance/UnnecessaryCopyInitialization.cpp b/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 37883e98..ed37b32e 100644
--- a/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -9,7 +9,8 @@
 
 #include "UnnecessaryCopyInitialization.h"
 
-#include "../utils/LexerUtils.h"
+#include "../utils/DeclRefExprUtils.h"
+#include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 
 namespace clang {
@@ -18,17 +19,13 @@ 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.
@@ -41,30 +38,44 @@ void UnnecessaryCopyInitialization::registerMatchers(
       callExpr(callee(functionDecl(returns(ConstReference))),
                unless(callee(cxxMethodDecl())));
   Finder->addMatcher(
-      varDecl(
-          hasLocalStorage(), hasType(isConstQualified()),
-          hasType(matchers::isExpensiveToCopy()),
-          hasInitializer(cxxConstructExpr(
-              hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
-              hasArgument(0, anyOf(ConstRefReturningFunctionCall,
+      compoundStmt(
+          forEachDescendant(
+              varDecl(
+                  hasLocalStorage(), hasType(matchers::isExpensiveToCopy()),
+                  hasInitializer(cxxConstructExpr(
+                      hasDeclaration(cxxConstructorDecl(isCopyConstructor())),
+                      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());
-  }
-  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, "&");
+  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(),
+           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
diff --git a/clang-tidy/performance/UnnecessaryCopyInitialization.h b/clang-tidy/performance/UnnecessaryCopyInitialization.h
index 22a80a86..73d0b341 100644
--- a/clang-tidy/performance/UnnecessaryCopyInitialization.h
+++ b/clang-tidy/performance/UnnecessaryCopyInitialization.h
@@ -16,10 +16,10 @@ 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,
diff --git a/clang-tidy/utils/CMakeLists.txt b/clang-tidy/utils/CMakeLists.txt
index 6cc91794..d391817d 100644
--- a/clang-tidy/utils/CMakeLists.txt
+++ b/clang-tidy/utils/CMakeLists.txt
@@ -1,6 +1,8 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyUtils
+  DeclRefExprUtils.cpp
+  FixItHintUtils.cpp
   HeaderGuard.cpp
   HeaderFileExtensionsUtils.cpp
   IncludeInserter.cpp
diff --git a/clang-tidy/utils/DeclRefExprUtils.cpp b/clang-tidy/utils/DeclRefExprUtils.cpp
new file mode 100644
index 00000000..c7b99a20
--- /dev/null
+++ b/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -0,0 +1,98 @@
+//===--- 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
diff --git a/clang-tidy/utils/DeclRefExprUtils.h b/clang-tidy/utils/DeclRefExprUtils.h
new file mode 100644
index 00000000..b3764ba4
--- /dev/null
+++ b/clang-tidy/utils/DeclRefExprUtils.h
@@ -0,0 +1,32 @@
+//===--- 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
diff --git a/clang-tidy/utils/FixItHintUtils.cpp b/clang-tidy/utils/FixItHintUtils.cpp
new file mode 100644
index 00000000..c8a28a0e
--- /dev/null
+++ b/clang-tidy/utils/FixItHintUtils.cpp
@@ -0,0 +1,36 @@
+//===--- 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
diff --git a/clang-tidy/utils/FixItHintUtils.h b/clang-tidy/utils/FixItHintUtils.h
new file mode 100644
index 00000000..8cca0feb
--- /dev/null
+++ b/clang-tidy/utils/FixItHintUtils.h
@@ -0,0 +1,32 @@
+//===--- 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
diff --git a/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst b/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
new file mode 100644
index 00000000..f1dc2652
--- /dev/null
+++ b/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
@@ -0,0 +1,34 @@
+.. 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");
+  }
diff --git a/test/clang-tidy/performance-unnecessary-copy-initialization.cpp b/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
index aaaeffd6..5a4520e0 100644
--- a/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
+++ b/test/clang-tidy/performance-unnecessary-copy-initialization.cpp
@@ -4,6 +4,7 @@ struct ExpensiveToCopyType {
   ExpensiveToCopyType() {}
   virtual ~ExpensiveToCopyType() {}
   const ExpensiveToCopyType &reference() const { return *this; }
+  void nonConstMethod() {}
 };
 
 struct TrivialToCopyType {
@@ -20,6 +21,11 @@ const TrivialToCopyType &TrivialTypeReference() {
   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]
@@ -114,11 +120,53 @@ 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) {
@@ -146,11 +194,32 @@ 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()
+}