[clang-tidy] bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

[NoQ]

Thx Dmitry!

Usama, I heard you have plans to make isUnevaluated() consume statements so that you could feed the entire loop into the machine without separating it into parts. You can either update this patch or do that as a follow-up patch.

Also I think it's time for you to ask for commit access, so please do! (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)

[usama54321]

Added a separate check for unevaluated statements. Updated InfiniteLoopCheck to use new check

[usama54321]

Updating patch

[NoQ]

clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h

43–44

I suspect this may lead to subtle bugs when a Stmt * that actually points to an Expr at runtime would cause the first overload to be chosen.

Maybe teach canResolveToExpr() to accept an Stmt instead (with early return)?

clang/lib/Analysis/ExprMutationAnalyzer.cpp

199

We're trying to avoid auto when it makes the code less readable i.e. when the type isn't obvious from the context (https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable).

[usama54321]

• updated canResolveToExpr to accept both statements and expressions. Removed unnecessary code

[NoQ]

clang/lib/Analysis/ExprMutationAnalyzer.cpp

229–231

Shouldn't this one be removed now?

[NoQ]

Whoops, no, this isn't an actual concern. Looks good then, please commit!

clang/lib/Analysis/ExprMutationAnalyzer.cpp

229–231

Nvm, it's the old interface.

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

while (true) {
 
       T = T->getPointeeType();
     }
   }
 
   return false;
 }
 
+bool isUnevaluated(const Decl *Func, const Stmt *LoopStmt, const Stmt *Cond,
+                   ASTContext *Context) {
+  if (const auto *Exp = dyn_cast<Expr>(Cond)) {
+    if (const auto *ForLoop = dyn_cast<ForStmt>(LoopStmt)) {
+      return (ForLoop->getInc() && ExprMutationAnalyzer::isUnevaluated(
+                                       Exp, *ForLoop->getInc(), *Context)) ||
+             (ForLoop->getBody() && ExprMutationAnalyzer::isUnevaluated(
+                                        Exp, *ForLoop->getBody(), *Context)) ||
+             (ForLoop->getCond() && ExprMutationAnalyzer::isUnevaluated(
+                                        Exp, *ForLoop->getCond(), *Context));
+    }
+    return ExprMutationAnalyzer::isUnevaluated(Exp, *LoopStmt, *Context);
+  }
+  return true;
+}
+
 /// Return whether at least one variable of `Cond` changed in `LoopStmt`.
 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)
if (const auto *While = dyn_cast<WhileStmt>(LoopStmt)) {
     if (const VarDecl *LoopVarDecl = While->getConditionVariable()) {
       if (const Expr *Init = LoopVarDecl->getInit()) {
         ShouldHaveConditionVariables = false;
         Cond = Init;
       }
     }
   }
 
+  if (isUnevaluated(Func, LoopStmt, Cond, Result.Context))
+    return;
+
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))
     return;
 
   std::string CondVarNames = getCondVarNames(Cond);
   if (ShouldHaveConditionVariables && CondVarNames.empty())
     return;
 
   if (CondVarNames.empty()) {

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

void test_dependent_condition() {
   do {
     // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: this loop is infinite; none of its condition variables (val3) are updated in the loop body [bugprone-infinite-loop]
   } while (1, (true) && val3 == 1);
 
   const int val4 = some_template_fn<T>();
   do {
   } while (1, (false) && val4 == 1);
 }
+
+void test_typeof() {
+  __typeof__({
+    for (int i = 0; i < 10; ++i) {
+    }
+    0;
+  }) x;
+}
+
+void test_typeof_infinite() {
+  __typeof__({
+    for (int i = 0; i < 10;) {
+    }
+    0;
+  }) x;
+}
+
+void test_typeof_while_infinite() {
+  __typeof__({
+    int i = 0;
+    while (i < 10) {
+    }
+    0;
+  }) x;
+}
+
+void test_typeof_dowhile_infinite() {
+  __typeof__({
+    int i = 0;
+    do {
+
+    } while (i < 10);
+    0;
+  }) x;
+}

clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h

public:
   bool isPointeeMutated(const Expr *Exp) {
     return findPointeeMutation(Exp) != nullptr;
   }
   bool isPointeeMutated(const Decl *Dec) {
     return findPointeeMutation(Dec) != nullptr;
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
+                            ASTContext &Context);
 
 private:
   using MutationFinder = const Stmt *(ExprMutationAnalyzer::*)(const Expr *);
   using ResultMap = llvm::DenseMap<const Expr *, const Stmt *>;
 
   const Stmt *findMutationMemoized(const Expr *Exp,
                                    llvm::ArrayRef<MutationFinder> Finders,
                                    ResultMap &MemoizedResults);
   const Stmt *tryEachDeclRef(const Decl *Dec, MutationFinder Finder);
 

clang/lib/Analysis/ExprMutationAnalyzer.cpp

const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const Decl *Dec,
   for (const auto &RefNodes : Refs) {
     const auto *E = RefNodes.getNodeAs<Expr>(NodeID<Expr>::value);
     if ((this->*Finder)(E))
       return E;
   }
   return nullptr;
 }
 
-bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
+                                         ASTContext &Context) {
   return selectFirst<Expr>(
              NodeID<Expr>::value,
              match(
                  findAll(
                      expr(canResolveToExpr(equalsNode(Exp)),
                           anyOf(
                               // `Exp` is part of the underlying expression of
                               // decltype/typeof if it has an ancestor of
                               // typeLoc.
return selectFirst<Expr>(
                                   // `GenericSelectionExpr` is unevaluated.
                                   genericSelectionExpr(hasControllingExpr(
                                       hasDescendant(equalsNode(Exp)))),
                                   cxxNoexceptExpr())))))
                          .bind(NodeID<Expr>::value)),
                  Stm, Context)) != nullptr;
 }
 
+bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {
+  return isUnevaluated(Exp, Stm, Context);
+}
+
 const Stmt *
 ExprMutationAnalyzer::findExprMutation(ArrayRef<BoundNodes> Matches) {
   return tryEachMatch<Expr>(Matches, this, &ExprMutationAnalyzer::findMutation);
 }
 
 const Stmt *
 ExprMutationAnalyzer::findDeclMutation(ArrayRef<BoundNodes> Matches) {
   return tryEachMatch<Decl>(Matches, this, &ExprMutationAnalyzer::findMutation);