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

[courbet]

Summary

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.

[flx]

Thanks for the fix!

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

AST_MATCHER_FUNCTION_P(StatementMatcher, isConstRefReturningMethodCall,
                        std::vector<std::string>, ExcludedContainerTypes) {
   // 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.
   return cxxMemberCallExpr(
-      callee(cxxMethodDecl(returns(matchers::isReferenceToConst()))
+      callee(cxxMethodDecl(
+                 returns(hasCanonicalType(matchers::isReferenceToConst())))
                  .bind(MethodDeclId)),
       on(declRefExpr(to(
           varDecl(
               unless(hasType(qualType(hasCanonicalType(hasDeclaration(namedDecl(
                   matchers::matchesAnyListedName(ExcludedContainerTypes))))))))
               .bind(ObjectArgId)))));
 }
 
 AST_MATCHER_FUNCTION(StatementMatcher, isConstRefReturningFunctionCall) {
   // 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);
 }
 
 AST_MATCHER_FUNCTION_P(StatementMatcher, initializerReturnsReferenceToConst,
                        std::vector<std::string>, ExcludedContainerTypes) {
   auto OldVarDeclRef =

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

 // RUN: %check_clang_tidy -std=c++17 %s performance-unnecessary-copy-initialization %t
 
 template <typename T>
 struct Iterator {
   void operator++();
   const T &operator*() const;
   bool operator!=(const Iterator &) const;
   typedef const T &const_reference;
 };
 
 struct ExpensiveToCopyType {
   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;
   void nonConstMethod();
   bool constMethod() const;
   template <typename A>
   const A &templatedAccessor() const;
   operator int() const; // Implicit conversion to int.
void positiveNonConstVarInCodeBlock(const ExpensiveToCopyType &Obj) {
     // CHECK-MESSAGES: [[@LINE-1]]:10: warning: the variable 'Assigned'
     // CHECK-FIXES: const auto& Assigned = Obj.reference();
     Assigned.reference();
     useAsConstReference(Assigned);
     useByValue(Assigned);
   }
 }
 
+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();
     // CHECK-FIXES: auto NonConstInvoked = Obj.reference();
     NonConstInvoked.nonConstMethod();
   }
   {
     auto Reassigned = Obj.reference();