https://github.com/HerrCai0907 updated
https://github.com/llvm/llvm-project/pull/95434
>From ecbd726bb937361b243ea4433e8c597c8d30f857 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Fri, 14 Jun 2024 00:50:04 +0800
Subject: [PATCH 1/2] [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 ¶m, const float ¶mf,
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
>From a249d9e97b5de7a4e6881e75312e12156b7d1fa1 Mon Sep 17 00:00:00 2001
From: Congcong Cai <congcongcai0...@163.com>
Date: Fri, 14 Jun 2024 23:53:04 +0800
Subject: [PATCH 2/2] fix
---
.../bugprone/ReturnConstRefFromParameterCheck.cpp | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
index ecdcd3c2d2571..adb26ade955c5 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp
@@ -32,11 +32,7 @@ void
ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
}
static bool isSameTypeIgnoringConst(QualType A, QualType B) {
- A = A.getCanonicalType();
- B = B.getCanonicalType();
- A.addConst();
- B.addConst();
- return A == B;
+ return A.getCanonicalType().withConst() == B.getCanonicalType().withConst();
}
static bool isSameTypeIgnoringConstRef(QualType A, QualType B) {
@@ -78,7 +74,6 @@ static const Decl *findRVRefOverload(const FunctionDecl &FD,
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
