Merge pull request #1804 from chipsalliance/autoclonetype2

Compiler plugin implemented autoclonetype

4 files changed, 220 insertions, 116 deletions

diff --git a/src/test/scala/chisel3/testers/TestUtils.scala b/src/test/scala/chisel3/testers/TestUtils.scala
index 12712bf7..c72c779a 100644
--- a/src/test/scala/chisel3/testers/TestUtils.scala
+++ b/src/test/scala/chisel3/testers/TestUtils.scala
@@ -3,10 +3,22 @@
 package chisel3.testers
 
 import TesterDriver.Backend
+import chisel3.{Bundle, RawModule}
+import chisel3.internal.firrtl.Circuit
+import chisel3.stage.ChiselStage
 import firrtl.AnnotationSeq
 
 object TestUtils {
   // Useful because TesterDriver.Backend is chisel3 package private
   def containsBackend(annos: AnnotationSeq): Boolean =
     annos.collectFirst { case b: Backend => b }.isDefined
+
+  // Allows us to check that the compiler plugin cloneType is actually working
+  val usingPlugin: Boolean = (new Bundle { def check = _usingPlugin }).check
+  def elaborateNoReflectiveAutoCloneType(f: => RawModule): Circuit = {
+    ChiselStage.elaborate {
+      chisel3.internal.Builder.allowReflectiveAutoCloneType = !usingPlugin
+      f
+    }
+  }
 }
diff --git a/src/test/scala/chiselTests/AutoClonetypeSpec.scala b/src/test/scala/chiselTests/AutoClonetypeSpec.scala
index b791297d..a6e5562a 100644
--- a/src/test/scala/chiselTests/AutoClonetypeSpec.scala
+++ b/src/test/scala/chiselTests/AutoClonetypeSpec.scala
@@ -3,7 +3,8 @@
 package chiselTests
 
 import chisel3._
