https://github.com/Expertcoderz created https://github.com/llvm/llvm-project/pull/226101
This is an NFC refactor of `clang/lib/Sema/SemaStmt.cpp` based on @Sirraide's suggestion in https://github.com/llvm/llvm-project/pull/225748#discussion_r4083103871. - For both `Sema::ActOnForStmt` and `Sema::ActOnWhileStmt`, `CommaVisitor` visiting and empty loop handling have been factored out into a new `CheckConditionalLoop()` function to reduce duplication. - Also added clarifying comments to explain the need for `setHasEmptyLoopBodies()` usage in the specific cases of `for`/`while` loops. This PR is intended to be merged prior to #225748, which will benefit from this refactor by having the redundant-defer checks for both `for`/`while` loops in the same `CheckConditionalLoop()` function instead of duplicating them. >From 0f54aada731474f4ca06dad02a379ad255e9bbae Mon Sep 17 00:00:00 2001 From: Expertcoderz <[email protected]> Date: Thu, 24 Sep 2026 03:44:08 +0000 Subject: [PATCH] [Clang][Sema] Refactor checks on for/while loops (NFC) CommaVisitor and empty loop handling have been moved into a new `CheckConditionalLoop()` function to reduce duplication. Also added clarifying comments to explain the need for `setHasEmptyLoopBodies()` usage in the specific cases of `for`/`while` loops. --- clang/lib/Sema/SemaStmt.cpp | 51 +++++++++++++++++++++++++------------ 1 file changed, 35 insertions(+), 16 deletions(-) diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index 74fe253efa137..ddaa8e11e80b4 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -462,7 +462,12 @@ StmtResult Sema::ActOnCompoundStmt(SourceLocation L, SourceLocation R, } // Check for suspicious empty body (null statement) in `for' and `while' - // statements. Don't do anything for template instantiations, this just adds + // statements, for example: + // + // for (;;); <- warning: for loop has empty body + // foo(); + // + // Don't do anything for template instantiations, this just adds // noise. if (NumElts != 0 && !CurrentInstantiationScope && getCurCompoundScope().HasEmptyLoopBodies) { @@ -1814,18 +1819,36 @@ Sema::DiagnoseAssignmentEnum(QualType DstType, QualType SrcType, << DstType.getUnqualifiedType(); } +// Checks for issues that are common to `for`/`while` statements. +static void CheckConditionalLoop(Sema &S, Expr *CondExpr, Stmt *Body) { + // Check for comma operator misuse. + if (CondExpr && + !S.Diags.isIgnored(diag::warn_comma_operator, CondExpr->getExprLoc())) + CommaVisitor(S).Visit(CondExpr); + + if (isa<NullStmt>(Body)) { + // Tell Sema::ActOnCompoundStmt to perform a check on + // this suspicious empty `for`/`while` loop when + // processing the compound statement that contains this loop. + // + // The actual check cannot be done here directly as it may + // depend on other statements following the `for`/`while` + // loop, in the outer enclosing CompoundStmt; see the + // comment in Sema::ActOnCompoundStmt for an example + // of when this happens. + // + // This does not apply for `if` statements and range-`for` + // loops which call DiagnoseEmptyStmtBody() directly. + S.getCurCompoundScope().setHasEmptyLoopBodies(); + } +} + StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, SourceLocation LParenLoc, ConditionResult Cond, SourceLocation RParenLoc, Stmt *Body) { if (Cond.isInvalid()) return StmtError(); - auto CondVal = Cond.get(); - - if (CondVal.second && - !Diags.isIgnored(diag::warn_comma_operator, CondVal.second->getExprLoc())) - CommaVisitor(*this).Visit(CondVal.second); - // OpenACC3.3 2.14.4: // The update directive is executable. It must not appear in place of the // statement following an 'if', 'while', 'do', 'switch', or 'label' in C or @@ -1835,8 +1858,9 @@ StmtResult Sema::ActOnWhileStmt(SourceLocation WhileLoc, Body = new (Context) NullStmt(Body->getBeginLoc()); } - if (isa<NullStmt>(Body)) - getCurCompoundScope().setHasEmptyLoopBodies(); + auto CondVal = Cond.get(); + + CheckConditionalLoop(*this, CondVal.second, Body); return WhileStmt::Create(Context, CondVal.first, CondVal.second, Body, WhileLoc, LParenLoc, RParenLoc); @@ -2320,14 +2344,9 @@ StmtResult Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc, Body); CheckForRedundantIteration(*this, third.get(), Body); - if (Second.get().second && - !Diags.isIgnored(diag::warn_comma_operator, - Second.get().second->getExprLoc())) - CommaVisitor(*this).Visit(Second.get().second); + CheckConditionalLoop(*this, Second.get().second, Body); - Expr *Third = third.release().getAs<Expr>(); - if (isa<NullStmt>(Body)) - getCurCompoundScope().setHasEmptyLoopBodies(); + Expr *Third = third.release().getAs<Expr>(); return new (Context) ForStmt(Context, First, Second.get().second, Second.get().first, Third, _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
