bugfix in InfiniteLoopCheck to not print warnings for unevaluated loops

Added a separate check for unevaluated statements. Updated InfiniteLoopCheck to use new check Differential Revision: https://reviews.llvm.org/D126246
llvm · May 24, 2022 · 63ecb7d · 63ecb7d
1 parent 6026822
commit 63ecb7d
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 44 deletions.
diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp
@@ -81,22 +81,6 @@ static bool isVarThatIsPossiblyChanged(const Decl *Func, const Stmt *LoopStmt,
   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) {
@@ -193,7 +177,7 @@ void InfiniteLoopCheck::check(const MatchFinder::MatchResult &Result) {
     }
   }
 
-  if (isUnevaluated(Func, LoopStmt, Cond, Result.Context))
+  if (ExprMutationAnalyzer::isUnevaluated(LoopStmt, *LoopStmt, *Result.Context))
     return;
 
   if (isAtLeastOneCondVarChanged(Func, LoopStmt, Cond, Result.Context))

diff --git a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h
@@ -38,6 +38,8 @@ class ExprMutationAnalyzer {
   }
   const Stmt *findPointeeMutation(const Expr *Exp);
   const Stmt *findPointeeMutation(const Decl *Dec);
+  static bool isUnevaluated(const Stmt *Smt, const Stmt &Stm,
+                            ASTContext &Context);
   static bool isUnevaluated(const Expr *Exp, const Stmt &Stm,
                             ASTContext &Context);
 

diff --git a/clang/lib/Analysis/ExprMutationAnalyzer.cpp b/clang/lib/Analysis/ExprMutationAnalyzer.cpp
@@ -194,36 +194,44 @@ const Stmt *ExprMutationAnalyzer::tryEachDeclRef(const Decl *Dec,
   return nullptr;
 }
 
+auto isUnevaluatedMatcher(const Stmt *Exp) {
+  return anyOf(
+      // `Exp` is part of the underlying expression of
+      // decltype/typeof if it has an ancestor of
+      // typeLoc.
+      hasAncestor(typeLoc(unless(hasAncestor(unaryExprOrTypeTraitExpr())))),
+      hasAncestor(expr(anyOf(
+          // `UnaryExprOrTypeTraitExpr` is unevaluated
+          // unless it's sizeof on VLA.
+          unaryExprOrTypeTraitExpr(
+              unless(sizeOfExpr(hasArgumentOfType(variableArrayType())))),
+          // `CXXTypeidExpr` is unevaluated unless it's
+          // applied to an expression of glvalue of
+          // polymorphic class type.
+          cxxTypeidExpr(unless(isPotentiallyEvaluated())),
+          // The controlling expression of
+          // `GenericSelectionExpr` is unevaluated.
+          genericSelectionExpr(
+              hasControllingExpr(hasDescendant(equalsNode(Exp)))),
+          cxxNoexceptExpr()))));
+}
+
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp, const Stmt &Stm,
                                          ASTContext &Context) {
-  return selectFirst<Expr>(
+  return selectFirst<Expr>(NodeID<Expr>::value,
+                           match(findAll(expr(canResolveToExpr(equalsNode(Exp)),
+                                              isUnevaluatedMatcher(Exp))
+                                             .bind(NodeID<Expr>::value)),
+                                 Stm, Context)) != nullptr;
+}
+
+bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm,
+                                         ASTContext &Context) {
+  return selectFirst<Stmt>(
              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.
-                              hasAncestor(typeLoc(unless(
-                                  hasAncestor(unaryExprOrTypeTraitExpr())))),
-                              hasAncestor(expr(anyOf(
-                                  // `UnaryExprOrTypeTraitExpr` is unevaluated
-                                  // unless it's sizeof on VLA.
-                                  unaryExprOrTypeTraitExpr(unless(sizeOfExpr(
-                                      hasArgumentOfType(variableArrayType())))),
-                                  // `CXXTypeidExpr` is unevaluated unless it's
-                                  // applied to an expression of glvalue of
-                                  // polymorphic class type.
-                                  cxxTypeidExpr(
-                                      unless(isPotentiallyEvaluated())),
-                                  // The controlling expression of
-                                  // `GenericSelectionExpr` is unevaluated.
-                                  genericSelectionExpr(hasControllingExpr(
-                                      hasDescendant(equalsNode(Exp)))),
-                                  cxxNoexceptExpr())))))
-                         .bind(NodeID<Expr>::value)),
-                 Stm, Context)) != nullptr;
+             match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp))
+                               .bind(NodeID<Expr>::value)),
+                   Stm, Context)) != nullptr;
 }
 
 bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {