Skip to content

Commit 3d25e5a

Browse files
authored
[clang-tidy] avoid false positive when overload for bugprone-return-const-ref-from-parameter (llvm#95434)
Fixes: llvm#90274
1 parent f00f11b commit 3d25e5a

File tree

2 files changed

+100
-5
lines changed

2 files changed

+100
-5
lines changed

clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "ReturnConstRefFromParameterCheck.h"
10-
#include "../utils/Matchers.h"
1110
#include "clang/ASTMatchers/ASTMatchFinder.h"
1211
#include "clang/ASTMatchers/ASTMatchers.h"
1312

@@ -18,20 +17,82 @@ namespace clang::tidy::bugprone {
1817
void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
1918
Finder->addMatcher(
2019
returnStmt(
21-
hasReturnValue(declRefExpr(to(parmVarDecl(hasType(hasCanonicalType(
22-
qualType(matchers::isReferenceToConst()).bind("type"))))))),
23-
hasAncestor(functionDecl(hasReturnTypeLoc(
24-
loc(qualType(hasCanonicalType(equalsBoundNode("type"))))))))
20+
hasReturnValue(declRefExpr(
21+
to(parmVarDecl(hasType(hasCanonicalType(
22+
qualType(lValueReferenceType(pointee(
23+
qualType(isConstQualified()))))
24+
.bind("type"))))
25+
.bind("param")))),
26+
hasAncestor(
27+
functionDecl(hasReturnTypeLoc(loc(qualType(
28+
hasCanonicalType(equalsBoundNode("type"))))))
29+
.bind("func")))
2530
.bind("ret"),
2631
this);
2732
}
2833

34+
static bool isSameTypeIgnoringConst(QualType A, QualType B) {
35+
return A.getCanonicalType().withConst() == B.getCanonicalType().withConst();
36+
}
37+
38+
static bool isSameTypeIgnoringConstRef(QualType A, QualType B) {
39+
return isSameTypeIgnoringConst(A.getCanonicalType().getNonReferenceType(),
40+
B.getCanonicalType().getNonReferenceType());
41+
}
42+
43+
static bool hasSameParameterTypes(const FunctionDecl &FD, const FunctionDecl &O,
44+
const ParmVarDecl &PD) {
45+
if (FD.getNumParams() != O.getNumParams())
46+
return false;
47+
for (unsigned I = 0, E = FD.getNumParams(); I < E; ++I) {
48+
const ParmVarDecl *DPD = FD.getParamDecl(I);
49+
const QualType OPT = O.getParamDecl(I)->getType();
50+
if (DPD == &PD) {
51+
if (!llvm::isa<RValueReferenceType>(OPT) ||
52+
!isSameTypeIgnoringConstRef(DPD->getType(), OPT))
53+
return false;
54+
} else {
55+
if (!isSameTypeIgnoringConst(DPD->getType(), OPT))
56+
return false;
57+
}
58+
}
59+
return true;
60+
}
61+
62+
static const Decl *findRVRefOverload(const FunctionDecl &FD,
63+
const ParmVarDecl &PD) {
64+
// Actually it would be better to do lookup in caller site.
65+
// But in most of cases, overloads of LVRef and RVRef will appear together.
66+
// FIXME:
67+
// 1. overload in anonymous namespace
68+
// 2. forward reference
69+
DeclContext::lookup_result LookupResult =
70+
FD.getParent()->lookup(FD.getNameInfo().getName());
71+
if (LookupResult.isSingleResult()) {
72+
return nullptr;
73+
}
74+
for (const Decl *Overload : LookupResult) {
75+
if (Overload == &FD)
76+
continue;
77+
if (const auto *O = dyn_cast<FunctionDecl>(Overload))
78+
if (hasSameParameterTypes(FD, *O, PD))
79+
return O;
80+
}
81+
return nullptr;
82+
}
83+
2984
void ReturnConstRefFromParameterCheck::check(
3085
const MatchFinder::MatchResult &Result) {
86+
const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("func");
87+
const auto *PD = Result.Nodes.getNodeAs<ParmVarDecl>("param");
3188
const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret");
3289
const SourceRange Range = R->getRetValue()->getSourceRange();
3390
if (Range.isInvalid())
3491
return;
92+
93+
if (findRVRefOverload(*FD, *PD) != nullptr)
94+
return;
95+
3596
diag(Range.getBegin(),
3697
"returning a constant reference parameter may cause use-after-free "
3798
"when the parameter is constructed from a temporary")

clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,37 @@ void instantiate(const int &param, const float &paramf, int &mut_param, float &m
143143
}
144144

145145
} // namespace valid
146+
147+
namespace overload {
148+
149+
int const &overload_base(int const &a) { return a; }
150+
int const &overload_base(int &&a);
151+
152+
int const &overload_ret_type(int const &a) { return a; }
153+
void overload_ret_type(int &&a);
154+
155+
int const &overload_params1(int p1, int const &a) { return a; }
156+
int const & overload_params1(int p1, int &&a);
157+
158+
int const &overload_params2(int p1, int const &a, int p2) { return a; }
159+
int const &overload_params2(int p1, int &&a, int p2);
160+
161+
int const &overload_params3(T p1, int const &a, int p2) { return a; }
162+
int const &overload_params3(int p1, int &&a, T p2);
163+
164+
int const &overload_params_const(int p1, int const &a, int const p2) { return a; }
165+
int const &overload_params_const(int const p1, int &&a, int p2);
166+
167+
int const &overload_params_difference1(int p1, int const &a, int p2) { return a; }
168+
// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter
169+
int const &overload_params_difference1(long p1, int &&a, int p2);
170+
171+
int const &overload_params_difference2(int p1, int const &a, int p2) { return a; }
172+
// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter
173+
int const &overload_params_difference2(int p1, int &&a, long p2);
174+
175+
int const &overload_params_difference3(int p1, int const &a, int p2) { return a; }
176+
// CHECK-MESSAGES: :[[@LINE-1]]:79: warning: returning a constant reference parameter
177+
int const &overload_params_difference3(int p1, long &&a, int p2);
178+
179+
} // namespace overload

0 commit comments

Comments
 (0)