[clang] [Clang][Sema] Refactor checks on for/while loops (NFC) (PR #226101)

[via cfe-commits]

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 <expertcoderzx at gmail.com>
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,