Author: Artem Dergachev Date: 2021-05-10T14:00:30-07:00 New Revision: 43f4331edfb595979f6854351d24f9a9219595fa
URL: https://github.com/llvm/llvm-project/commit/43f4331edfb595979f6854351d24f9a9219595fa DIFF: https://github.com/llvm/llvm-project/commit/43f4331edfb595979f6854351d24f9a9219595fa.diff LOG: [clang-tidy] Aliasing: Add support for captures. The utility function clang::tidy::utils::hasPtrOrReferenceInFunc() scans the function for pointer/reference aliases to a given variable. It currently scans for operator & over that variable and for declarations of references to that variable. This patch makes it also scan for C++ lambda captures by reference and for Objective-C block captures. Differential Revision: https://reviews.llvm.org/D96215 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 3a88126a9ee6a..f9eb147ce6720 100644 --- 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 @@ static bool isAccessForVar(const Stmt *S, const VarDecl *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. + if (Var->isEscapingByref()) + return true; + if (const auto *DS = dyn_cast<DeclStmt>(S)) { for (const Decl *D : DS->getDeclGroup()) { if (const auto *LeftVar = dyn_cast<VarDecl>(D)) { @@ -35,6 +40,12 @@ static bool isPtrOrReferenceForVar(const Stmt *S, const VarDecl *Var) { } else if (const auto *UnOp = dyn_cast<UnaryOperator>(S)) { if (UnOp->getOpcode() == UO_AddrOf) 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 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 index 0b1baf04fee2f..d6cb954a3beb6 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 @@ -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 @@ void lambda_capture() { } while (i < Limit); } +template <typename T> 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 index e59d664628993..0865b2a1930e0 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 @@ -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 @@ void negative_else_branch(bool isHot) { } } +// Lambda / block captures. + +template <typename T> 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() { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
