From e2c05ae15edbf55f3933efc960c1220cc9f95642 Mon Sep 17 00:00:00 2001
From: Geza Lore <gezalore@gmail.com>
Date: Fri, 5 Dec 2025 15:07:53 +0000
Subject: [PATCH] Fix select assignment expansion (#6757)

---
 src/V3Broken.cpp    | 29 ++++++++++++++++++++++++++++-
 src/V3Expand.cpp    |  3 ++-
 src/V3Inline.cpp    |  1 +
 src/V3Inst.cpp      |  6 +++++-
 src/V3Randomize.cpp |  2 ++
 5 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp
index d7916185b..355ac9e29 100644
--- a/src/V3Broken.cpp
+++ b/src/V3Broken.cpp
@@ -158,6 +158,10 @@ class BrokenCheckVisitor final : public VNVisitorConst {
     std::map<const AstVar*, const AstNodeVarRef*> m_suspectRefs;
     // Local variables declared in the scope of the current statement
     std::vector<std::unordered_set<const AstVar*>> m_localsStack;
+    // Number of write references encountered
+    size_t m_nWriteRefs = 0;
+    // Number of function calls encountered
+    size_t m_nCalls = 0;
 
     // STATE - for current visit position (use VL_RESTORER)
     const AstCFunc* m_cfuncp = nullptr;  // Current CFunc, if any
@@ -225,7 +229,17 @@ private:
     }
     // VISITORS
     void visit(AstNodeAssign* nodep) override {
-        processAndIterate(nodep);
+        processEnter(nodep);
+        iterateConst(nodep->rhsp());
+        const size_t nWriteRefs = m_nWriteRefs;
+        const size_t nCalls = m_nCalls;
+        iterateConst(nodep->lhsp());
+        // TODO: Enable this when #6756 is fixed
+        // Only check if there are no calls on the LHS, as calls might return an LValue
+        if (false && v3Global.assertDTypesResolved() && m_nCalls == nCalls) {
+            UASSERT_OBJ(m_nWriteRefs > nWriteRefs, nodep, "No write refs on LHS of assignment");
+        }
+        processExit(nodep);
         UASSERT_OBJ(!(v3Global.assertDTypesResolved() && VN_IS(nodep->lhsp(), NodeVarRef)
                       && !VN_AS(nodep->lhsp(), NodeVarRef)->access().isWriteOrRW()),
                     nodep, "Assignment LHS is not an lvalue");
@@ -270,6 +284,19 @@ private:
                 }
             }
         }
+        if (nodep->access().isWriteOrRW()) ++m_nWriteRefs;
+    }
+    void visit(AstNodeCCall* nodep) override {
+        ++m_nCalls;
+        processAndIterate(nodep);
+    }
+    void visit(AstCMethodHard* nodep) override {
+        ++m_nCalls;
+        processAndIterate(nodep);
+    }
+    void visit(AstNodeFTaskRef* nodep) override {
+        ++m_nCalls;
+        processAndIterate(nodep);
     }
     void visit(AstCFunc* nodep) override {
         UASSERT_OBJ(!m_cfuncp, nodep, "Nested AstCFunc");
diff --git a/src/V3Expand.cpp b/src/V3Expand.cpp
index 43d3b37b7..08227a3db 100644
--- a/src/V3Expand.cpp
+++ b/src/V3Expand.cpp
@@ -728,7 +728,8 @@ class ExpandVisitor final : public VNVisitor {
                 UINFO(8, "    ASSIGNSEL(varlsb,wide,1bit) " << nodep);
                 AstNodeExpr* const rhsp = nodep->rhsp()->unlinkFrBack();
                 AstNodeExpr* const destp = lhsp->fromp()->unlinkFrBack();
-                AstNodeExpr* oldvalp = newWordSelBit(lfl, destp, lhsp->lsbp());
+                AstNodeExpr* oldvalp
+                    = newWordSelBit(lfl, destp->cloneTreePure(false), lhsp->lsbp());
                 fixCloneLvalue(oldvalp);
                 if (!ones) {
                     oldvalp = new AstAnd{
diff --git a/src/V3Inline.cpp b/src/V3Inline.cpp
index eab6919ef..9473164b7 100644
--- a/src/V3Inline.cpp
+++ b/src/V3Inline.cpp
@@ -566,6 +566,7 @@ void inlineCell(AstNodeModule* modp, AstCell* cellp, bool last) {
 
         // Warn
         V3Inst::checkOutputShort(pinp);
+        if (!pinp->exprp()) continue;
 
         // Pick up the old and new port variables signal (new is the same on last instance)
         const AstVar* const oldModVarp = pinp->modVarp();
diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp
index c57dd1f84..5b754d13f 100644
--- a/src/V3Inst.cpp
+++ b/src/V3Inst.cpp
@@ -58,9 +58,10 @@ class InstVisitor final : public VNVisitor {
             // Simplify it
             V3Inst::pinReconnectSimple(nodep, m_cellp, false);
         }
-        if (!nodep->exprp()) return;  // No-connect
         UINFOTREE(9, nodep, "", "Pin_oldb");
+        if (!nodep->exprp()) return;  // No-connect
         V3Inst::checkOutputShort(nodep);
+        if (!nodep->exprp()) return;  // Connection removed by checkOutputShort
         // Use user1p on the PIN to indicate we created an assign for this pin
         if (!nodep->user1SetOnce()) {
             // Make an ASSIGNW (expr, pin)
@@ -644,6 +645,7 @@ public:
             // Make a new temp wire
             // UINFOTREE(9, pinp, "", "in_pin");
             V3Inst::checkOutputShort(pinp);
+            if (!pinp->exprp()) return nullptr;
             // Simplify, so stuff like '{a[0], b[0]}[1]' produced during
             // instance array expansion are brought to normal 'a[0]'
             AstNodeExpr* const pinexprp
@@ -700,6 +702,8 @@ void V3Inst::checkOutputShort(const AstPin* nodep) {
             // Uses v3warn for error, as might be found multiple times
             nodep->v3warn(E_PORTSHORT, "Output port is connected to a constant pin,"
                                        " electrical short");
+            // Delete so we don't create a 'CONST = ...' assignment
+            nodep->exprp()->unlinkFrBack()->deleteTree();
         }
     }
 }
diff --git a/src/V3Randomize.cpp b/src/V3Randomize.cpp
index eeb270378..1ba038bae 100644
--- a/src/V3Randomize.cpp
+++ b/src/V3Randomize.cpp
@@ -2174,6 +2174,8 @@ class RandomizeVisitor final : public VNVisitor {
     AstNodeExpr* makeSiblingRefp(AstNodeExpr* const exprp, AstVar* const varp,
                                  const VAccess access) {
         if (AstMemberSel* const memberSelp = VN_CAST(exprp, MemberSel)) {
+            // TODO: this ignored 'access' and will create a read reference in
+            // t_randomize_inline_var_ctl, see issue #6756
             return new AstMemberSel{exprp->fileline(), memberSelp->fromp()->cloneTree(false),
                                     varp};
         }