From 63ecb7dcc80d17770461c8bf01bddeb2b795625b Mon Sep 17 00:00:00 2001 From: usama hameed Date: Mon, 23 May 2022 14:52:14 -0700 Subject: [PATCH] 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 --- .../clang-tidy/bugprone/InfiniteLoopCheck.cpp | 18 +----- .../Analysis/Analyses/ExprMutationAnalyzer.h | 2 + clang/lib/Analysis/ExprMutationAnalyzer.cpp | 62 +++++++++++-------- 3 files changed, 38 insertions(+), 44 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/InfiniteLoopCheck.cpp index cb9bd7bb43f5..8815de220882 100644 --- 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(Cond)) { - if (const auto *ForLoop = dyn_cast(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 index 1b2b7ffcfb67..a48b9735dfee 100644 --- a/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h +++ b/clang/include/clang/Analysis/Analyses/ExprMutationAnalyzer.h @@ -38,6 +38,8 @@ public: } 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 index 4d7decc1137c..135327a3cfe5 100644 --- 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( + return selectFirst(NodeID::value, + match(findAll(expr(canResolveToExpr(equalsNode(Exp)), + isUnevaluatedMatcher(Exp)) + .bind(NodeID::value)), + Stm, Context)) != nullptr; +} + +bool ExprMutationAnalyzer::isUnevaluated(const Stmt *Exp, const Stmt &Stm, + ASTContext &Context) { + return selectFirst( NodeID::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::value)), - Stm, Context)) != nullptr; + match(findAll(stmt(equalsNode(Exp), isUnevaluatedMatcher(Exp)) + .bind(NodeID::value)), + Stm, Context)) != nullptr; } bool ExprMutationAnalyzer::isUnevaluated(const Expr *Exp) {