Bugfix - OpaqueSlot replace invalid localName (backport #2701) (#2702)

* Bugfix - OpaqueSlot replace invalid localName (#2701)

(cherry picked from commit fb8ea2a2fac227f2570da992d7877de2eb1cf801)

* Fix cloneTypes (#2703)

Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>

4 files changed, 60 insertions, 6 deletions

diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index 07fc80eb..9f79fe1e 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -233,7 +233,7 @@ private[chisel3] trait HasId extends InstanceId {
   }
   private[chisel3] def setRef(parent: HasId, name: String, opaque: Boolean = false): Unit = {
     if (!opaque) setRef(Slot(Node(parent), name))
-    else setRef(OpaqueSlot(Node(parent), name))
+    else setRef(OpaqueSlot(Node(parent)))
   }
 
   private[chisel3] def setRef(parent: HasId, index: Int):  Unit = setRef(Index(Node(parent), ILit(index)))
@@ -528,7 +528,7 @@ private[chisel3] object Builder extends LazyLogging {
     def buildAggName(id: HasId): Option[String] = {
       def getSubName(field: Data): Option[String] = field.getOptionRef.flatMap {
         case Slot(_, field)       => Some(field) // Record
-        case OpaqueSlot(_, field) => None // Record with single element
+        case OpaqueSlot(_)        => None // OpaqueSlots don't contribute to the name
         case Index(_, ILit(n))    => Some(n.toString) // Vec static indexing
         case Index(_, ULit(n, _)) => Some(n.toString) // Vec lit indexing
         case Index(_, _: Node) => None // Vec dynamic indexing
diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
index 56422b85..fe95445c 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
@@ -68,7 +68,7 @@ private[chisel3] object Converter {
       fir.Reference(name, fir.UnknownType)
     case Slot(imm, name) =>
       fir.SubField(convert(imm, ctx, info), name, fir.UnknownType)
-    case OpaqueSlot(imm, name) =>
+    case OpaqueSlot(imm) =>
       convert(imm, ctx, info)
     case Index(imm, ILit(idx)) =>
       fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType)
diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
index 37fb2f8b..d177c859 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala
@@ -91,7 +91,7 @@ object Arg {
     case Some(Index(Node(imm), Node(value))) => s"${earlyLocalName(imm)}[${earlyLocalName(imm)}]"
     case Some(Index(Node(imm), arg))         => s"${earlyLocalName(imm)}[${arg.localName}]"
     case Some(Slot(Node(imm), name))         => s"${earlyLocalName(imm)}.$name"
-    case Some(OpaqueSlot(Node(imm), name))   => s"${earlyLocalName(imm)}"
+    case Some(OpaqueSlot(Node(imm)))         => s"${earlyLocalName(imm)}"
     case Some(arg)                           => arg.name
     case None =>
       id match {
@@ -219,9 +219,9 @@ case class Slot(imm: Node, name: String) extends Arg {
   }
 }
 
-case class OpaqueSlot(imm: Node, name: String) extends Arg {
+case class OpaqueSlot(imm: Node) extends Arg {
   override def contextualName(ctx: Component): String = imm.name
-  override def localName: String = imm.name
+  override def name: String = imm.name
 }
 
 case class Index(imm: Arg, value: Arg) extends Arg {
diff --git a/src/test/scala/chiselTests/RecordSpec.scala b/src/test/scala/chiselTests/RecordSpec.scala
index 718c1acb..5080f15f 100644
--- a/src/test/scala/chiselTests/RecordSpec.scala
+++ b/src/test/scala/chiselTests/RecordSpec.scala
@@ -132,6 +132,35 @@ trait RecordSpecUtils {
     out := in1 + in2
   }
 
+  class InnerRecord extends Record {
+    val k = new InnerInnerRecord
+    val elements = SeqMap("" -> k)
+    override def opaqueType = elements.size == 1
+    override def cloneType: this.type = (new InnerRecord).asInstanceOf[this.type]
+  }
+
+  class InnerInnerRecord extends Record {
+    val k = new SingleElementRecord
+    val elements = SeqMap("" -> k)
+    override def opaqueType = elements.size == 1
+    override def cloneType: this.type = (new InnerInnerRecord).asInstanceOf[this.type]
+  }
+
+  class NestedRecordModule extends Module {
+    val in = IO(Input(new InnerRecord))
+    val out = IO(Output(new InnerRecord))
+    val inst = Module(new InnerModule)
+    inst.foo := in
+    out := inst.bar
+  }
+  class InnerModule extends Module {
+    val foo = IO(Input(new InnerRecord))
+    val bar = IO(Output(new InnerRecord))
+
+    // DO NOT do this; just for testing element connections
+    bar.elements.head._2 := foo.elements.head._2
+  }
+
   class NamedSingleElementRecord extends Record {
     private val underlying = UInt(8.W)
     val elements = SeqMap("unused" -> underlying)
@@ -205,6 +234,31 @@ class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils {
     singleElementChirrtl should include("add(in1, in2)")
   }
 
+  they should "work correctly for toTarget in nested opaque type Records" in {
+    var mod: NestedRecordModule = null
+    ChiselStage.elaborate { mod = new NestedRecordModule; mod }
+    val testStrings = Seq(
+      mod.in.toTarget.toString(),
+      mod.in.k.toTarget.toString(),
+      mod.in.k.k.toTarget.toString(),
+      mod.in.elements.head._2.toTarget.toString(),
+      mod.in.k.elements.head._2.toTarget.toString(),
+      mod.in.k.k.elements.head._2.toTarget.toString()
+    )
+    testStrings.foreach(x => assert(x == "~NestedRecordModule|NestedRecordModule>in"))
+  }
+
+  they should "work correctly when connecting nested opaque type elements" in {
+    val nestedRecordChirrtl = ChiselStage.emitChirrtl { new NestedRecordModule }
+    nestedRecordChirrtl should include("input in : UInt<8>")
+    nestedRecordChirrtl should include("output out : UInt<8>")
+    nestedRecordChirrtl should include("inst.foo <= in")
+    nestedRecordChirrtl should include("out <= inst.bar")
+    nestedRecordChirrtl should include("input foo : UInt<8>")
+    nestedRecordChirrtl should include("output bar : UInt<8>")
+    nestedRecordChirrtl should include("bar <= foo")
+  }
+
   they should "throw an error when map contains a named element and opaqueType is overriden to true" in {
     (the[Exception] thrownBy extractCause[Exception] {
       ChiselStage.elaborate { new NamedSingleElementModule }