Internals: Refactor V3Task and add loop assert (#6218)

src/V3Task.cpp

@@ -1435,9 +1435,9 @@ class TaskVisitor final : public VNVisitor {
         VL_RESTORER(m_modp);
         VL_RESTORER(m_modNCalls);
         m_modp = nodep;
-        m_insStmtp = nullptr;
         m_modNCalls = 0;
         iterateChildren(nodep);
+        UASSERT_OBJ(!m_insStmtp, nodep, "Didn't finish out last statement");
     }
     void visit(AstWith* nodep) override {
         if (nodep->user1SetOnce()) {
@@ -1451,8 +1451,8 @@ class TaskVisitor final : public VNVisitor {
     void visit(AstScope* nodep) override {
         VL_RESTORER(m_scopep);
         m_scopep = nodep;
-        m_insStmtp = nullptr;
         iterateChildren(nodep);
+        UASSERT_OBJ(!m_insStmtp, nodep, "Didn't finish out last statement");
     }
     void visit(AstNodeFTaskRef* nodep) override {
         if (m_inSensesp) {
@@ -1486,7 +1486,7 @@ class TaskVisitor final : public VNVisitor {
             ++m_statInlines;
         }
 
-        if (VN_IS(nodep, New)) {
+        if (VN_IS(nodep, New)) {   // New not legal as while() condition
             insertBeforeStmt(nodep, beginp);
             UASSERT_OBJ(cnewp, nodep, "didn't create cnew for new");
             nodep->replaceWith(cnewp);
@@ -1508,7 +1508,7 @@ class TaskVisitor final : public VNVisitor {
             nodep->replaceWith(beginp);
             VL_DO_DANGLING(nodep->deleteTree(), nodep);
             VIsCached::clearCacheTree();
-        } else {
+        } else {  // VN_IS(nodep->backp(), StmtExpr)
             insertBeforeStmt(nodep, beginp);
             if (nodep->taskp()->isFunction()) {
                 nodep->v3warn(
@@ -1588,16 +1588,18 @@ class TaskVisitor final : public VNVisitor {
     }
     void visit(AstWhile* nodep) override {
         // Special, as statements need to be put in different places
-        // Conditions insert first at end of precondsp.
+        {
-        // TODO: is this right? This is how it used to be.
+            // Conditions will create a StmtExpr
-        m_insStmtp = nodep;
+            // Leave m_instStmtp = null, so will assert if not
-        iterateAndNextNull(nodep->condp());
+            iterateAndNextNull(nodep->condp());
-        // Body insert just before themselves
+        }
-        m_insStmtp = nullptr;  // First thing should be new statement
+        {
-        iterateAndNextNull(nodep->stmtsp());
+            // Body insert just before themselves
-        iterateAndNextNull(nodep->incsp());
+            VL_RESTORER(m_insStmtp);
-        // Done the loop
+            m_insStmtp = nullptr;  // First thing should be new statement
-        m_insStmtp = nullptr;  // Next thing should be new statement
+            iterateAndNextNull(nodep->stmtsp());
+            iterateAndNextNull(nodep->incsp());
+        }
     }
     void visit(AstNodeForeach* nodep) override {  // LCOV_EXCL_LINE
         nodep->v3fatalSrc(
@@ -1608,15 +1610,15 @@ class TaskVisitor final : public VNVisitor {
             "For statements should have been converted to while statements in V3Begin.cpp");
     }
     void visit(AstNodeStmt* nodep) override {
+        VL_RESTORER(m_insStmtp);
         m_insStmtp = nodep;
         iterateChildren(nodep);
-        m_insStmtp = nullptr;  // Next thing should be new statement
     }
     void visit(AstStmtExpr* nodep) override {
+        VL_RESTORER(m_insStmtp);
         m_insStmtp = nodep;
         iterateChildren(nodep);
         if (!nodep->exprp()) VL_DO_DANGLING(nodep->unlinkFrBack()->deleteTree(), nodep);
-        m_insStmtp = nullptr;  // Next thing should be new statement
     }
     void visit(AstSenItem* nodep) override {
         UASSERT_OBJ(!m_inSensesp, nodep, "Senitem under senitem?");

test_regress/t/t_func_while2.py

@@ -0,0 +1,18 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2025 by Wilson Snyder. This program is free software; you
+# can redistribute it and/or modify it under the terms of either the GNU
+# Lesser General Public License Version 3 or the Perl Artistic License
+# Version 2.0.
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+import vltest_bootstrap
+
+test.scenarios('simulator_st')
+
+test.compile()
+
+test.execute()
+
+test.passes()

test_regress/t/t_func_while2.v

@@ -0,0 +1,36 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2025 by Wilson Snyder.
+// SPDX-License-Identifier: CC0-1.0
+
+// verilog_format: off
+`define stop $stop
+`define checks(gotv,expv) do if ((gotv) != (expv)) begin $write("%%Error: %s:%0d:  got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0);
+// verilog_format: on
+
+module t;
+
+  int i;
+  string value;
+
+  function automatic int count();
+    ++i;
+    value = {value, $sformatf(" count%0d", i)};
+    return i;
+  endfunction
+
+  initial begin
+    value = "";
+    i = 0;
+    while (count() <= 2) begin
+      // verilator unroll_disable
+      value = {value, " loop"};
+    end
+    `checks(value, " count1 loop count2 loop count3");
+
+    $write("*-* All Finished *-*\n");
+    $finish;
+  end
+
+endmodule