-import chisel3.stage.ChiselStage
+import chisel3.testers.TestUtils
+import chisel3.util.QueueIO
 
 class BundleWithIntArg(val i: Int) extends Bundle {
   val out = UInt(i.W)
@@ -65,10 +66,19 @@ class NestedAnonymousBundle extends Bundle {
 // Not necessarily good style (and not necessarily recommended), but allowed to preserve compatibility.
 class BundleWithArgumentField(val x: Data, val y: Data) extends Bundle
 
+// Needs to be top-level so that reflective autoclonetype works
+class InheritingBundle extends QueueIO(UInt(8.W), 8) {
+  val error = Output(Bool())
+}
+
+// TODO all `.suggestNames` are due to https://github.com/chipsalliance/chisel3/issues/1802
 class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
+  val usingPlugin: Boolean = TestUtils.usingPlugin
+  val elaborate = TestUtils.elaborateNoReflectiveAutoCloneType _
+
   "Bundles with Scala args" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(new Bundle{})
+    elaborate { new Module {
+      val io = IO(new Bundle{}).suggestName("io")
 
       val myWire = Wire(new BundleWithIntArg(8))
       assert(myWire.i == 8)
@@ -76,8 +86,8 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
   }
 
   "Bundles with Scala implicit args" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(new Bundle{})
+    elaborate { new Module {
+      val io = IO(new Bundle{}).suggestName("io")
 
       implicit val implicitInt: Int = 4
       val myWire = Wire(new BundleWithImplicit())
@@ -87,8 +97,8 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
   }
 
   "Bundles with Scala explicit and impicit args" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(new Bundle{})
+    elaborate { new Module {
+      val io = IO(new Bundle{}).suggestName("io")
 
       implicit val implicitInt: Int = 4
       val myWire = Wire(new BundleWithArgAndImplicit(8))
@@ -99,16 +109,16 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
   }
 
   "Subtyped Bundles" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(new Bundle{})
+    elaborate { new Module {
+      val io = IO(new Bundle{}).suggestName("io")
 
       val myWire = Wire(new SubBundle(8, 4))
 
       assert(myWire.i == 8)
       assert(myWire.i2 == 4)
     } }
-    ChiselStage.elaborate { new Module {
-      val io = IO(new Bundle{})
+    elaborate { new Module {
+      val io = IO(new Bundle{}).suggestName("io")
 
       val myWire = Wire(new SubBundleVal(8, 4))
 
@@ -117,68 +127,84 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
     } }
   }
 
-  "Subtyped Bundles that don't clone well" should "be caught" in {
-    a [ChiselException] should be thrownBy extractCause[ChiselException] {
-      ChiselStage.elaborate { new Module {
-        val io = IO(new Bundle{})
-        val myWire = Wire(new SubBundleInvalid(8, 4))
-      } }
+  def checkSubBundleInvalid() = {
+    elaborate { new Module {
+      val io = IO(new Bundle{}).suggestName("io")
+      val myWire = Wire(new SubBundleInvalid(8, 4))
+    } }
+  }
+  if (usingPlugin) {
+    "Subtyped Bundles that don't clone well" should "be now be supported!" in {
+      checkSubBundleInvalid()
+    }
+  } else {
+    "Subtyped Bundles that don't clone well" should "be caught" in {
+      a [ChiselException] should be thrownBy extractCause[ChiselException] {
+        checkSubBundleInvalid()
+      }
     }
   }
 
   "Inner bundles with Scala args" should "not need clonetype" in {
-    ChiselStage.elaborate { new ModuleWithInner }
+    elaborate { new ModuleWithInner }
   }
 
   "Bundles with arguments as fields" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(Output(new BundleWithArgumentField(UInt(8.W), UInt(8.W))))
+    elaborate { new Module {
+      val io = IO(Output(new BundleWithArgumentField(UInt(8.W), UInt(8.W)))).suggestName("io")
       io.x := 1.U
       io.y := 1.U
     } }
   }
 
+  it should "also work when giving directions to the fields" in {
+    elaborate { new Module {
+      val io = IO(new BundleWithArgumentField(Input(UInt(8.W)), Output(UInt(8.W)))).suggestName("io")
+      io.y := io.x
+    } }
+  }
+
   "Bundles inside companion objects" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(Output(new CompanionObjectWithBundle.Inner))
+    elaborate { new Module {
+      val io = IO(Output(new CompanionObjectWithBundle.Inner)).suggestName("io")
       io.data := 1.U
     } }
   }
 
   "Parameterized bundles inside companion objects" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(Output(new CompanionObjectWithBundle.ParameterizedInner(8)))
+    elaborate { new Module {
+      val io = IO(Output(new CompanionObjectWithBundle.ParameterizedInner(8))).suggestName("io")
       io.data := 1.U
     } }
   }
 
   "Nested directioned anonymous Bundles" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
-      val io = IO(new NestedAnonymousBundle)
+    elaborate { new Module {
+      val io = IO(new NestedAnonymousBundle).suggestName("io")
       val a = WireDefault(io)
       io.a.a := 1.U
     } }
   }
 
   "3.0 null compatibility" should "not need clonetype" in {
-    ChiselStage.elaborate { new Module {
+    elaborate { new Module {
       class InnerClassThing {
         def createBundle: Bundle = new Bundle {
           val a = Output(UInt(8.W))
         }
       }
-      val io = IO((new InnerClassThing).createBundle)
+      val io = IO((new InnerClassThing).createBundle).suggestName("io")
       val a = WireDefault(io)
     } }
   }
 
   "Aliased fields" should "be caught" in {
     a [ChiselException] should be thrownBy extractCause[ChiselException] {
-      ChiselStage.elaborate { new Module {
+      elaborate { new Module {
         val bundleFieldType = UInt(8.W)
         val io = IO(Output(new Bundle {
           val a = bundleFieldType
-        }))
+        })).suggestName("io")
         io.a := 0.U
       } }
     }
@@ -190,8 +216,8 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
         val a = typeTuple._1
       }
 
-      ChiselStage.elaborate { new Module {
-        val io = IO(Output(new BadBundle(UInt(8.W), 1)))
+      elaborate { new Module {
+        val io = IO(Output(new BadBundle(UInt(8.W), 1))).suggestName("io")
         io.a := 0.U
       } }
     }
@@ -204,7 +230,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
         val x = Output(UInt(3.W))
       }))
     }
-    ChiselStage.elaborate { new TestModule }
+    elaborate { new TestModule }
   }
 
   "Wrapped IO construction with parent references" should "not fail for autoclonetype" in {
@@ -216,6 +242,108 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils {
         val x = Output(UInt(blah.W))
       }))
     }
