[PATCH] D112722: [clang-tidy]performance-unnecessary-copy-initialization: fix false negative

[Clement Courbet via Phabricator via cfe-commits]

courbet created this revision.
courbet added reviewers: flx, ymandel, aaron.ballman.
Herald added subscribers: carlosgalvezp, xazax.hun.
courbet requested review of this revision.
Herald added a project: clang-tools-extra.

We're missing all cases where the return value is a type alias.

Unfortunately, this includes things we care about, such as
`std::vector<T>::operator[]` (return value is `const_reference`,
not `const T&`).

Match the canonical type instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112722

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -12,6 +12,8 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType &reference() const;
+  using ConstRef = const ExpensiveToCopyType &;
+  ConstRef referenceWithAlias() const;
   const ExpensiveToCopyType *pointer() const;
   Iterator<ExpensiveToCopyType> begin() const;
   Iterator<ExpensiveToCopyType> end() const;
@@ -206,6 +208,15 @@
   }
 }
 
+void positiveNonConstVarInCodeBlockWithAlias(const ExpensiveToCopyType &Obj) {
+  {
+    const ExpensiveToCopyType Assigned = Obj.referenceWithAlias();
+    // CHECK-MESSAGES: [[@LINE-1]]:31: warning: the const qualified variable
+    // CHECK-FIXES: const ExpensiveToCopyType& Assigned = Obj.referenceWithAlias();
+    useAsConstReference(Assigned);
+  }
+}
+
 void negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType &Obj) {
   {
     auto NonConstInvoked = Obj.reference();
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===================================================================
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -84,7 +84,8 @@
   // returned either points to a global static variable or to a member of the
   // called object.
   return cxxMemberCallExpr(
-      callee(cxxMethodDecl(returns(matchers::isReferenceToConst()))
+      callee(cxxMethodDecl(
+                 returns(hasCanonicalType(matchers::isReferenceToConst())))
                  .bind(MethodDeclId)),
       on(declRefExpr(to(
           varDecl(
@@ -97,7 +98,8 @@
   // Only allow initialization of a const reference from a free function if it
   // has no arguments. Otherwise it could return an alias to one of its
   // arguments and the arguments need to be checked for const use as well.
-  return callExpr(callee(functionDecl(returns(matchers::isReferenceToConst()))
+  return callExpr(callee(functionDecl(returns(hasCanonicalType(
+                                          matchers::isReferenceToConst())))
                              .bind(FunctionDeclId)),
                   argumentCountIs(0), unless(callee(cxxMethodDecl())))
       .bind(InitFunctionCallId);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112722.383038.patch
Type: text/x-patch
Size: 2577 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211028/f149e05b/attachment.bin>