the cloneType and chiselCloneType hot mess 🔥 (#653)

Addresses #419

cloneType is now marked (through comments only) as an internal API.
chiselCloneType deprecated (and changed to cloneTypeFull internally, analogous to cloneTypeWidth).
chiselTypeOf(data) introduced as the external API to get a chisel type from a hardware object
Intended usage: cloning is an implementation detail, and chisel types and hardware objects both should act as immutable types, with operations like Input(...), Reg(...), etc returning a copy and leaving the original unchanged. Hence, the clone operations are all deprecated.
Deletes what appears to be an unused Bundle companion object.
Input(...), Output(...), Flipped(...) require the object to be unbound

4 files changed, 56 insertions, 38 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
index 8700b444..47f4b3e7 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
@@ -93,7 +93,7 @@ trait VecFactory {
     if (compileOptions.declaredTypeMustBeUnbound) {
       requireIsChiselType(gen, "vec type")
     }
-    new Vec(gen.chiselCloneType, n)
+    new Vec(gen.cloneTypeFull, n)
   }
 
   /** Truncate an index to implement modulo-power-of-2 addressing. */
@@ -580,9 +580,3 @@ class Bundle(implicit compileOptions: CompileOptions) extends Record {
     */
   override def toPrintable: Printable = toPrintableHelper(elements.toList.reverse)
 }
-
-private[core] object Bundle {
-  val keywords = List("flip", "asInput", "asOutput", "cloneType", "chiselCloneType", "toBits",
-    "widthOption", "signalName", "signalPathName", "signalParent", "signalComponent")
-}
-
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Data.scala b/chiselFrontend/src/main/scala/chisel3/core/Data.scala
index b5c9d319..aa286e0d 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Data.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Data.scala
@@ -87,6 +87,10 @@ object DataMirror {
   // Internal reflection-style APIs, subject to change and removal whenever.
   object internal {
     def isSynthesizable(target: Data) = target.hasBinding
+    // For those odd cases where you need to care about object reference and uniqueness
+    def chiselTypeClone[T<:Data](target: Data): T = {
+      target.cloneTypeFull.asInstanceOf[T]
+    }
   }
 }
 
@@ -126,7 +130,7 @@ private[core] object cloneSupertype {
           throw new AssertionError(
             s"can't create $createdType with heterogeneous Bits types ${elt1.getClass} and ${elt2.getClass}")
       }).asInstanceOf[T] }
-      model.chiselCloneType
+      model.cloneTypeFull
     }
     else {
       for (elt <- elts.tail) {
@@ -135,11 +139,20 @@ private[core] object cloneSupertype {
         require(elt typeEquivalent elts.head,
           s"can't create $createdType with non-equivalent types ${elts.head} and ${elt}")
       }
-      elts.head.chiselCloneType
+      elts.head.cloneTypeFull
     }
   }
 }
 