-    ChiselStage.elaborate { new TestModule(3) }
+    elaborate { new TestModule(3) }
+  }
+
+  "Autoclonetype" should "support Bundles with if-blocks" in {
+    class MyModule(n: Int) extends MultiIOModule {
+      val io = IO(new Bundle {
+        val in = Input(UInt(8.W))
+        val out = Output(UInt(8.W))
+        if (n > 4) {
+          println("Here we are!")
+        }
+      })
+      io.out := io.in
+    }
+    elaborate(new MyModule(3))
+  }
+
+  behavior of "Compiler Plugin Autoclonetype"
+
+  // Necessary test for 3.4.x, but we will break this (for non-plugin users) in 3.5
+  it should "NOT break code that extends chisel3.util Bundles (whether they use the plugin or not)" in {
+    class MyModule extends MultiIOModule {
+      val io = IO(new InheritingBundle)
+      io.deq <> io.enq
+      io.count := 0.U
+      io.error := true.B
+    }
+    elaborate(new MyModule)
+  }
+
+  // New tests from the plugin
+  if (usingPlugin) {
+    it should "support Bundles with non-val parameters" in {
+      class MyBundle(i: Int) extends Bundle {
+        val foo = UInt(i.W)
+      }
+      elaborate { new MultiIOModule {
+        val in = IO(Input(new MyBundle(8)))
+        val out = IO(Output(new MyBundle(8)))
+        out := in
+      }}
+    }
+
+    it should "support type-parameterized Bundles" in {
+      class MyBundle[T <: Data](gen: T) extends Bundle {
+        val foo = gen
+      }
+      elaborate { new MultiIOModule {
+        val in = IO(Input(new MyBundle(UInt(8.W))))
+        val out = IO(Output(new MyBundle(UInt(8.W))))
+        out := in
+      }}
+    }
+
+    it should "support Bundles with non-val implicit parameters" in {
+      class MyBundle(implicit i: Int) extends Bundle {
+        val foo = UInt(i.W)
+      }
+      elaborate { new MultiIOModule {
+        implicit val x = 8
+        val in = IO(Input(new MyBundle))
+        val out = IO(Output(new MyBundle))
+        out := in
+      }}
+    }
+
+    it should "support Bundles with multiple parameter lists" in {
+      class MyBundle(i: Int)(j: Int, jj: Int)(k: UInt) extends Bundle {
+        val foo = UInt((i + j + jj + k.getWidth).W)
+      }
+      elaborate {
+        new MultiIOModule {
+          val in = IO(Input(new MyBundle(8)(8, 8)(UInt(8.W))))
+          val out = IO(Output(new MyBundle(8)(8, 8)(UInt(8.W))))
+          out := in
+        }
+      }
+    }
+
+    it should "support Bundles that implement their own cloneType" in {
+      class MyBundle(i: Int) extends Bundle {
+        val foo = UInt(i.W)
+        override def cloneType = new MyBundle(i).asInstanceOf[this.type]
+      }
+      elaborate { new MultiIOModule {
+        val in = IO(Input(new MyBundle(8)))
+        val out = IO(Output(new MyBundle(8)))
+        out := in
+      }}
+    }
+
+    it should "support Bundles that capture type parameters from their parent scope" in {
+      class MyModule[T <: Data](gen: T) extends MultiIOModule {
+        class MyBundle(n: Int) extends Bundle {
+          val foo = Vec(n, gen)
+        }
+        val in = IO(Input(new MyBundle(4)))
+        val out = IO(Output(new MyBundle(4)))
+        out := in
+      }
+      elaborate(new MyModule(UInt(8.W)))
+    }
+
   }
 }
