ivanmurashko created this revision.
ivanmurashko added reviewers: alexfh, sammccall, mboehme, aaron.ballman.
ivanmurashko added projects: clang, clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
ivanmurashko requested review of this revision.
Herald added a subscriber: cfe-commits.
The diff adds processing for `std::move` inside lambda captures at
`bugprone-use-after-move` check. Especially it detects invalid `std::move`
inside following code
int foo() {
int a = 0;
auto fun = [aa = std::move(a)]{
return aa;
};
return a;
}
Test Plan
./bin/llvm-lit -v
../clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119165
Files:
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1350,3 +1350,18 @@
private:
std::string val_;
};
+
+// Check std::move usage inside lambda captures
+int lambdaCaptures() {
+ int a = 0;
+ int b = 0;
+ int c = 0;
+ auto foo = [aa = std::move(a), bb = std::move(b), cc = c] {
+ // CHECK-NOTES: [[@LINE+6]]:10: warning: 'a' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:20: note: move occurred here
+ // CHECK-NOTES: [[@LINE+4]]:14: warning: 'b' used after it was moved
+ // CHECK-NOTES: [[@LINE-4]]:39: note: move occurred here
+ return aa + bb + cc;
+ };
+ return a + b + c;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -397,11 +397,13 @@
}
void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
- auto CallMoveMatcher =
+ auto AncestorMatcher =
+ anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+ hasAncestor(functionDecl().bind("containing-func")));
+
+ auto BaseMoveMatcher =
callExpr(callee(functionDecl(hasName("::std::move"))),
argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
- anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
- hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a
// bool to tell callers whether it moved. Ignore std::move
inside
@@ -411,6 +413,16 @@
callee(cxxMethodDecl(hasName("try_emplace")))))))
.bind("call-move");
+ auto CallMoveMatcherLambda = lambdaExpr(
+ forEachLambdaCapture(lambdaCapture(capturesVar(varDecl(
+ hasInitializer(expr(ignoringParenImpCasts(BaseMoveMatcher))))))),
+ AncestorMatcher);
+
+ Finder->addMatcher(CallMoveMatcherLambda, this);
+
+ auto CallMoveMatcherNoLambda =
+ expr(ignoringParenImpCasts(BaseMoveMatcher), AncestorMatcher);
+
Finder->addMatcher(
traverse(
TK_AsIs,
@@ -418,7 +430,7 @@
// for the direct ancestor of the std::move() that isn't one of the
// node types ignored by ignoringParenImpCasts().
stmt(
- forEach(expr(ignoringParenImpCasts(CallMoveMatcher))),
+ forEach(CallMoveMatcherNoLambda),
// Don't allow an InitListExpr to be the moving call. An
// InitListExpr has both a syntactic and a semantic form, and the
// parent-child relationships are different between the two. This
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -1350,3 +1350,18 @@
private:
std::string val_;
};
+
+// Check std::move usage inside lambda captures
+int lambdaCaptures() {
+ int a = 0;
+ int b = 0;
+ int c = 0;
+ auto foo = [aa = std::move(a), bb = std::move(b), cc = c] {
+ // CHECK-NOTES: [[@LINE+6]]:10: warning: 'a' used after it was moved
+ // CHECK-NOTES: [[@LINE-2]]:20: note: move occurred here
+ // CHECK-NOTES: [[@LINE+4]]:14: warning: 'b' used after it was moved
+ // CHECK-NOTES: [[@LINE-4]]:39: note: move occurred here
+ return aa + bb + cc;
+ };
+ return a + b + c;
+};
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -397,11 +397,13 @@
}
void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
- auto CallMoveMatcher =
+ auto AncestorMatcher =
+ anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+ hasAncestor(functionDecl().bind("containing-func")));
+
+ auto BaseMoveMatcher =
callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
- anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
- hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a
// bool to tell callers whether it moved. Ignore std::move inside
@@ -411,6 +413,16 @@
callee(cxxMethodDecl(hasName("try_emplace")))))))
.bind("call-move");
+ auto CallMoveMatcherLambda = lambdaExpr(
+ forEachLambdaCapture(lambdaCapture(capturesVar(varDecl(
+ hasInitializer(expr(ignoringParenImpCasts(BaseMoveMatcher))))))),
+ AncestorMatcher);
+
+ Finder->addMatcher(CallMoveMatcherLambda, this);
+
+ auto CallMoveMatcherNoLambda =
+ expr(ignoringParenImpCasts(BaseMoveMatcher), AncestorMatcher);
+
Finder->addMatcher(
traverse(
TK_AsIs,
@@ -418,7 +430,7 @@
// for the direct ancestor of the std::move() that isn't one of the
// node types ignored by ignoringParenImpCasts().
stmt(
- forEach(expr(ignoringParenImpCasts(CallMoveMatcher))),
+ forEach(CallMoveMatcherNoLambda),
// Don't allow an InitListExpr to be the moving call. An
// InitListExpr has both a syntactic and a semantic form, and the
// parent-child relationships are different between the two. This
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
