[clang-tools-extra] 46c6c08 - [clang-tidy] bugprone-infinite-loop: forFunction() -> forCallable().

Artem Dergachev via cfe-commits cfe-commits at lists.llvm.org
Thu May 13 11:25:12 PDT 2021


Author: Artem Dergachev
Date: 2021-05-13T11:25:01-07:00
New Revision: 46c6c08c9428a36bdf88f51b1a14164aa4cbb93e

URL: https://github.com/llvm/llvm-project/commit/46c6c08c9428a36bdf88f51b1a14164aa4cbb93e
DIFF: https://github.com/llvm/llvm-project/commit/46c6c08c9428a36bdf88f51b1a14164aa4cbb93e.diff

LOG: [clang-tidy] bugprone-infinite-loop: forFunction() -> forCallable().

Take advantage of the new ASTMatcher added in D102213 to fix massive false negatives of the infinite loop checker on Objective-C.

Differential Revision: https://reviews.llvm.org/D102214

Added: 
    clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm

Modified: 
    clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
    clang-tools-extra/clang-tidy/utils/Aliasing.cpp
    clang-tools-extra/clang-tidy/utils/Aliasing.h
    clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
index 777d309b5ccb1..ab62b128d700f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -21,6 +21,7 @@ namespace bugprone {
 
 static internal::Matcher<Stmt>
 loopEndingStmt(internal::Matcher<Stmt> Internal) {
+  // FIXME: Cover noreturn ObjC methods (and blocks?).
   return stmt(anyOf(
       mapAnyOf(breakStmt, returnStmt, gotoStmt, cxxThrowExpr).with(Internal),
       callExpr(Internal, callee(functionDecl(isNoReturn())))));
@@ -43,9 +44,8 @@ static bool isChanged(const Stmt *LoopStmt, const VarDecl *Var,
 }
 
 /// Return whether `Cond` is a variable that is possibly changed in `LoopStmt`.
-static bool isVarThatIsPossiblyChanged(const FunctionDecl *Func,
-                                       const Stmt *LoopStmt, const Stmt *Cond,
-                                       ASTContext *Context) {
+static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
+                                       const Stmt *Cond, ASTContext *Context) {
   if (const auto *DRE = dyn_cast<DeclRefExpr>(Cond)) {
     if (const auto *Var = dyn_cast<VarDecl>(DRE->getDecl())) {
       if (!Var->isLocalVarDeclOrParm())
@@ -70,9 +70,8 @@ static bool isVarThatIsPossiblyChanged(const FunctionDecl *Func,
 }
 
 /// Return whether at least one variable of `Cond` changed in `LoopStmt`.
-static bool isAtLeastOneCondVarChanged(const FunctionDecl *Func,
-                                       const Stmt *LoopStmt, const Stmt *Cond,
-                                       ASTContext *Context) {
+static bool isAtLeastOneCondVarChanged(const Decl *Func, const Stmt *LoopStmt,
+                                       const Stmt *Cond, ASTContext *Context) {
   if (isVarThatIsPossiblyChanged(Func, LoopStmt, Cond, Context))
     return true;
 
@@ -118,9 +117,9 @@ static bool isKnownFalse(const Expr &Cond, const ASTContext &Ctx) {
 void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) {
   const auto LoopCondition = allOf(
       hasCondition(
-          expr(forFunction(functionDecl().bind("func"))).bind("condition")),
+          expr(forCallable(decl().bind("func"))).bind("condition")),
       unless(hasBody(hasDescendant(
-          loopEndingStmt(forFunction(equalsBoundNode("func")))))));
+          loopEndingStmt(forCallable(equalsBoundNode("func")))))));
 
   Finder->addMatcher(mapAnyOf(whileStmt, doStmt, forStmt)
                          .with(LoopCondition)
@@ -131,7 +130,7 @@ void InfiniteLoopCheck::registerMatchers(MatchFinder *Finder) {
 void InfiniteLoopCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Cond = Result.Nodes.getNodeAs<Expr>("condition");
   const auto *LoopStmt = Result.Nodes.getNodeAs<Stmt>("loop-stmt");
-  const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("func");
+  const auto *Func = Result.Nodes.getNodeAs<Decl>("func");
 
   if (isKnownFalse(*Cond, *Result.Context))
     return;

diff  --git a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
index 69aea1145de50..c272453761a6b 100644
--- a/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
+++ b/clang-tools-extra/clang-tidy/utils/Aliasing.cpp
@@ -78,7 +78,7 @@ static bool hasPtrOrReferenceInStmt(const Stmt *S, const VarDecl *Var) {
   return false;
 }
 
-static bool refersToEnclosingLambdaCaptureByRef(const FunctionDecl *Func,
+static bool refersToEnclosingLambdaCaptureByRef(const Decl *Func,
                                                 const VarDecl *Var) {
   const auto *MD = dyn_cast<CXXMethodDecl>(Func);
   if (!MD)
@@ -91,7 +91,7 @@ static bool refersToEnclosingLambdaCaptureByRef(const FunctionDecl *Func,
   return capturesByRef(RD, Var);
 }
 
-bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var) {
+bool hasPtrOrReferenceInFunc(const Decl *Func, const VarDecl *Var) {
   return hasPtrOrReferenceInStmt(Func->getBody(), Var) ||
          refersToEnclosingLambdaCaptureByRef(Func, Var);
 }

diff  --git a/clang-tools-extra/clang-tidy/utils/Aliasing.h b/clang-tools-extra/clang-tidy/utils/Aliasing.h
index e43995a6714ad..668c04a2ac22c 100644
--- a/clang-tools-extra/clang-tidy/utils/Aliasing.h
+++ b/clang-tools-extra/clang-tidy/utils/Aliasing.h
@@ -27,7 +27,7 @@ namespace utils {
 /// For `f()` and `n` the function returns ``true`` because `p` is a
 /// pointer to `n` created in `f()`.
 
-bool hasPtrOrReferenceInFunc(const FunctionDecl *Func, const VarDecl *Var);
+bool hasPtrOrReferenceInFunc(const Decl *Func, const VarDecl *Var);
 
 } // namespace utils
 } // namespace tidy

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 2765b181db2c5..6afde606481c1 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
@@ -511,8 +511,7 @@ 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.
+      // 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();
     }
   };

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
new file mode 100644
index 0000000000000..77d85acf248e7
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.mm
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- -fblocks
+
+ at interface I
+-(void) instanceMethod;
++(void) classMethod;
+ at end
+
+void plainCFunction() {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+    j++;
+  }
+}
+
+ at implementation I
+- (void)instanceMethod {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+    j++;
+  }
+}
+
++ (void)classMethod {
+  int i = 0;
+  int j = 0;
+  while (i < 10) {
+    // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (i) are updated in the loop body [bugprone-infinite-loop]
+    j++;
+  }
+}
+ at end


        


More information about the cfe-commits mailing list