diff --git a/src/test/scala/chiselTests/AutoNestedCloneSpec.scala b/src/test/scala/chiselTests/AutoNestedCloneSpec.scala
index 8e40ad20..401766e2 100644
--- a/src/test/scala/chiselTests/AutoNestedCloneSpec.scala
+++ b/src/test/scala/chiselTests/AutoNestedCloneSpec.scala
@@ -1,10 +1,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
 package chiselTests
-import Chisel.ChiselException
-import org.scalatest._
 import chisel3._
-import chisel3.stage.ChiselStage
+import chisel3.testers.TestUtils
 import org.scalatest.matchers.should.Matchers
 
 class BundleWithAnonymousInner(val w: Int) extends Bundle {
@@ -13,11 +11,15 @@ class BundleWithAnonymousInner(val w: Int) extends Bundle {
   }
 }
 
+// TODO all `.suggestNames` are due to https://github.com/chipsalliance/chisel3/issues/1802
 class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
+  val usingPlugin: Boolean = TestUtils.usingPlugin
+  val elaborate = TestUtils.elaborateNoReflectiveAutoCloneType _
+
   behavior of "autoCloneType of inner Bundle in Chisel3"
 
   it should "clone a doubly-nested inner bundle successfully" in {
-    ChiselStage.elaborate {
+    elaborate {
       class Outer(val w: Int) extends Module {
         class Middle(val w: Int) {
           class InnerIOType extends Bundle {
@@ -25,7 +27,7 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
           }
           def getIO: InnerIOType = new InnerIOType
         }
-        val io = IO(new Bundle {})
+        val io = IO(new Bundle {}).suggestName("io")
         val myWire = Wire((new Middle(w)).getIO)
       }
       new Outer(2)
@@ -33,9 +35,9 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
   }
 
   it should "clone an anonymous inner bundle successfully" in {
-    ChiselStage.elaborate {
+    elaborate {
       class TestTop(val w: Int) extends Module {
-        val io = IO(new Bundle {})
+        val io = IO(new Bundle {}).suggestName("io")
         val myWire = Wire(new Bundle{ val a = UInt(w.W) })
       }
       new TestTop(2)
@@ -43,18 +45,18 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
   }
 
   it should "pick the correct $outer instance for an anonymous inner bundle" in {
-    ChiselStage.elaborate {
+    elaborate {
       class Inner(val w: Int) extends Module {
         val io = IO(new Bundle{
           val in = Input(UInt(w.W))
           val out = Output(UInt(w.W))
-        })
+        }).suggestName("io")
       }
       class Outer(val w: Int) extends Module {
         val io = IO(new Bundle{
           val in = Input(UInt(w.W))
           val out = Output(UInt(w.W))
-        })
+        }).suggestName("io")
         val i = Module(new Inner(w))
         val iw = Wire(chiselTypeOf(i.io))
         iw <> io
@@ -65,9 +67,9 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
   }
 
   it should "clone an anonymous, bound, inner bundle of another bundle successfully" in {
-    ChiselStage.elaborate {
+    elaborate {
       class TestModule(w: Int) extends Module {
-        val io = IO(new BundleWithAnonymousInner(w) )
+        val io = IO(new BundleWithAnonymousInner(w) ).suggestName("io")
         val w0 = WireDefault(io)
         val w1 = WireDefault(io.inner)
       }
@@ -76,14 +78,14 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
   }
 
   it should "clone an anonymous, inner bundle of a Module, bound to another bundle successfully" in {
-    ChiselStage.elaborate {
+    elaborate {
       class TestModule(w: Int) extends Module {
         val bun = new Bundle {
           val foo = UInt(w.W)
         }
         val io = IO(new Bundle {
           val inner = Input(bun)
-        })
+        }).suggestName("io")
         val w0 = WireDefault(io)
         val w1 = WireDefault(io.inner)
       }
@@ -92,31 +94,48 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils {
   }
 
   it should "clone a double-nested anonymous Bundle" in {
-    ChiselStage.elaborate {
+    elaborate {
       class TestModule() extends Module {
         val io = IO(new Bundle {
           val inner = Input(new Bundle {
             val x = UInt(8.W)
           })
-        })
+        }).suggestName("io")
       }
       new TestModule()
     }
   }
 
-  // Test ignored because the compatibility null-inserting autoclonetype doesn't trip this
-  ignore should "fail on anonymous doubly-nested inner bundle with clear error" in {
-    intercept[ChiselException] { extractCause[ChiselException] { ChiselStage.elaborate {
-      class Outer(val w: Int) extends Module {
-        class Middle(val w: Int) {
-          def getIO: Bundle = new Bundle {
-            val in = Input(UInt(w.W))
+  if (usingPlugin) {
+    // This works with the plugin, but is a null pointer exception when using reflective autoclonetype
+    it should "support an anonymous doubly-nested inner bundle" in {
+      elaborate {
+        class Outer(val w: Int) extends Module {
+          class Middle(val w: Int) {
+            def getIO: Bundle = new Bundle {
+              val in = Input(UInt(w.W))
+            }
           }
+          val io = IO(new Bundle {}).suggestName("io")
+          val myWire = Wire((new Middle(w)).getIO)
         }
-        val io = IO(new Bundle {})
-        val myWire = Wire((new Middle(w)).getIO)
+        new Outer(2)
       }
-      new Outer(2)
-    }}}.getMessage should include("Unable to determine instance")
+    }
+
+    it should "support anonymous Inner bundles that capture type parameters from outer Bundles" in {
+      elaborate(new MultiIOModule {
+        class MyBundle[T <: Data](n: Int, gen: T) extends Bundle {
+          val foo = new Bundle {
+            val x = Input(Vec(n, gen))
+          }
+          val bar = Output(Option(new { def mkBundle = new Bundle { val x = Vec(n, gen) }}).get.mkBundle)
+        }
+        val io = IO(new MyBundle(4, UInt(8.W)))
+        val myWire = WireInit(io.foo)
+        val myWire2 = WireInit(io.bar)
+        io.bar.x := io.foo.x
+      })
+    }
   }
 }
diff --git a/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala b/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala
deleted file mode 100644
index 28673495..00000000
--- a/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala
+++ /dev/null
@@ -1,55 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0
-
-package chiselTests
-import Chisel.ChiselException
-import chisel3.stage.ChiselStage
-import org.scalatest._
-import org.scalatest.matchers.should.Matchers
-
-class MissingCloneBindingExceptionSpec extends ChiselFlatSpec with Matchers with Utils {
-  behavior of "missing cloneType in Chisel3"
-  ( the [ChiselException] thrownBy extractCause[ChiselException] {
-    import chisel3._
-
-    class Test extends Module {
-      class TestIO(w: Int) extends Bundle {
-        val a = Input(Vec(4, UInt(w.W)))
-      }
-
-      val io = IO(new TestIO(32))
-    }
-
-    class TestTop extends Module {
-      val io = IO(new Bundle {})
-
-      val subs = VecInit(Seq.fill(2) {
-        Module(new Test).io
-      })
-    }
-
-    ChiselStage.elaborate(new TestTop)
-  }).getMessage should include("make all parameters immutable")
-
-  behavior of "missing cloneType in Chisel2"
-  ( the [ChiselException] thrownBy extractCause[ChiselException] {
-    import Chisel._
-
-    class Test extends Module {
-      class TestIO(w: Int) extends Bundle {
-        val a = Vec(4, UInt(width = w)).asInput
-      }
-
-      val io = IO(new TestIO(32))
-    }
-
-    class TestTop extends Module {
-      val io = IO(new Bundle {})
-
-      val subs = Vec.fill(2) {
-        Module(new Test).io
-      }
-    }
-
-    ChiselStage.elaborate(new TestTop)
-  }).getMessage should include("make all parameters immutable")
-}