Switch to using experimental trait for OpaqueTypes (backport #2783) (#2836)

* Switch to using experimental trait for OpaqueTypes (#2783)

This makes it more clear that the feature is experimental. Users may
still override the opaqueType method for more dynamic control over when
instances of a given Record are OpaqueTypes or not, but they are
discouraged from doing so.

(cherry picked from commit 7525dc71ccc2050d8e4a68b38f3b76920ba693fc)

* Fix cloneType in RecordSpec

Co-authored-by: Jack Koenig <koenig@sifive.com>

2 files changed, 72 insertions, 17 deletions

diff --git a/src/test/scala/chiselTests/Direction.scala b/src/test/scala/chiselTests/Direction.scala
index 03755e83..642a507c 100644
--- a/src/test/scala/chiselTests/Direction.scala
+++ b/src/test/scala/chiselTests/Direction.scala
@@ -4,6 +4,7 @@ package chiselTests
 
 import org.scalatest._
 import chisel3._
+import chisel3.experimental.OpaqueType
 import chisel3.stage.ChiselStage
 import org.scalatest.matchers.should.Matchers
 
@@ -370,10 +371,9 @@ class DirectionSpec extends ChiselPropSpec with Matchers with Utils {
       val valid = Bool()
       val ready = Flipped(Bool())
     }
-    class MyOpaqueType extends Record {
+    class MyOpaqueType extends Record with OpaqueType {
       val k = new Decoupled()
       val elements = SeqMap("" -> k)
-      override def opaqueType = elements.size == 1
       override def cloneType: this.type = (new MyOpaqueType).asInstanceOf[this.type]
     }
     class MyModule extends RawModule {
diff --git a/src/test/scala/chiselTests/RecordSpec.scala b/src/test/scala/chiselTests/RecordSpec.scala
index 509edbbc..3414ec8a 100644
--- a/src/test/scala/chiselTests/RecordSpec.scala
+++ b/src/test/scala/chiselTests/RecordSpec.scala
@@ -6,7 +6,7 @@ import chisel3._
 import chisel3.stage.ChiselStage
 import chisel3.testers.BasicTester
 import chisel3.util.{Counter, Queue}
-import chisel3.experimental.DataMirror
+import chisel3.experimental.{DataMirror, OpaqueType}
 
 import scala.collection.immutable.SeqMap
 
@@ -109,10 +109,9 @@ trait RecordSpecUtils {
     require(!DataMirror.checkTypeEquivalence(wire1, wire2))
   }
 
-  class SingleElementRecord extends Record {
+  class SingleElementRecord extends Record with OpaqueType {
     private val underlying = UInt(8.W)
     val elements = SeqMap("" -> underlying)
-    override def opaqueType = elements.size == 1
     override def cloneType: this.type = (new SingleElementRecord).asInstanceOf[this.type]
 
     def +(that: SingleElementRecord): SingleElementRecord = {
@@ -132,17 +131,15 @@ trait RecordSpecUtils {
     out := in1 + in2
   }
 
-  class InnerRecord extends Record {
+  class InnerRecord extends Record with OpaqueType {
     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 {
+  class InnerInnerRecord extends Record with OpaqueType {
     val k = new SingleElementRecord
     val elements = SeqMap("" -> k)
-    override def opaqueType = elements.size == 1
     override def cloneType: this.type = (new InnerInnerRecord).asInstanceOf[this.type]
   }
 
@@ -163,11 +160,10 @@ trait RecordSpecUtils {
     io.bar.elements.head._2 := io.foo.elements.head._2
   }
 
-  class NamedSingleElementRecord extends Record {
+  class NamedSingleElementRecord extends Record with OpaqueType {
     private val underlying = UInt(8.W)
     val elements = SeqMap("unused" -> underlying)
 
-    override def opaqueType = elements.size == 1
     override def cloneType: this.type = (new NamedSingleElementRecord).asInstanceOf[this.type]
   }
 
@@ -177,7 +173,7 @@ trait RecordSpecUtils {
     out := in
   }
 
-  class ErroneousOverride extends Record {
+  class ErroneousOverride extends Record with OpaqueType {
     private val underlyingA = UInt(8.W)
     private val underlyingB = UInt(8.W)
     val elements = SeqMap("x" -> underlyingA, "y" -> underlyingB)
@@ -191,6 +187,44 @@ trait RecordSpecUtils {
     val out = IO(Output(new ErroneousOverride))
     out := in
   }
+
+  class NotActuallyOpaqueType extends Record with OpaqueType {
+    private val underlyingA = UInt(8.W)
+    private val underlyingB = UInt(8.W)
+    val elements = SeqMap("x" -> underlyingA, "y" -> underlyingB)
+
+    override def opaqueType = false
+    override def cloneType: this.type = (new NotActuallyOpaqueType).asInstanceOf[this.type]
+  }
+
+  class NotActuallyOpaqueTypeModule extends Module {
+    val in = IO(Input(new NotActuallyOpaqueType))
+    val out = IO(Output(new NotActuallyOpaqueType))
+    out := in
+  }
+
+  // Illustrate how to dyanmically decide between OpaqueType or not
+  sealed trait MaybeBoxed[T <: Data] extends Record {
+    def underlying: T
+    def boxed:      Boolean
+  }
+  object MaybeBoxed {
+    def apply[T <: Data](gen: T, boxed: Boolean): MaybeBoxed[T] = {
+      if (boxed) new Boxed(gen) else new Unboxed(gen)
+    }
+  }
+  class Boxed[T <: Data](gen: T) extends MaybeBoxed[T] {
+    def boxed = true
+    lazy val elements = SeqMap("underlying" -> gen.cloneType)
+    def underlying = elements.head._2
+    override def cloneType: this.type = (new Boxed(gen)).asInstanceOf[this.type]
+  }
+  class Unboxed[T <: Data](gen: T) extends MaybeBoxed[T] with OpaqueType {
+    def boxed = false
+    lazy val elements = SeqMap("" -> gen.cloneType)
+    def underlying = elements.head._2
+    override def cloneType: this.type = (new Unboxed(gen)).asInstanceOf[this.type]
+  }
 }
 
 class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils {
@@ -229,14 +263,14 @@ class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils {
     e.getMessage should include("contains aliased fields named (bar,foo)")
   }
 
-  they should "be OpaqueType for maps with single unnamed elements" in {
+  they should "support OpaqueType for maps with single unnamed elements" in {
     val singleElementChirrtl = ChiselStage.emitChirrtl { new SingleElementRecordModule }
     singleElementChirrtl should include("input in1 : UInt<8>")
     singleElementChirrtl should include("input in2 : UInt<8>")
     singleElementChirrtl should include("add(in1, in2)")
   }
 
-  they should "work correctly for toTarget in nested opaque type Records" in {
+  they should "work correctly for toTarget in nested OpaqueType Records" in {
     var mod: NestedRecordModule = null
     ChiselStage.elaborate { mod = new NestedRecordModule; mod }
     val testStrings = Seq(
@@ -250,7 +284,7 @@ class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils {
     testStrings.foreach(x => assert(x == "~NestedRecordModule|InnerModule>io.foo"))
   }
 
-  they should "work correctly when connecting nested opaque type elements" in {
+  they should "work correctly when connecting nested OpaqueType elements" in {
     val nestedRecordChirrtl = ChiselStage.emitChirrtl { new NestedRecordModule }
     nestedRecordChirrtl should include("input in : UInt<8>")
     nestedRecordChirrtl should include("output out : UInt<8>")
@@ -260,18 +294,39 @@ class RecordSpec extends ChiselFlatSpec with RecordSpecUtils with Utils {
     nestedRecordChirrtl should include("io.bar <= io.foo")
   }
 
-  they should "throw an error when map contains a named element and opaqueType is overriden to true" in {
+  they should "throw an error when map contains a named element and OpaqueType is mixed in" in {
     (the[Exception] thrownBy extractCause[Exception] {
       ChiselStage.elaborate { new NamedSingleElementModule }
     }).getMessage should include("Opaque types must have exactly one element with an empty name")
   }
 
-  they should "throw an error when map contains more than one element and opaqueType is overriden to true" in {
+  they should "throw an error when map contains more than one element and OpaqueType is mixed in" in {
     (the[Exception] thrownBy extractCause[Exception] {
       ChiselStage.elaborate { new ErroneousOverrideModule }
     }).getMessage should include("Opaque types must have exactly one element with an empty name")
   }
 
+  they should "work correctly when an OpaqueType overrides the def as false" in {
+    val chirrtl = ChiselStage.emitChirrtl(new NotActuallyOpaqueTypeModule)
+    chirrtl should include("input in : { y : UInt<8>, x : UInt<8>}")
+    chirrtl should include("output out : { y : UInt<8>, x : UInt<8>}")
+    chirrtl should include("out <= in")
+  }
+
+  they should "support conditional OpaqueTypes via traits and factory methods" in {
+    class MyModule extends Module {
+      val in0 = IO(Input(MaybeBoxed(UInt(8.W), true)))
+      val out0 = IO(Output(MaybeBoxed(UInt(8.W), true)))
+      val in1 = IO(Input(MaybeBoxed(UInt(8.W), false)))
+      val out1 = IO(Output(MaybeBoxed(UInt(8.W), false)))
+      out0 := in0
+      out1 := in1
+    }
+    val chirrtl = ChiselStage.emitChirrtl(new MyModule)
+    chirrtl should include("input in0 : { underlying : UInt<8>}")
+    chirrtl should include("input in1 : UInt<8>")
+  }
+
   they should "work with .toTarget" in {
     var m: SingleElementRecordModule = null
     ChiselStage.elaborate { m = new SingleElementRecordModule; m }