Skip to content

Commit bb5062f

Browse files
committed
[clang-tidy][missing-std-forward]report diagnotics for using forward in lambda body
Fixes: llvm#83845 Capturing variable in lambda by reference and doing forward in lambda body are like constructing a struct by reference and using std::forward in some member function. It is dangerous if the lifetime of lambda expression is longer then current function. And it goes against what this check is intended to check. This PR wants to revert this behavior introduced in llvm#77056 partially.
1 parent 27ce512 commit bb5062f

File tree

3 files changed

+34
-76
lines changed

3 files changed

+34
-76
lines changed

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

Lines changed: 5 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include "MissingStdForwardCheck.h"
1010
#include "../utils/Matchers.h"
1111
#include "clang/AST/ASTContext.h"
12-
#include "clang/AST/ExprConcepts.h"
1312
#include "clang/ASTMatchers/ASTMatchFinder.h"
1413

1514
using namespace clang::ast_matchers;
@@ -53,68 +52,22 @@ AST_MATCHER(ParmVarDecl, isTemplateTypeParameter) {
5352
FuncTemplate->getTemplateParameters()->getDepth();
5453
}
5554

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-
8255
} // namespace
8356

8457
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-
10658
auto CapturedInLambda = hasDeclContext(cxxRecordDecl(
10759
isLambda(),
108-
hasParent(lambdaExpr(forCallable(equalsBoundNode("func")),
109-
anyOf(CapturedInCaptureList, CapturedInBody)))));
60+
hasParent(
61+
lambdaExpr(forCallable(equalsBoundNode("func")),
62+
hasAnyCapture(capturesVar(varDecl(hasInitializer(
63+
ignoringParenImpCasts(equalsBoundNode("call"))))))))));
11064

11165
auto ToParam = hasAnyParameter(parmVarDecl(equalsBoundNode("param")));
11266

11367
auto ForwardCallMatcher = callExpr(
11468
callExpr().bind("call"), argumentCountIs(1),
11569
hasArgument(
116-
0, declRefExpr(to(
117-
varDecl(optionally(equalsBoundNode("param"))).bind("var")))),
70+
0, declRefExpr(to(varDecl(equalsBoundNode("param")).bind("var")))),
11871
forCallable(anyOf(equalsBoundNode("func"), CapturedInLambda)),
11972
callee(unresolvedLookupExpr(hasAnyDeclaration(
12073
namedDecl(hasUnderlyingDecl(hasName("::std::forward")))))),

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,9 @@ Changes in existing checks
142142

143143
- Improved :doc:`cppcoreguidelines-missing-std-forward
144144
<clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by no longer
145-
giving false positives for deleted functions.
145+
giving false positives for deleted functions, avoiding false negative when
146+
multiple arguments in a function, reporting diagnostics when using forward in
147+
body of lambdas.
146148

147149
- Cleaned up :doc:`cppcoreguidelines-prefer-member-initializer
148150
<clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>`

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

Lines changed: 26 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ class AClass {
7575
T data;
7676
};
7777

78+
template <typename X, typename Y> void foo(X &&x, Y &&y) {
79+
// CHECK-MESSAGES: :[[@LINE-1]]:55: warning: forwarding reference parameter 'y' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
80+
auto newx = std::forward<X>(x);
81+
}
82+
7883
template <class T>
7984
void does_not_forward_in_evaluated_code(T&& t) {
8085
// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
@@ -90,9 +95,27 @@ void lambda_value_capture(T&& t) {
9095
}
9196

9297
template <class 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); };
98+
void lambda_value_reference(T&& t) {
99+
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
100+
[&]() { T other = std::forward<T>(t); };
101+
}
102+
103+
template<typename T>
104+
void lambda_value_reference_capture_list_ref_1(T&& t) {
105+
// CHECK-MESSAGES: :[[@LINE-1]]:52: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
106+
[=, &t] { T other = std::forward<T>(t); };
107+
}
108+
109+
template<typename T>
110+
void lambda_value_reference_capture_list_ref_2(T&& t) {
111+
// CHECK-MESSAGES: :[[@LINE-1]]:52: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
112+
[&t] { T other = std::forward<T>(t); };
113+
}
114+
115+
template <class T>
116+
void lambda_value_reference_auxiliary_var(T&& t) {
117+
// CHECK-MESSAGES: :[[@LINE-1]]:47: warning: forwarding reference parameter 't' is never forwarded inside the function body [cppcoreguidelines-missing-std-forward]
118+
[&x = t]() { T other = std::forward<T>(x); };
96119
}
97120

98121
} // namespace positive_cases
@@ -147,31 +170,11 @@ class AClass {
147170
T data;
148171
};
149172

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-
165173
template<typename T>
166174
void lambda_value_reference_capture_list(T&& t) {
167175
[t = std::forward<T>(t)] { t(); };
168176
}
169177

170-
template <class T>
171-
void lambda_value_reference_auxiliary_var(T&& t) {
172-
[&x = t]() { T other = std::forward<T>(x); };
173-
}
174-
175178
} // namespace negative_cases
176179

177180
namespace deleted_functions {

0 commit comments

Comments
 (0)