From cc982ec7fe889fc1e5d2bb71cce3239fca6e1a03 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Krzysztof=20Boro=C5=84ski?=
 <94375110+kboronski-ant@users.noreply.github.com>
Date: Wed, 15 Nov 2023 14:24:41 +0100
Subject: [PATCH] Fix range access to fields under classes that depend on
 parameter resolution (#4681)

Signed-off-by: Krzysztof Boronski <kboronski@antmicro.com>
---
 src/V3AstNodes.cpp                            |  2 +-
 src/V3LinkDot.cpp                             | 16 ++++++-
 src/V3LinkResolve.cpp                         | 13 ------
 .../t/t_selextract_in_paramextends.pl         | 22 ++++++++++
 test_regress/t/t_selextract_in_paramextends.v | 44 +++++++++++++++++++
 5 files changed, 82 insertions(+), 15 deletions(-)
 create mode 100755 test_regress/t/t_selextract_in_paramextends.pl
 create mode 100644 test_regress/t/t_selextract_in_paramextends.v

diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp
index 93c7892dd..5b717f513 100644
--- a/src/V3AstNodes.cpp
+++ b/src/V3AstNodes.cpp
@@ -931,7 +931,7 @@ AstNode* AstArraySel::baseFromp(AstNode* nodep, bool overMembers) {
             nodep = VN_AS(nodep, MemberSel)->fromp();
             continue;
         }
-        // AstNodeSelPre stashes the associated variable under an ATTROF
+        // AstNodePreSel stashes the associated variable under an ATTROF
         // of VAttrType::VAR_BASE so it isn't constified
         else if (VN_IS(nodep, AttrOf)) {
             nodep = VN_AS(nodep, AttrOf)->fromp();
diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp
index 9edecc5f7..066d144bf 100644
--- a/src/V3LinkDot.cpp
+++ b/src/V3LinkDot.cpp
@@ -3343,7 +3343,19 @@ class LinkDotResolveVisitor final : public VNVisitor {
             m_ds.init(m_curSymp);
             iterateAndNextNull(nodep->rhsp());
             iterateAndNextNull(nodep->thsp());
-            iterateAndNextNull(nodep->attrp());
+        }
+
+        if (nodep->attrp()) {
+            AstNode* const attrp = nodep->attrp()->unlinkFrBack();
+            VL_DO_DANGLING(attrp->deleteTree(), attrp);
+        }
+        AstNode* const basefromp = AstArraySel::baseFromp(nodep, false);
+        if (VN_IS(basefromp, Replicate)) {
+            // From {...}[...] syntax in IEEE 2017
+            if (basefromp) UINFO(1, "    Related node: " << basefromp << endl);
+        } else {
+            nodep->attrp(new AstAttrOf{nodep->fileline(), VAttrType::VAR_BASE,
+                                       basefromp->cloneTree(false)});
         }
     }
     void visit(AstMemberSel* nodep) override {
@@ -3742,6 +3754,8 @@ class LinkDotResolveVisitor final : public VNVisitor {
         iterateChildren(nodep);
     }
 
+    void visit(AstAttrOf* nodep) override { iterateChildren(nodep); }
+
     void visit(AstNode* nodep) override {
         checkNoDot(nodep);
         iterateChildren(nodep);
diff --git a/src/V3LinkResolve.cpp b/src/V3LinkResolve.cpp
index 530b6a0ef..f4f8fb653 100644
--- a/src/V3LinkResolve.cpp
+++ b/src/V3LinkResolve.cpp
@@ -181,19 +181,6 @@ class LinkResolveVisitor final : public VNVisitor {
             nodep->scopeNamep(new AstScopeName{nodep->fileline(), false});
         }
     }
-    void visit(AstNodePreSel* nodep) override {
-        if (!nodep->attrp()) {
-            iterateChildren(nodep);
-            AstNode* const basefromp = AstArraySel::baseFromp(nodep, false);
-            if (VN_IS(basefromp, Replicate)) {
-                // From {...}[...] syntax in IEEE 2017
-                if (basefromp) UINFO(1, "    Related node: " << basefromp << endl);
-            } else {
-                nodep->attrp(new AstAttrOf{nodep->fileline(), VAttrType::VAR_BASE,
-                                           basefromp->cloneTree(false)});
-            }
-        }
-    }
 
     void visit(AstCaseItem* nodep) override {
         // Move default caseItems to the bottom of the list
diff --git a/test_regress/t/t_selextract_in_paramextends.pl b/test_regress/t/t_selextract_in_paramextends.pl
new file mode 100755
index 000000000..b267631e9
--- /dev/null
+++ b/test_regress/t/t_selextract_in_paramextends.pl
@@ -0,0 +1,22 @@
+#!/usr/bin/env perl
+if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; }
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# Copyright 2023 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
+
+scenarios(simulator => 1);
+
+compile(
+    verilator_flags2 => ["--timing"],
+    );
+
+execute(
+    check_finished => 1,
+    );
+
+ok(1);
+1;
diff --git a/test_regress/t/t_selextract_in_paramextends.v b/test_regress/t/t_selextract_in_paramextends.v
new file mode 100644
index 000000000..f371482d6
--- /dev/null
+++ b/test_regress/t/t_selextract_in_paramextends.v
@@ -0,0 +1,44 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// This file ONLY is placed under the Creative Commons Public Domain, for
+// any use, without warranty, 2023 by Antmicro Ltd.
+// SPDX-License-Identifier: CC0-1.0
+
+typedef class Foo;
+
+virtual class Bar #(type T);
+  T m_val;
+endclass
+
+class Baz;
+  rand bit [3:0] m_sus;
+endclass
+
+class Foo extends Bar#(Baz);
+  function new();
+    Baz baz;
+    super.new();
+    baz = new;
+    super.m_val = baz;
+  endfunction
+
+  task update_value(Foo foo, bit [1:0] val);
+    m_val.m_sus[1:0] = val;
+  endtask
+endclass
+
+module test();
+  initial begin
+    Foo foo = new;
+
+    for (int i = 0; i < 10; i++) begin
+      logic [3:0] v;
+      foo.update_value(foo, i[1:0]);
+      v = foo.m_val.m_sus;
+      if (v[1:0] != i[1:0]) $stop;
+    end
+
+    $write("*-* All Finished *-*\n");
+    $finish;
+  end
+endmodule