Cloning IO with compatibility 🦆 (#754)

Changes the API such that IO(...) clones. All Bundles will need to be clone-able, but auto clone type is expected to handle most cases.

4 files changed, 36 insertions, 31 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala
index 3fdc383c..a6cc8bac 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala
@@ -23,7 +23,7 @@ object Binding {
 object requireIsHardware {
   def apply(node: Data, msg: String = "") = {
     node._parent match {  // Compatibility layer hack
-      case Some(x: BaseModule) => x._autoWrapPorts
+      case Some(x: BaseModule) => x._compatAutoWrapPorts
       case _ =>
     }
     if (!node.hasBinding) {
diff --git a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala
index aa0f8064..ff54e9e6 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala
@@ -128,10 +128,10 @@ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param
   def io: Record
 
   // Allow access to bindings from the compatibility package
-  protected def _ioPortBound() = portsContains(io)
+  protected def _compatIoPortBound() = portsContains(io)
 
   private[core] override def generateComponent(): Component = {
-    _autoWrapPorts()  // pre-IO(...) compatibility hack
+    _compatAutoWrapPorts()  // pre-IO(...) compatibility hack
 
     // Restrict IO to just io, clock, and reset
     require(io != null, "BlackBox must have io")
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
index fa9ab082..74479c6b 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
@@ -191,7 +191,7 @@ abstract class BaseModule extends HasId {
     *
     * TODO: remove this, perhaps by removing Bindings checks in compatibility mode.
     */
-  def _autoWrapPorts() {}
+  def _compatAutoWrapPorts() {}
 
   //
   // BaseModule User API functions
@@ -200,29 +200,10 @@ abstract class BaseModule extends HasId {
     Builder.annotations += annotation
   }
 
-  /**
-   * This must wrap the datatype used to set the io field of any Module.
-   * i.e. All concrete modules must have defined io in this form:
-   * [lazy] val io[: io type] = IO(...[: io type])
-   *
-   * Items in [] are optional.
-   *
-   * The granted iodef WILL NOT be cloned (to allow for more seamless use of
-   * anonymous Bundles in the IO) and thus CANNOT have been bound to any logic.
-   * This will error if any node is bound (e.g. due to logic in a Bundle
-   * constructor, which is considered improper).
-   *
-   * Also registers a Data as a port, also performing bindings. Cannot be called once ports are
-   * requested (so that all calls to ports will return the same information).
-   * Internal API.
-   *
-   * TODO(twigg): Specifically walk the Data definition to call out which nodes
-   * are problematic.
-   */
-  protected def IO[T<:Data](iodef: T): iodef.type = {
-    require(!_closed, "Can't add more ports after module close")
-    requireIsChiselType(iodef, "io type")
-
+  /** Chisel2 code didn't require the IO(...) wrapper and would assign a Chisel type directly to
+    * io, then do operations on it. This binds a Chisel type in-place (mutably) as an IO.
+    */
+  protected def _bindIoInPlace(iodef: Data): Unit = {
     // Compatibility code: Chisel2 did not require explicit direction on nodes
     // (unspecified treated as output, and flip on nothing was input).
     // This sets assigns the explicit directions required by newer semantics on
@@ -249,10 +230,34 @@ abstract class BaseModule extends HasId {
     }
     assignCompatDir(iodef, false)
 
-    // Bind each element of the iodef to being a Port
     iodef.bind(PortBinding(this))
     _ports += iodef
-    iodef
+  }
+
+  /**
+   * This must wrap the datatype used to set the io field of any Module.
+   * i.e. All concrete modules must have defined io in this form:
+   * [lazy] val io[: io type] = IO(...[: io type])
+   *
+   * Items in [] are optional.
+   *
+   * The granted iodef must be a chisel type and not be bound to hardware.
+   *
+   * Also registers a Data as a port, also performing bindings. Cannot be called once ports are
+   * requested (so that all calls to ports will return the same information).
+   * Internal API.
+   *
+   * TODO(twigg): Specifically walk the Data definition to call out which nodes
+   * are problematic.
+   */
+  protected def IO[T<:Data](iodef: T): T = {
+    require(!_closed, "Can't add more ports after module close")
+    requireIsChiselType(iodef, "io type")
+
+    // Clone the IO so we preserve immutability of data types
+    val iodefClone = iodef.cloneTypeFull
+    _bindIoInPlace(iodefClone)
+    iodefClone
   }
 
   //
diff --git a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala
index 1411fa80..17b8a09e 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala
@@ -157,7 +157,7 @@ abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions)
   def io: Record
 
   // Allow access to bindings from the compatibility package
-  protected def _ioPortBound() = portsContains(io)
+  protected def _compatIoPortBound() = portsContains(io)
 
   protected override def nameIds(rootClass: Class[_]): HashMap[HasId, String] = {
     val names = super.nameIds(rootClass)
@@ -171,7 +171,7 @@ abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions)
   }
 
   private[core] override def generateComponent(): Component = {
-    _autoWrapPorts()  // pre-IO(...) compatibility hack
+    _compatAutoWrapPorts()  // pre-IO(...) compatibility hack
 
     // Restrict IO to just io, clock, and reset
     require(io != null, "Module must have io")