diff --git a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp --- a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp +++ b/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(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 auto *MD = dyn_cast(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 --- 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 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 --- 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 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() {