Fix interface instance name collision (#7591) (#7593)

Fixes #7591.

Co-authored-by: Leela Pakanati <41307800+cachanova@users.noreply.github.com>

docs/CONTRIBUTORS

@@ -259,6 +259,7 @@ Srinivasan Venkataramanan
 Stefan Wallentowitz
 Stephen Henry
 Steven Hugg
+Stuart Morris
 Szymon Gizler
 Sören Tempel
 Teng Huang

src/V3Inline.cpp

@@ -256,6 +256,14 @@ class InlineRelinkVisitor final : public VNVisitor {
 
     // STATE
     std::unordered_set<std::string> m_renamedInterfaces;  // Name of renamed interface variables
+    std::unordered_set<std::string>
+        m_priorInlinedCells;  // Cells previously inlined into the module being inlined here.
+                              // Used to recognize VarXRefs whose inlinedDots was stamped by a
+                              // prior V3Inline pass (vs. by V3Begin generate-block unrolling).
+    std::unordered_set<const AstVarXRef*>
+        m_pinSubstitutedXRefs;  // VarXRefs created by pin substitution in this relink pass.
+                                // Their dotted/inlinedDots already represent the parent's scope
+                                // and must not be rewritten on the immediate visit.
     AstNodeModule* const m_modp;  // The module we are inlining into
     const AstCell* const m_cellp;  // The cell being inlined
     size_t m_nPlaceholders = 0;  // Unique identifier sequence number for placeholder variables
@@ -373,20 +381,47 @@ class InlineRelinkVisitor final : public VNVisitor {
         AstVarXRef* const newp
             = new AstVarXRef{nodep->fileline(), xrefp->name(), xrefp->dotted(), nodep->access()};
         newp->varp(xrefp->varp());
-        // Copy inlinedDots from pin expression - the normal visitor iteration will
-        // prepend the cell name when this VarXRef is visited later
+        // The pin expression came from m_modp (the parent we are inlining into), so its
+        // dotted/inlinedDots already describe a path in m_modp's scope. Record this xref
+        // so visit(AstVarXRef) leaves it alone on the immediate visit; later inline
+        // passes will prepend their cell names normally.
         newp->inlinedDots(xrefp->inlinedDots());
+        m_pinSubstitutedXRefs.insert(newp);
         nodep->replaceWith(newp);
         VL_DO_DANGLING(nodep->deleteTree(), nodep);
-        // Note: Don't call iterate(newp) here - the node will be visited during
-        // normal tree iteration which will apply the inlining transformations
     }
     void visit(AstVarXRef* nodep) override {
+        // VarXRefs just created by pin substitution in this pass already describe a path
+        // in m_modp's scope (the parent we are inlining into). Leave them untouched on
+        // this immediate visit; subsequent inline passes will prepend their cell names.
+        if (m_pinSubstitutedXRefs.erase(nodep)) {
+            iterateChildren(nodep);
+            return;
+        }
         // Track what scope it was originally under so V3LinkDot can resolve it
-        nodep->inlinedDots(VString::dot(m_cellp->name(), ".", nodep->inlinedDots()));
+        const string origInlinedDots = nodep->inlinedDots();
+        nodep->inlinedDots(VString::dot(m_cellp->name(), ".", origInlinedDots));
+        // If origInlinedDots starts with the name of a previously-inlined cell, this
+        // VarXRef came from that cell's body and its dotted refers to that child's
+        // local scope; renaming it against m_renamedInterfaces would wrongly alias it
+        // to a coincidentally-named var in the current module (#5120). VarXRefs whose
+        // inlinedDots was stamped by V3Begin generate-block unrolling are unaffected,
+        // since V3Begin's CellInlines have origModName "__BEGIN__" and don't appear in
+        // m_priorInlinedCells.
+        const string::size_type firstDot = origInlinedDots.find('.');
+        const string firstSeg
+            = firstDot == string::npos ? origInlinedDots : origInlinedDots.substr(0, firstDot);
+        const bool fromPriorInline = m_priorInlinedCells.count(firstSeg);
         for (string tryname = nodep->dotted(); true;) {
             if (m_renamedInterfaces.count(tryname)) {
-                nodep->dotted(m_cellp->name() + "__DOT__" + nodep->dotted());
+                // matchIsRenamed: the matched name itself was created by a prior V3Inline
+                // rename (contains "__DOT__"). When true, we are following the chain of
+                // renames for the same var across nested inlines, so apply the rename
+                // even if the VarXRef came from a prior-inlined child.
+                const bool matchIsRenamed = tryname.find("__DOT__") != string::npos;
+                if (!fromPriorInline || matchIsRenamed) {
+                    nodep->dotted(m_cellp->name() + "__DOT__" + nodep->dotted());
+                }
                 break;
             }
             // If foo.bar, and foo is an interface, then need to search again for foo
@@ -432,6 +467,16 @@ public:
     InlineRelinkVisitor(AstNodeModule* cloneModp, AstNodeModule* oldModp, AstCell* cellp)
         : m_modp{oldModp}
         , m_cellp{cellp} {
+        // CellInlines added by V3Begin for generate/named blocks have origModName
+        // "__BEGIN__"; only those added by prior V3Inline passes carry a real module
+        // name. Track the latter so visit(AstVarXRef) can distinguish VarXRefs
+        // originating from previously-inlined children.
+        for (AstNode* nodep = cloneModp->inlinesp(); nodep; nodep = nodep->nextp()) {
+            const AstCellInline* const cip = VN_CAST(nodep, CellInline);
+            if (cip && cip->origModName() != "__BEGIN__") {
+                m_priorInlinedCells.insert(cip->name());
+            }
+        }
         iterate(cloneModp);
     }
     ~InlineRelinkVisitor() override = default;

test_regress/t/t_iface_name_collision.py

@@ -0,0 +1,16 @@
+#!/usr/bin/env python3
+# DESCRIPTION: Verilator: Verilog Test driver/expect definition
+#
+# 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-FileCopyrightText: 2026 Wilson Snyder
+# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0
+
+import vltest_bootstrap
+
+test.scenarios('vlt')
+
+test.lint(verilator_flags2=["--lint-only -Werror-MULTIDRIVEN -Werror-UNOPTFLAT"])
+
+test.passes()

test_regress/t/t_iface_name_collision.v

@@ -0,0 +1,49 @@
+// DESCRIPTION: Verilator: Verilog Test module
+//
+// The code describes a very simple hierarchy of modules.
+// The lowest level instantiates avst_interface with name my_avst_if.
+// The highest level also instantiates the same interface type
+// with the exact same name. The file should lint with no errors
+// or warnings other than those disabled by lint_off.
+//
+// This file ONLY is placed under the Creative Commons Public Domain.
+// SPDX-FileCopyrightText: 2026 Wilson Snyder
+// SPDX-License-Identifier: CC0-1.0
+
+/* verilator lint_off DECLFILENAME */
+/* verilator lint_off UNUSEDSIGNAL */
+interface avst_interface;
+  logic ready;
+  logic valid;
+
+  modport sink_mp (output ready, input  valid);
+endinterface
+
+module child (
+  output logic ready_out
+);
+  avst_interface my_avst_if();
+
+  assign ready_out = my_avst_if.ready;
+  assign my_avst_if.ready = 1'b1; // drives child.my_avst_if.ready only
+endmodule
+
+module wrapper (
+  avst_interface.sink_mp my_avst_if
+);
+  child child_inst (
+    .ready_out (my_avst_if.ready) // sole driver of outer my_avst_if.ready
+  );
+endmodule
+
+module top (
+  input  logic in_valid,
+  output logic out_ready
+);
+  avst_interface my_avst_if();
+
+  assign my_avst_if.valid = in_valid;
+  assign out_ready = my_avst_if.ready;
+
+  wrapper wrapper_inst (.my_avst_if (my_avst_if));
+endmodule