NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr2, aaron.ballman, vsavchenko.
Herald added subscribers: martong, mgehre, xazax.hun.
NoQ requested review of this revision.
Herald added a project: clang-tools-extra.
D96215 <https://reviews.llvm.org/D96215> takes care of the situation where the
variable is captured into a nearby lambda. This patch takes care of the
situation where the current function //is// the lambda and the variable is one
of its captures from an enclosing scope.
The analogous problem for ^{blocks} is already handled automagically by the
previous patch. That said, I added two FIXME tests that demonstrate an
unrelated (but fairly disturbing) problem: because `BlockDecl` isn't a
`FunctionDecl`, the `forFunction()` matcher doesn't behave as expected. This
time it manifests through alias analysis analyzing more code than necessary but
there are other annoying effects of this bug that I'll get to later.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D101787
Files:
clang-tools-extra/clang-tidy/utils/Aliasing.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -1259,6 +1259,71 @@
}
}
+void mutate_at_any_time(bool *x);
+
+void capture_with_branches_inside_lambda_bad() {
+ bool x = true;
+ accept_callback([=]() {
+ if (x) {
+ wait();
+ if (x) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'x' [bugprone-redundant-branch-condition]
+ }
+ }
+ });
+ mutate_at_any_time(&x);
+}
+
+void capture_with_branches_inside_lambda_good() {
+ bool x = true;
+ accept_callback([&]() {
+ if (x) {
+ wait();
+ if (x) {
+ }
+ }
+ });
+ mutate_at_any_time(&x);
+}
+
+void capture_with_branches_inside_block_bad() {
+ bool x = true;
+ accept_callback(^{
+ if (x) {
+ wait();
+ if (x) {
+ // FIXME: Should warn. It currently reacts to &x outside the block
+ // which ideally shouldn't have any effect.
+ }
+ }
+ });
+ mutate_at_any_time(&x);
+}
+
+void capture_with_branches_inside_block_bad_simpler() {
+ bool x = true;
+ accept_callback(^{
+ if (x) {
+ wait();
+ if (x) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant condition 'x' [bugprone-redundant-branch-condition]
+ }
+ }
+ });
+}
+
+void capture_with_branches_inside_block_good() {
+ __block bool x = true;
+ accept_callback(^{
+ if (x) {
+ wait();
+ if (x) {
+ }
+ }
+ });
+ mutate_at_any_time(&x);
+}
+
// GNU Expression Statements
void negative_gnu_expression_statement() {
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -457,6 +457,92 @@
}
}
+void finish_at_any_time(bool *finished);
+
+void lambda_capture_with_loop_inside_lambda_bad() {
+ bool finished = false;
+ auto lambda = [=]() {
+ while (!finished) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: this loop is infinite; none of its condition variables (finished) are updated in the loop body [bugprone-infinite-loop]
+ wait();
+ }
+ };
+ finish_at_any_time(&finished);
+ lambda();
+}
+
+void lambda_capture_with_loop_inside_lambda_bad_init_capture() {
+ bool finished = false;
+ auto lambda = [captured_finished=finished]() {
+ while (!captured_finished) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: this loop is infinite; none of its condition variables (captured_finished) are updated in the loop body [bugprone-infinite-loop]
+ wait();
+ }
+ };
+ finish_at_any_time(&finished);
+ lambda();
+}
+
+void lambda_capture_with_loop_inside_lambda_good() {
+ bool finished = false;
+ auto lambda = [&]() {
+ while (!finished) {
+ wait(); // No warning: the variable may be updated
+ // from outside the lambda.
+ }
+ };
+ finish_at_any_time(&finished);
+ lambda();
+}
+
+void lambda_capture_with_loop_inside_lambda_good_init_capture() {
+ bool finished = false;
+ auto lambda = [&captured_finished=finished]() {
+ while (!captured_finished) {
+ wait(); // No warning: the variable may be updated
+ // from outside the lambda.
+ }
+ };
+ finish_at_any_time(&finished);
+ lambda();
+}
+
+void block_capture_with_loop_inside_block_bad() {
+ bool finished = false;
+ auto block = ^() {
+ while (!finished) {
+ // FIXME: This should warn. It currently reacts to &finished
+ // outside the block which ideally shouldn't have any effect.
+ wait();
+ }
+ };
+ finish_at_any_time(&finished);
+ block();
+}
+
+void block_capture_with_loop_inside_block_bad_simpler() {
+ bool finished = false;
+ auto block = ^() {
+ while (!finished) {
+ // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: this loop is infinite; none of its condition variables (finished) are updated in the loop body [bugprone-infinite-loop]
+ wait();
+ }
+ };
+ block();
+}
+
+void block_capture_with_loop_inside_block_good() {
+ __block bool finished = false;
+ auto block = ^() {
+ while (!finished) {
+ wait(); // No warning: the variable may be updated
+ // from outside the block.
+ }
+ };
+ finish_at_any_time(&finished);
+ block();
+}
+
void evaluatable(bool CondVar) {
for (; false && CondVar;) {
}
Index: clang-tools-extra/clang-tidy/utils/Aliasing.cpp
===================================================================
--- clang-tools-extra/clang-tidy/utils/Aliasing.cpp
+++ clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -23,6 +23,13 @@
return false;
}
+static bool capturesByRef(const CXXRecordDecl *RD, const VarDecl *Var) {
+ return llvm::any_of(RD->captures(), [Var](const LambdaCapture &C) {
+ return C.capturesVariable() && C.getCaptureKind() == LCK_ByRef &&
+ C.getCapturedVar() == Var;
+ });
+}
+
/// Return whether \p Var has a pointer or reference in \p S.
static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
// Treat block capture by reference as a form of taking a reference.
@@ -42,10 +49,7 @@
return isAccessForVar(UnOp->getSubExpr(), Var);
} else if (const auto *LE = dyn_cast<LambdaExpr>(S)) {
// Treat lambda capture by reference as a form of taking a reference.
- return llvm::any_of(LE->captures(), [Var](const LambdaCapture &C) {
- return C.capturesVariable() && C.getCaptureKind() == LCK_ByRef &&
- C.getCapturedVar() == Var;
- });
+ return capturesByRef(LE->getLambdaClass(), Var);
}
return false;
@@ -67,8 +71,22 @@
return false;
}
+static bool refersToEnclosingLambdaCaptureByRef(const FunctionDecl *Func,
+ const VarDecl *Var) {
+ const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Func);
+ if (!MD)
+ return false;
+
+ const CXXRecordDecl *RD = MD->getParent();
+ if (!RD->isLambda())
+ return false;
+
+ return capturesByRef(RD, Var);
+}
+
bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var) {
- return hasPtrOrReferenceInStmt(Func->getBody(), Var);
+ return hasPtrOrReferenceInStmt(Func->getBody(), Var) ||
+ refersToEnclosingLambdaCaptureByRef(Func, Var);
}
} // namespace utils
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