+/** Returns the chisel type of a hardware object, allowing other hardware to be constructed from it.
+  */
+object chiselTypeOf {
+  def apply[T <: Data](target: T): T = {
+    requireIsHardware(target)
+    target.cloneTypeFull.asInstanceOf[T]
+  }
+}
+
 /**
 * Input, Output, and Flipped are used to define the directions of Module IOs.
 *
@@ -148,22 +161,31 @@ private[core] object cloneSupertype {
 * Thus, an error will be thrown if these are used on bound Data
 */
 object Input {
-  def apply[T<:Data](source: T): T = {
-    val out = source.cloneType
+  def apply[T<:Data](source: T)(implicit compileOptions: CompileOptions): T = {
+    if (compileOptions.checkSynthesizable) {
+      requireIsChiselType(source)
+    }
+    val out = source.cloneType.asInstanceOf[T]
     out.specifiedDirection = SpecifiedDirection.Input
     out
   }
 }
 object Output {
-  def apply[T<:Data](source: T): T = {
-    val out = source.cloneType
+  def apply[T<:Data](source: T)(implicit compileOptions: CompileOptions): T = {
+    if (compileOptions.checkSynthesizable) {
+      requireIsChiselType(source)
+    }
+    val out = source.cloneType.asInstanceOf[T]
     out.specifiedDirection = SpecifiedDirection.Output
     out
   }
 }
 object Flipped {
-  def apply[T<:Data](source: T): T = {
-    val out = source.cloneType
+  def apply[T<:Data](source: T)(implicit compileOptions: CompileOptions): T = {
+    if (compileOptions.checkSynthesizable) {
+      requireIsChiselType(source)
+    }
+    val out = source.cloneType.asInstanceOf[T]
     out.specifiedDirection = SpecifiedDirection.flip(source.specifiedDirection)
     out
   }
@@ -314,25 +336,27 @@ abstract class Data extends HasId {
   private[chisel3] def width: Width
   private[core] def legacyConnect(that: Data)(implicit sourceInfo: SourceInfo): Unit
 
-  /** cloneType must be defined for any Chisel object extending Data.
+  /** Internal API; Chisel users should look at chisel3.chiselTypeOf(...).
+    *
+    * cloneType must be defined for any Chisel object extending Data.
     * It is responsible for constructing a basic copy of the object being cloned.
-    * If cloneType needs to recursively clone elements of an object, it should call
-    * the cloneType methods on those elements.
+    *
     * @return a copy of the object.
     */
   def cloneType: this.type
 
-  /** chiselCloneType is called at the top-level of a clone chain.
-    * It calls the client's cloneType() method to construct a basic copy of the object being cloned,
-    * then performs any fixups required to reconstruct the appropriate core state of the cloned object.
-    * @return a copy of the object with appropriate core state.
+  /** Internal API; Chisel users should look at chisel3.chiselTypeOf(...).
+    *
+    * Returns a copy of this data type, with hardware bindings (if any) removed.
+    * Directionality data is still preserved.
     */
-  def chiselCloneType: this.type = {
-    val clone = this.cloneType  // get a fresh object, without bindings
+  private[chisel3] def cloneTypeFull: this.type = {
+    val clone = this.cloneType.asInstanceOf[this.type]  // get a fresh object, without bindings
     // Only the top-level direction needs to be fixed up, cloneType should do the rest
     clone.specifiedDirection = specifiedDirection
     clone
   }
+
   final def := (that: Data)(implicit sourceInfo: SourceInfo, connectionCompileOptions: CompileOptions): Unit = this.connect(that)(sourceInfo, connectionCompileOptions)
   final def <> (that: Data)(implicit sourceInfo: SourceInfo, connectionCompileOptions: CompileOptions): Unit = this.bulkConnect(that)(sourceInfo, connectionCompileOptions)
   def litArg(): Option[LitArg] = None
@@ -357,7 +381,7 @@ abstract class Data extends HasId {
   /** Does a reinterpret cast of the bits in this node into the format that provides.
     * Returns a new Wire of that type. Does not modify existing nodes.
     *
-    * x.asTypeOf(that) performs the inverse operation of x = that.toBits.
+    * x.asTypeOf(that) performs the inverse operation of x := that.toBits.
     *
     * @note bit widths are NOT checked, may pad or drop bits from input
     * @note that should have known widths
@@ -365,7 +389,7 @@ abstract class Data extends HasId {
   def asTypeOf[T <: Data](that: T): T = macro SourceInfoTransform.thatArg
 
   def do_asTypeOf[T <: Data](that: T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
-    val thatCloned = Wire(that.chiselCloneType)
+    val thatCloned = Wire(that.cloneTypeFull)
     thatCloned.connectFromBits(this.asUInt())
     thatCloned
   }
@@ -395,7 +419,7 @@ trait WireFactory {
     if (compileOptions.declaredTypeMustBeUnbound) {
       requireIsChiselType(t, "wire type")
     }
-    val x = t.chiselCloneType
+    val x = t.cloneTypeFull
 
     // Bind each element of x to being a Wire
     x.bind(WireBinding(Builder.forcedUserModule))
@@ -413,10 +437,10 @@ object WireInit {
   def apply[T <: Data](init: T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
     val model = (init.litArg match {
       // For e.g. Wire(init=0.U(k.W)), fix the Reg's width to k
-      case Some(lit) if lit.forcedWidth => init.chiselCloneType
+      case Some(lit) if lit.forcedWidth => init.cloneTypeFull
       case _ => init match {
         case init: Bits => init.cloneTypeWidth(Width())
-        case init => init.chiselCloneType
+        case init => init.cloneTypeFull
       }
     }).asInstanceOf[T]
     apply(model, init)
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Mem.scala b/chiselFrontend/src/main/scala/chisel3/core/Mem.scala
index 1e7a795a..72d91c3c 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Mem.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Mem.scala
@@ -23,7 +23,7 @@ object Mem {
     if (compileOptions.declaredTypeMustBeUnbound) {
       requireIsChiselType(t, "memory type")
     }
-    val mt  = t.chiselCloneType
+    val mt  = t.cloneTypeFull
     val mem = new Mem(mt, size)
     pushCommand(DefMemory(sourceInfo, mem, mt, size))
     mem
@@ -92,7 +92,7 @@ sealed abstract class MemBase[T <: Data](t: T, val length: Int) extends HasId {
 
     val port = pushCommand(
       DefMemPort(sourceInfo,
-       t.chiselCloneType, Node(this), dir, i.ref, Node(Builder.forcedClock))
+       t.cloneTypeFull, Node(this), dir, i.ref, Node(Builder.forcedClock))
     ).id
     // Bind each element of port to being a MemoryPort
     port.bind(MemoryPortBinding(Builder.forcedUserModule))
@@ -126,7 +126,7 @@ object SyncReadMem {
     if (compileOptions.declaredTypeMustBeUnbound) {
       requireIsChiselType(t, "memory type")
     }
-    val mt  = t.chiselCloneType
+    val mt  = t.cloneTypeFull
     val mem = new SyncReadMem(mt, size)
     pushCommand(DefSeqMemory(sourceInfo, mem, mt, size))
     mem
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Reg.scala b/chiselFrontend/src/main/scala/chisel3/core/Reg.scala
index 19bbee1c..14674b37 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Reg.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Reg.scala
@@ -19,7 +19,7 @@ object Reg {
     if (compileOptions.declaredTypeMustBeUnbound) {
       requireIsChiselType(t, "reg type")
     }
-    val reg = t.chiselCloneType
+    val reg = t.cloneTypeFull
     val clock = Node(Builder.forcedClock)
 
     reg.bind(RegBinding(Builder.forcedUserModule))
@@ -36,7 +36,7 @@ object RegNext {
   def apply[T <: Data](next: T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
     val model = (next match {
       case next: Bits => next.cloneTypeWidth(Width())
-      case next => next.chiselCloneType
+      case next => next.cloneTypeFull
     }).asInstanceOf[T]
     val reg = Reg(model)
 
@@ -53,7 +53,7 @@ object RegNext {
   def apply[T <: Data](next: T, init: T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
     val model = (next match {
       case next: Bits => next.cloneTypeWidth(Width())
-      case next => next.chiselCloneType
+      case next => next.cloneTypeFull
     }).asInstanceOf[T]
     val reg = RegInit(model, init)  // TODO: this makes NO sense
 
@@ -71,10 +71,10 @@ object RegInit {
   def apply[T <: Data](init: T)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
     val model = (init.litArg match {
       // For e.g. Reg(init=UInt(0, k)), fix the Reg's width to k
-      case Some(lit) if lit.forcedWidth => init.chiselCloneType
+      case Some(lit) if lit.forcedWidth => init.cloneTypeFull
       case _ => init match {
         case init: Bits => init.cloneTypeWidth(Width())
-        case init => init.chiselCloneType
+        case init => init.cloneTypeFull
       }
     }).asInstanceOf[T]
     RegInit(model, init)
@@ -86,7 +86,7 @@ object RegInit {
     if (compileOptions.declaredTypeMustBeUnbound) {
       requireIsChiselType(t, "reg type")
     }
-    val reg = t.chiselCloneType
+    val reg = t.cloneTypeFull
     val clock = Node(Builder.forcedClock)
     val reset = Node(Builder.forcedReset)