[clang-tools-extra] 9b292e0 - [clang-tidy] Aliasing: Add more support for captures.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Mon May 10 14:00:40 PDT 2021
Author: Artem Dergachev
Date: 2021-05-10T14:00:30-07:00
New Revision: 9b292e0edcd4e889dbcf4bbaad6c1cc80fffcfd1
URL: https://github.com/llvm/llvm-project/commit/9b292e0edcd4e889dbcf4bbaad6c1cc80fffcfd1
DIFF: https://github.com/llvm/llvm-project/commit/9b292e0edcd4e889dbcf4bbaad6c1cc80fffcfd1.diff
LOG: [clang-tidy] Aliasing: Add more support for captures.
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 D96215.
Differential Revision: https://reviews.llvm.org/D101787
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
index f9eb147ce6720..12a8ca8185b0f 100644
--- a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
+++ b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -23,6 +23,13 @@ static bool isAccessForVar(const Stmt *S, const VarDecl *Var) {
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 @@ static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) {
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 @@ static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
return false;
}
+static bool refersToEnclosingLambdaCaptureByRef(const FunctionDecl *Func,
+ const VarDecl *Var) {
+ const auto *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
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
index d6cb954a3beb6..5b477130a7b01 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp
@@ -457,6 +457,92 @@ void block_capture_from_outside_but_unchanged() {
}
}
+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;) {
}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
index 0865b2a1930e0..9b84406e70e03 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-redundant-branch-condition.cpp
@@ -1260,6 +1260,71 @@ void capture_by_block_but_not_mutate() {
}
}
+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() {
More information about the cfe-commits
mailing list