Skip to content

Commit 880554d

Browse files
author
huqizhi
committed
[clang-tidy] fix false positive in cppcoreguidelines-missing-std-forward
1 parent 59af659 commit 880554d

File tree

3 files changed

+90
-5
lines changed

3 files changed

+90
-5
lines changed

clang-tools-extra/clang-tidy/cppcoreguidelines/MissingStdForwardCheck.cpp

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,72 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) {
5353
FuncTemplate->getTemplateParameters()->getDepth();
5454
}
5555

56+
AST_MATCHER_P(NamedDecl, hasSameNameAsBoundNode, std::string, BindingID) {
57+
IdentifierInfo *II = Node.getIdentifier();
58+
if (nullptr == II)
59+
return false;
60+
StringRef Name = II->getName();
61+
62+
return Builder->removeBindings(
63+
[this, Name](const ast_matchers::internal::BoundNodesMap &Nodes) {
64+
const DynTypedNode &BN = Nodes.getNode(this->BindingID);
65+
if (const auto *ND = BN.get<NamedDecl>()) {
66+
if (!isa<FieldDecl, CXXMethodDecl, VarDecl>(ND))
67+
return true;
68+
return ND->getName() != Name;
69+
}
70+
return true;
71+
});
72+
}
73+
74+
AST_MATCHER_P(LambdaCapture, hasCaptureKind, LambdaCaptureKind, Kind) {
75+
return Node.getCaptureKind() == Kind;
76+
}
77+
78+
AST_MATCHER_P(LambdaExpr, hasCaptureDefaultKind, LambdaCaptureDefault, Kind) {
79+
return Node.getCaptureDefault() == Kind;
80+
}
81+
5682
} // namespace
5783

5884
void MissingStdForwardCheck::registerMatchers(MatchFinder *Finder) {
85+
auto RefToParmImplicit = allOf(
86+
equalsBoundNode("var"), hasInitializer(ignoringParenImpCasts(
87+
declRefExpr(to(equalsBoundNode("param"))))));
88+
auto RefToParm = capturesVar(
89+
varDecl(anyOf(hasSameNameAsBoundNode("param"), RefToParmImplicit)));
90+
auto HasRefToParm = hasAnyCapture(RefToParm);
91+
92+
auto CaptureInRef =
93+
allOf(hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByRef),
94+
unless(hasAnyCapture(
95+
capturesVar(varDecl(hasSameNameAsBoundNode("param"))))));
96+
auto CaptureInCopy = allOf(
97+
hasCaptureDefaultKind(LambdaCaptureDefault::LCD_ByCopy), HasRefToParm);
98+
auto CaptureByRefExplicit = hasAnyCapture(
99+
allOf(hasCaptureKind(LambdaCaptureKind::LCK_ByRef), RefToParm));
100+
101+
auto CapturedInBody =
102+
lambdaExpr(anyOf(CaptureInRef, CaptureInCopy, CaptureByRefExplicit));
103+
auto CapturedInCaptureList = hasAnyCapture(capturesVar(
104+
varDecl(hasInitializer(ignoringParenImpCasts(equalsBoundNode("call"))))));
105+
106+
auto CapturedInLambda = hasDeclContext(cxxRecordDecl(
107+
isLambda(),
108+
hasParent(lambdaExpr(forCallable(equalsBoundNode("func")),
109+
anyOf(CapturedInCaptureList, CapturedInBody)))));
110+
59111
auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
60112

61113
auto ForwardCallMatcher = callExpr(
62-
forCallable(equalsBoundNode("func")), argumentCountIs(1),
114+
callExpr().bind("call"), argumentCountIs(1),
115+
hasArgument(
116+
0, declRefExpr(to(
117+
varDecl(optionally(equalsBoundNode("param"))).bind("var")))),
118+
forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)),
63119
callee(unresolvedLookupExpr(hasAnyDeclaration(
64120
namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),
65-
hasArgument(0, declRefExpr(to(equalsBoundNode("param"))).bind("ref")),
121+
66122
unless(anyOf(hasAncestor(typeLoc()),
67123
hasAncestor(expr(hasUnevaluatedContext())))));
68124

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,10 @@ Changes in existing checks
295295
coroutine functions and increase issue detection for cases involving type
296296
aliases with references.
297297

298+
- Improved :doc:`cppcoreguidelines-missing-std-forward
299+
<clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check to
300+
address false positives in the capture list and body of lambdas.
301+
298302
- Improved :doc:`cppcoreguidelines-narrowing-conversions
299303
<clang-tidy/checks/cppcoreguidelines/narrowing-conversions>` check by
300304
extending the `IgnoreConversionFromTypes` option to include types without a

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/missing-std-forward.cpp

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ void lambda_value_capture(T&& t) {
9090
}
9191

9292
template <class T>
93-
void lambda_value_reference(T&& t) {
94-
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
95-
[&]() { T other = std::forward<T>(t); };
93+
void lambda_value_capture_copy(T&& t) {
94+
// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
95+
[&,t]() { T other = std::forward<T>(t); };
9696
}
9797

9898
} // namespace positive_cases
@@ -147,4 +147,29 @@ class AClass {
147147
T data;
148148
};
149149

150+
template <class T>
151+
void lambda_value_reference(T&& t) {
152+
[&]() { T other = std::forward<T>(t); };
153+
}
154+
155+
template<typename T>
156+
void lambda_value_reference_capture_list_ref_1(T&& t) {
157+
[=, &t] { T other = std::forward<T>(t); };
158+
}
159+
160+
template<typename T>
161+
void lambda_value_reference_capture_list_ref_2(T&& t) {
162+
[&t] { T other = std::forward<T>(t); };
163+
}
164+
165+
template<typename T>
166+
void lambda_value_reference_capture_list(T&& t) {
167+
[t = std::forward<T>(t)] { t(); };
168+
}
169+
170+
template <class T>
171+
void lambda_value_reference_auxiliary_var(T&& t) {
172+
[&x = t]() { T other = std::forward<T>(x); };
173+
}
174+
150175
} // namespace negative_cases

0 commit comments

Comments
 (0)