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

[NoQ]

Summary

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

[vsavchenko]

Now, when we are done with forCallable, let's get to this one.
It looks great to me, thanks for fixing it!

clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp

 using clang::tidy::utils::hasPtrOrReferenceInFunc;
 
 namespace clang {
 namespace tidy {
 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())))));
 }
 
 /// Return whether `Var` was changed in `LoopStmt`.
 static bool isChanged(const Stmt *LoopStmt, const VarDecl *Var,
                       ASTContext *Context) {
   if (const auto *ForLoop = dyn_cast<ForStmt>(LoopStmt))
     return (ForLoop->getInc() &&
             ExprMutationAnalyzer(*ForLoop->getInc(), *Context)
                 .isMutated(Var)) ||
            (ForLoop->getBody() &&
             ExprMutationAnalyzer(*ForLoop->getBody(), *Context)
                 .isMutated(Var)) ||
            (ForLoop->getCond() &&
             ExprMutationAnalyzer(*ForLoop->getCond(), *Context).isMutated(Var));
 
   return ExprMutationAnalyzer(*LoopStmt, *Context).isMutated(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())
         return true;
 
       if (Var->getType().isVolatileQualified())
         return true;
 
       if (!Var->getType().getTypePtr()->isIntegerType())
         return true;
 
       return hasPtrOrReferenceInFunc(Func, Var) ||
              isChanged(LoopStmt, Var, Context);
       // FIXME: Track references.
     }
   } else if (isa<MemberExpr>(Cond) || isa<CallExpr>(Cond)) {
     // FIXME: Handle MemberExpr.
     return true;
   }
 
   return false;
 }
 
 /// 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;
 
   for (const Stmt *Child : Cond->children()) {
     if (!Child)
       continue;
 
     if (isAtLeastOneCondVarChanged(Func, LoopStmt, Child, Context))
static bool isKnownFalse(const Expr &Cond, const ASTContext &Ctx) {
   if (Cond.EvaluateAsBooleanCondition(Result, Ctx))
     return !Result;
   return false;
 }
 
 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)
                          .bind("loop-stmt"),
                      this);
 }
 
 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;
 
   bool ShouldHaveConditionVariables = true;
   if (const auto *While = dyn_cast<WhileStmt>(LoopStmt)) {
     if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {
       if (const Expr *Init = LoopVarDecl->getInit()) {

clang-tools-extra/clang-tidy/utils/Aliasing.h

 ///   int n;
 ///   ...
 ///   int *p = &n;
 /// }
 ///
 /// 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
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_ALIASING_H

clang-tools-extra/clang-tidy/utils/Aliasing.cpp

for (const Stmt *Child : S->children()) {
 
     if (hasPtrOrReferenceInStmt(Child, Var))
       return true;
   }
 
   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)
     return false;
 
   const CXXRecordDecl *RD = MD->getParent();
   if (!RD->isLambda())
     return false;
 
   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);
 }
 
 } // namespace utils
 } // namespace tidy
 } // namespace clang

clang-tools-extra/test/clang-tidy/checkers/bugprone-infinite-loop.cpp

void lambda_capture_with_loop_inside_lambda_good_init_capture() {
   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.
+      // 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);
   block();
 }
 
 void block_capture_with_loop_inside_block_bad_simpler() {

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

+// RUN: %check_clang_tidy %s bugprone-infinite-loop %t -- -- -fblocks
+
+@interface I
+-(void) instanceMethod;
++(void) classMethod;
+@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++;
+  }
+}
+
+@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++;
+  }
+}
+@end