Refactor visitor structure to successively collect state vs having multiple flag-controlled passes

Signed-off-by: Matthew Ballance <matt.ballance@gmail.com>

src/V3Active.cpp

@@ -620,59 +620,78 @@ public:
 };
 
 //######################################################################
-// Automatic covergroup sampling visitor
+// Shared state for covergroup sampling passes
-// This runs after ActiveVisitor to add automatic sample() calls for covergroups
-// declared with sensitivity events (e.g., covergroup cg @(posedge clk);)
 
-class CovergroupSamplingVisitor final : public VNVisitor {
+struct CovergroupState final {
-    // STATE
-    ActiveNamer m_namer;  // Reuse active naming infrastructure
-    bool m_inFirstPass = true;  // First pass collects CFuncs, second pass adds sampling
     std::unordered_map<const AstClass*, AstCFunc*>
-        m_covergroupSampleFuncs;  // Class -> sample CFunc
+        m_sampleFuncs;  // Class -> sample CFunc
     std::unordered_map<const AstClass*, AstSenTree*>
-        m_covergroupEvents;  // Class -> sampling event (if any)
+        m_samplingEvents;  // Class -> owned sampling event (if any)
+};
+
+//######################################################################
+// Pass 1: collect sample CFuncs and sampling events from covergroup class scopes
+
+class CovergroupCollectVisitor final : public VNVisitor {
+    // STATE
+    CovergroupState& m_state;
+
+    // VISITORS
+    void visit(AstScope* nodep) override {
+        AstClass* const classp = VN_CAST(nodep->modp(), Class);
+        if (classp && classp->isCovergroup()) {
+            for (AstNode* itemp = nodep->blocksp(); itemp; itemp = itemp->nextp()) {
+                if (AstCFunc* const cfuncp = VN_CAST(itemp, CFunc)) {
+                    if (cfuncp->name().find("sample") != string::npos) {
+                        m_state.m_sampleFuncs[classp] = cfuncp;
+                        cfuncp->isCovergroupSample(true);
+                        break;
+                    }
+                }
+            }
+            for (AstNode* memberp = classp->membersp(); memberp;) {
+                AstNode* const nextp = memberp->nextp();
+                if (AstCovergroup* const cgp = VN_CAST(memberp, Covergroup)) {
+                    // Unlink eventp from cgp so it survives cgp's deletion,
+                    // then take ownership in the map for use during the second pass.
+                    if (cgp->eventp())
+                        m_state.m_samplingEvents[classp] = cgp->eventp()->unlinkFrBack();
+                    cgp->unlinkFrBack();
+                    VL_DO_DANGLING(cgp->deleteTree(), cgp);
+                    break;
+                }
+                memberp = nextp;
+            }
+        }
+        iterateChildren(nodep);
+    }
+
+    void visit(AstNode* nodep) override { iterateChildren(nodep); }
+
+public:
+    // CONSTRUCTORS
+    CovergroupCollectVisitor(AstNetlist* nodep, CovergroupState& state)
+        : m_state{state} {
+        iterate(nodep);
+    }
+    ~CovergroupCollectVisitor() override = default;
+};
+
+//######################################################################
+// Pass 2: inject automatic sample() calls for covergroup instances
+
+class CovergroupInjectVisitor final : public VNVisitor {
+    // STATE
+    CovergroupState& m_state;
+    ActiveNamer m_namer;  // Reuse active naming infrastructure
 
     // VISITORS
     void visit(AstScope* nodep) override {
         m_namer.main(nodep);  // Initialize active naming for this scope
-
-        // First pass: collect sample CFuncs and sampling events from covergroup class scopes
-        if (m_inFirstPass) {
-            AstClass* const classp = VN_CAST(nodep->modp(), Class);
-            if (classp && classp->isCovergroup()) {
-                for (AstNode* itemp = nodep->blocksp(); itemp; itemp = itemp->nextp()) {
-                    if (AstCFunc* const cfuncp = VN_CAST(itemp, CFunc)) {
-                        if (cfuncp->name().find("sample") != string::npos) {
-                            m_covergroupSampleFuncs[classp] = cfuncp;
-                            cfuncp->isCovergroupSample(true);
-                            break;
-                        }
-                    }
-                }
-                for (AstNode* memberp = classp->membersp(); memberp;) {
-                    AstNode* const nextp = memberp->nextp();
-                    if (AstCovergroup* const cgp = VN_CAST(memberp, Covergroup)) {
-                        // Unlink eventp from cgp so it survives cgp's deletion,
-                        // then take ownership in the map for use during the second pass.
-                        if (cgp->eventp())
-                            m_covergroupEvents[classp] = cgp->eventp()->unlinkFrBack();
-                        cgp->unlinkFrBack();
-                        VL_DO_DANGLING(cgp->deleteTree(), cgp);
-                        break;
-                    }
-                    memberp = nextp;
-                }
-            }
-        }
-
         iterateChildren(nodep);
     }
 
     void visit(AstVarScope* nodep) override {
-        // Only process VarScopes in the second pass
-        if (m_inFirstPass) return;
-
         // Get the underlying var
         AstVar* const varp = nodep->varp();
         if (!varp) return;
@@ -688,14 +707,14 @@ class CovergroupSamplingVisitor final : public VNVisitor {
         if (!classp || !classp->isCovergroup()) return;
 
         // Check if this covergroup has an automatic sampling event
-        const auto evtIt = m_covergroupEvents.find(classp);
+        const auto evtIt = m_state.m_samplingEvents.find(classp);
-        if (evtIt == m_covergroupEvents.end())
+        if (evtIt == m_state.m_samplingEvents.end())
             return;  // No automatic sampling for this covergroup
         AstSenTree* const eventp = evtIt->second;
 
         // Get the sample CFunc from the map populated during the first pass
-        const auto it = m_covergroupSampleFuncs.find(classp);
+        const auto it = m_state.m_sampleFuncs.find(classp);
-        if (it == m_covergroupSampleFuncs.end()) {
+        if (it == m_state.m_sampleFuncs.end()) {
             UINFO(4, "Could not find sample() CFunc for covergroup " << classp->name() << endl);
             return;
         }
@@ -737,25 +756,11 @@ class CovergroupSamplingVisitor final : public VNVisitor {
 
 public:
     // CONSTRUCTORS
-    explicit CovergroupSamplingVisitor(AstNetlist* nodep) {
+    CovergroupInjectVisitor(AstNetlist* nodep, CovergroupState& state)
-
+        : m_state{state} {
-        UINFO(4, "CovergroupSamplingVisitor: Starting" << endl);
-
-        // First pass: collect sample CFuncs and sampling events; delete AstCovergroup holders
-        m_inFirstPass = true;
         iterate(nodep);
-
-        // Second pass: add automatic sampling to covergroup instances
-        m_inFirstPass = false;
-        iterate(nodep);
-
-        // Release the owned AstSenTree nodes that were unlinked from AstCovergroup during
-        // the first pass; they are no longer needed after all clones have been made.
-        for (const auto& itpair : m_covergroupEvents) itpair.second->deleteTree();
-
-        UINFO(4, "CovergroupSamplingVisitor: Complete" << endl);
     }
-    ~CovergroupSamplingVisitor() override = default;
+    ~CovergroupInjectVisitor() override = default;
 };
 
 //######################################################################
@@ -764,6 +769,12 @@ public:
 void V3Active::activeAll(AstNetlist* nodep) {
     UINFO(2, __FUNCTION__ << ":");
     { ActiveVisitor{nodep}; }  // Destruct before checking
-    { CovergroupSamplingVisitor{nodep}; }  // Add automatic covergroup sampling
+    {
+        // Add automatic covergroup sampling in two focused passes
+        CovergroupState state;
+        CovergroupCollectVisitor{nodep, state};  // Pass 1: collect CFuncs and events
+        CovergroupInjectVisitor{nodep, state};   // Pass 2: inject sample() calls
+        for (const auto& itpair : state.m_samplingEvents) itpair.second->deleteTree();
+    }
     V3Global::dumpCheckGlobalTree("active", 0, dumpTreeEitherLevel() >= 3);
 }