Fix array of struct member overwrites on member update (#5605) (#5618) (#5628)

src/V3Unknown.cpp

@@ -99,7 +99,7 @@ class UnknownVisitor final : public VNVisitor {
         AstNodeExpr* prep = nodep;
 
         // Scan back to put the condlvalue above all selects (IE top of the lvalue)
-        while (VN_IS(prep->backp(), NodeSel) || VN_IS(prep->backp(), Sel)) {
+        while (VN_IS(prep->backp(), NodeSel) || VN_IS(prep->backp(), Sel) || VN_IS(prep->backp(), StructSel)) {
             prep = VN_AS(prep->backp(), NodeExpr);
         }
         FileLine* const fl = nodep->fileline();
@@ -119,28 +119,7 @@ class UnknownVisitor final : public VNVisitor {
                 = new AstVar{fl, VVarType::MODULETEMP, m_lvboundNames.get(prep), prep->dtypep()};
             m_modp->addStmtsp(varp);
             AstNode* const abovep = prep->backp();  // Grab above point before we replace 'prep'
-            AstNode* currentStmtp = abovep;
+
-            while (currentStmtp && !VN_IS(currentStmtp, NodeStmt))
-                currentStmtp = currentStmtp->backp();
-            VNRelinker linkContext;
-            currentStmtp = currentStmtp->unlinkFrBackWithNext(&linkContext);
-            AstNodeExpr* const selExprp = prep->cloneTree(true);
-            AstNodeExpr* currentExprp = selExprp;
-            while (AstNodeExpr* itrSelExprp = VN_AS(currentExprp->op1p(), NodeExpr)) {
-                if (AstNodeVarRef* const selRefp = VN_CAST(itrSelExprp, NodeVarRef)) {
-                    // Mark the variable reference as READ access to avoid assignment issues
-                    selRefp->access(VAccess::READ);
-                    break;
-                }
-                currentExprp = itrSelExprp;
-            }
-            // Before assigning the value to the temporary variable, first assign the current array
-            // element to it. This ensures any field modifications happen on the correct instance
-            // and prevents overwriting other fields.
-            AstNode* const newAssignp
-                = new AstAssign{fl, new AstVarRef{fl, varp, VAccess::WRITE}, selExprp};
-            newAssignp->addNextStmt(currentStmtp, newAssignp);
-            linkContext.relink(newAssignp);
             prep->replaceWith(new AstVarRef{fl, varp, VAccess::WRITE});
             if (m_timingControlp) m_timingControlp->unlinkFrBack();
             AstIf* const newp = new AstIf{

test_regress/t/t_struct_array_assignment_delayed.py

@@ -0,0 +1,18 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2024 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')
+
+test.compile(timing_loop=True, verilator_flags2=["--timing"])
+
+test.execute()
+
+test.passes()

test_regress/t/t_struct_array_assignment_delayed.v

@@ -0,0 +1,35 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2024 by sumpster.
+// SPDX-License-Identifier: CC0-1.0
+
+module tb;
+    typedef struct {
+        logic a;
+        logic b;
+    } SimpleStruct;
+
+    SimpleStruct s [1];
+
+    logic clock;
+
+    always @(posedge clock) begin
+        for (int i = 0; i < 1; i++) begin
+            s[i].a <= 1;
+            s[i].b <= 0;
+        end
+    end
+
+    initial begin
+        clock = 0;
+        s[0].a = 0;
+        s[0].b = 0;
+
+        #1 clock = 1;
+        #1 if (s[0].a != 1) $stop;
+
+        $write("*-* All Finished *-*\n");
+        $finish;
+    end
+endmodule