[clang-tools-extra] ace9653 - [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

[Felix Berger via cfe-commits]

Author: Felix Berger
Date: 2020-11-16T17:08:18-05:00
New Revision: ace9653c11c6308401dcda2e8b26bf97e6e66e30

URL: https://github.com/llvm/llvm-project/commit/ace9653c11c6308401dcda2e8b26bf97e6e66e30
DIFF: https://github.com/llvm/llvm-project/commit/ace9653c11c6308401dcda2e8b26bf97e6e66e30.diff

LOG: [clang-tidy] performance-unnecessary-copy-initialization: Check for const reference arguments that are replaced template parameter type.

This fixes false positive cases where a non-const reference is passed to a
std::function but interpreted as a const reference.

Fix the definition of the fake std::function added in the test to match
std::function and make the bug reproducible.

Reviewed-by: aaron.ballman

Differential Revision: https://reviews.llvm.org/D90042

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
    clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
index c4bc1835057b..d0933f0860c8 100644
--- a/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/DeclRefExprUtils.cpp
@@ -59,9 +59,13 @@ constReferenceDeclRefExprs(const VarDecl &VarDecl, const Stmt &Stmt,
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   auto ConstReferenceOrValue =
       qualType(anyOf(referenceType(pointee(qualType(isConstQualified()))),
-                     unless(anyOf(referenceType(), pointerType()))));
+                     unless(anyOf(referenceType(), pointerType(),
+                                  substTemplateTypeParmType()))));
+  auto ConstReferenceOrValueOrReplaced = qualType(anyOf(
+      ConstReferenceOrValue,
+      substTemplateTypeParmType(hasReplacementType(ConstReferenceOrValue))));
   auto UsedAsConstRefOrValueArg = forEachArgumentWithParam(
-      DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValue)));
+      DeclRefToVar, parmVarDecl(hasType(ConstReferenceOrValueOrReplaced)));
   Matches = match(findAll(callExpr(UsedAsConstRefOrValueArg)), Stmt, Context);
   extractNodesByIdTo(Matches, "declRef", DeclRefs);
   Matches =

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
index 9055529c7a3f..120902d0eade 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -411,12 +411,12 @@ inline namespace __1 {
 
 template <class>
 class function;
-template <class R, class... Args>
-class function<R(Args...)> {
+template <class R, class... ArgTypes>
+class function<R(ArgTypes...)> {
 public:
   function();
-  function(const function &other);
-  R operator()(Args &&...args) const;
+  function(const function &Other);
+  R operator()(ArgTypes... Args) const;
 };
 
 } // namespace __1
@@ -460,3 +460,19 @@ void positiveFakeStdFunction(std::function<void(int)> F) {
 }
 
 } // namespace fake
+
+void positiveInvokedOnStdFunction(
+    std::function<void(const ExpensiveToCopyType &)> Update,
+    const ExpensiveToCopyType Orig) {
+  auto Copy = Orig.reference();
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: the variable 'Copy' is copy-constructed from a const reference
+  // CHECK-FIXES: const auto& Copy = Orig.reference();
+  Update(Copy);
+}
+
+void negativeInvokedOnStdFunction(
+    std::function<void(ExpensiveToCopyType &)> Update,
+    const ExpensiveToCopyType Orig) {
+  auto Copy = Orig.reference();
+  Update(Copy);
+}