[clang-tools-extra] [clang-tidy] avoid false positive when overload for bugprone-return-const-ref-from-parameter (PR #95434)

[Congcong Cai via cfe-commits]

https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/95434

Fixes: #90274


>From ecbd726bb937361b243ea4433e8c597c8d30f857 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0907 at 163.com>
Date: Fri, 14 Jun 2024 00:50:04 +0800
Subject: [PATCH] [clang-tidy] avoid false positive when overload for
 bugprone-return-const-ref-from-parameter

Fixes: #90274
---
 .../ReturnConstRefFromParameterCheck.cpp      | 76 +++++++++++++++++--
 .../return-const-ref-from-parameter.cpp       | 34 +++++++++
 2 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index cacba38b4a5aa..ecdcd3c2d2571 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -7,7 +7,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "ReturnConstRefFromParameterCheck.h"
-#include "../utils/Matchers.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 
@@ -18,20 +17,87 @@ namespace clang::tidy::bugprone {
 void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       returnStmt(
-          hasReturnValue(declRefExpr(to(parmVarDecl(hasType(hasCanonicalType(
-              qualType(matchers::isReferenceToConst()).bind("type"))))))),
-          hasAncestor(functionDecl(hasReturnTypeLoc(
-              loc(qualType(hasCanonicalType(equalsBoundNode("type"))))))))
+          hasReturnValue(declRefExpr(
+              to(parmVarDecl(hasType(hasCanonicalType(
+                                 qualType(lValueReferenceType(pointee(
+                                              qualType(isConstQualified()))))
+                                     .bind("type"))))
+                     .bind("param")))),
+          hasAncestor(
+              functionDecl(hasReturnTypeLoc(loc(qualType(
+                               hasCanonicalType(equalsBoundNode("type"))))))
+                  .bind("func")))
           .bind("ret"),
       this);
 }
 
+static bool isSameTypeIgnoringConst(QualType A, QualType B) {
+  A = A.getCanonicalType();
+  B = B.getCanonicalType();
+  A.addConst();
+  B.addConst();
+  return A == B;
+}
+
+static bool isSameTypeIgnoringConstRef(QualType A, QualType B) {
+  return isSameTypeIgnoringConst(A.getCanonicalType().getNonReferenceType(),
+                                 B.getCanonicalType().getNonReferenceType());
+}
+
+static bool hasSameParameterTypes(const FunctionDecl &FD, const FunctionDecl &O,
+                                  const ParmVarDecl &PD) {
+  if (FD.getNumParams() != O.getNumParams())
+    return false;
+  for (unsigned I = 0, E = FD.getNumParams(); I < E; ++I) {
+    const ParmVarDecl *DPD = FD.getParamDecl(I);
+    const QualType OPT = O.getParamDecl(I)->getType();
+    if (DPD == &PD) {
+      if (!llvm::isa<RValueReferenceType>(OPT) ||
+          !isSameTypeIgnoringConstRef(DPD->getType(), OPT))
+        return false;
+    } else {
+      if (!isSameTypeIgnoringConst(DPD->getType(), OPT))
+        return false;
+    }
+  }
+  return true;
+}
+
+static const Decl *findRVRefOverload(const FunctionDecl &FD,
+                                     const ParmVarDecl &PD) {
+  // Actually it would be better to do lookup in caller site.
+  // But in most of cases, overloads of LVRef and RVRef will appear together.
+  // FIXME:
+  // 1. overload in anonymous namespace
+  // 2. forward reference
+  DeclContext::lookup_result LookupResult =
+      FD.getParent()->lookup(FD.getNameInfo().getName());
+  if (LookupResult.isSingleResult()) {
+    return nullptr;
+  }
+  for (const Decl *Overload : LookupResult) {
+    if (Overload == &FD)
+      continue;
+    Overload->dumpColor();
+    if (const auto *O = dyn_cast<FunctionDecl>(Overload))
+      if (hasSameParameterTypes(FD, *O, PD))
+        return O;
+  }
+  return nullptr;
+}
+
 void ReturnConstRefFromParameterCheck::check(
     const MatchFinder::MatchResult &Result) {
+  const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  const auto *PD = Result.Nodes.getNodeAs<ParmVarDecl>("param");
   const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret");
   const SourceRange Range = R->getRetValue()->getSourceRange();
   if (Range.isInvalid())
     return;
+
+  if (findRVRefOverload(*FD, *PD) != nullptr)
+    return;
+
   diag(Range.getBegin(),
        "returning a constant reference parameter may cause use-after-free "
        "when the parameter is constructed from a temporary")
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
index ca41bdf74a107..d13c127da7c2a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp
@@ -143,3 +143,37 @@ void instantiate(const int &param, const float &paramf, int &mut_param, float &m
 }
 
 } // namespace valid
+
+namespace overload {
+
+int const &overload_base(int const &a) { return a; }
+int const &overload_base(int &&a);
+
+int const &overload_ret_type(int const &a) { return a; }
+void overload_ret_type(int &&a);
+
+int const &overload_params1(int p1, int const &a) { return a; }
+int const & overload_params1(int p1, int &&a);
+
+int const &overload_params2(int p1, int const &a, int p2) { return a; }
+int const &overload_params2(int p1, int &&a, int p2);
+
+int const &overload_params3(T p1, int const &a, int p2) { return a; }
+int const &overload_params3(int p1, int &&a, T p2);
+
+int const &overload_params_const(int p1, int const &a, int const p2) { return a; }
+int const &overload_params_const(int const p1, int &&a, int p2);
+
+int const &overload_params_difference1(int p1, int const &a, int p2) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter
+int const &overload_params_difference1(long p1, int &&a, int p2);
+
+int const &overload_params_difference2(int p1, int const &a, int p2) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter
+int const &overload_params_difference2(int p1, int &&a, long p2);
+
+int const &overload_params_difference3(int p1, int const &a, int p2) { return a; }
+// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter
+int const &overload_params_difference3(int p1, long &&a, int p2);
+
+} // namespace overload