[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

diff --git a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
index f9eb147..12a8ca8 100644 (file)
--- 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 d6cb954..5b47713 100644 (file)
--- 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 0865b2a..9b84406 100644 (file)
--- 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() {