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 @@ -9,6 +9,7 @@ #include "Aliasing.h" #include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" namespace clang { namespace tidy { @@ -24,6 +25,10 @@ /// 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. + if (Var->isEscapingByref()) + return true; + if (const auto *DS = dyn_cast(S)) { for (const Decl *D : DS->getDeclGroup()) { if (const auto *LeftVar = dyn_cast(D)) { @@ -35,6 +40,12 @@ } else if (const auto *UnOp = dyn_cast(S)) { if (UnOp->getOpcode() == UO_AddrOf) 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 false; 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 @@ -1,4 +1,5 @@ -// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- -fexceptions +// RUN: %check_clang_tidy %s bugprone-infinite-loop %t \ +// RUN: -- -- -fexceptions -fblocks void simple_infinite_loop1() { int i = 0; @@ -378,6 +379,84 @@ } while (i < Limit); } +template void accept_callback(T t) { + // Potentially call the callback. + // Possibly on a background thread or something. +} + +void accept_block(void (^)(void)) { + // Potentially call the callback. + // Possibly on a background thread or something. +} + +void wait(void) { + // Wait for the previously passed callback to be called. +} + +void lambda_capture_from_outside() { + bool finished = false; + accept_callback([&]() { + finished = true; + }); + while (!finished) { + wait(); + } +} + +void lambda_capture_from_outside_by_value() { + bool finished = false; + accept_callback([finished]() { + if (finished) {} + }); + while (!finished) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (finished) are updated in the loop body [bugprone-infinite-loop] + wait(); + } +} + +void lambda_capture_from_outside_but_unchanged() { + bool finished = false; + accept_callback([&finished]() { + if (finished) {} + }); + while (!finished) { + // FIXME: Should warn. + wait(); + } +} + +void block_capture_from_outside() { + __block bool finished = false; + accept_block(^{ + finished = true; + }); + while (!finished) { + wait(); + } +} + +void block_capture_from_outside_by_value() { + bool finished = false; + accept_block(^{ + if (finished) {} + }); + while (!finished) { + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (finished) are updated in the loop body [bugprone-infinite-loop] + wait(); + } +} + +void block_capture_from_outside_but_unchanged() { + __block bool finished = false; + accept_block(^{ + if (finished) {} + }); + while (!finished) { + // FIXME: Should warn. + wait(); + } +} + 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 @@ -1,4 +1,5 @@ -// RUN: %check_clang_tidy %s bugprone-redundant-branch-condition %t +// RUN: %check_clang_tidy %s bugprone-redundant-branch-condition %t \ +// RUN: -- -- -fblocks extern unsigned peopleInTheBuilding; extern unsigned fireFighters; @@ -1179,6 +1180,86 @@ } } +// Lambda / block captures. + +template void accept_callback(T t) { + // Potentially call the callback. + // Possibly on a background thread or something. +} + +void accept_block(void (^)(void)) { + // Potentially call the callback. + // Possibly on a background thread or something. +} + +void wait(void) { + // Wait for the previously passed callback to be called. +} + +void capture_and_mutate_by_lambda() { + bool x = true; + accept_callback([&]() { x = false; }); + if (x) { + wait(); + if (x) { + } + } +} + +void lambda_capture_by_value() { + bool x = true; + accept_callback([x]() { if (x) {} }); + if (x) { + wait(); + if (x) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'x' [bugprone-redundant-branch-condition] + } + } +} + +void capture_by_lambda_but_not_mutate() { + bool x = true; + accept_callback([&]() { if (x) {} }); + if (x) { + wait(); + // FIXME: Should warn. + if (x) { + } + } +} + +void capture_and_mutate_by_block() { + __block bool x = true; + accept_block(^{ x = false; }); + if (x) { + wait(); + if (x) { + } + } +} + +void block_capture_by_value() { + bool x = true; + accept_block(^{ if (x) {} }); + if (x) { + wait(); + if (x) { + // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: redundant condition 'x' [bugprone-redundant-branch-condition] + } + } +} + +void capture_by_block_but_not_mutate() { + __block bool x = true; + accept_callback(^{ if (x) {} }); + if (x) { + wait(); + // FIXME: Should warn. + if (x) { + } + } +} + // GNU Expression Statements void negative_gnu_expression_statement() {