[clang-tools-extra] 43f4331 - [clang-tidy] Aliasing: Add support for captures.

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Mon May 10 14:00:38 PDT 2021


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() {


        


More information about the cfe-commits mailing list