Fix ModuleChoice under D/I (#4569)

* Use FileCheck in ModuleChoice tests; add a test
* DRY out module definitions in ModuleChoiceSpec
* Add a ModuleChoice test using Definition
* Ensure Groups are propagated to parent builder under D/I

Author: davidbiancolin

core/src/main/scala/chisel3/experimental/hierarchy/core/Definition.scala

@@ -126,6 +126,7 @@ object Definition extends SourceInfoDoc {
     Builder.components ++= ir.components
     Builder.annotations ++= ir.annotations: @nowarn // this will go away when firrtl is merged
     Builder.layers ++= dynamicContext.layers
+    Builder.options ++= dynamicContext.options
     module._circuit = Builder.currentModule
     module.toDefinition
   }

src/test/scala/chiselTests/ModuleChoiceSpec.scala

@@ -5,6 +5,7 @@ package chiselTests
 import chisel3._
 import chisel3.choice.{Case, Group, ModuleChoice}
 import chiselTests.{ChiselFlatSpec, MatchesAndOmits, Utils}
+import chisel3.experimental.hierarchy.Definition
 import _root_.circt.stage.ChiselStage
 
 object Platform extends Group {
@@ -17,54 +18,69 @@ class TargetIO(width: Int) extends Bundle {
   val out = UInt(width.W)
 }
 
-class FPGATarget extends FixedIOExtModule[TargetIO](new TargetIO(8))
+class FPGATarget extends FixedIORawModule[TargetIO](new TargetIO(8)) {
+  io.out := io.in
+}
 
 class ASICTarget extends FixedIOExtModule[TargetIO](new TargetIO(8))
 
-class VerifTarget extends FixedIORawModule[TargetIO](new TargetIO(8))
+class VerifTarget extends FixedIORawModule[TargetIO](new TargetIO(8)) {
+  io.out := io.in
+}
 
-class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits {
-  it should "emit options and cases" in {
-    class ModuleWithChoice extends Module {
-      val out = IO(UInt(8.W))
+class ModuleWithChoice[T <: Data](
+  default:        => FixedIOBaseModule[T]
+)(alternateImpls: Seq[(Case, () => FixedIOBaseModule[T])])
+    extends Module {
+  val inst = ModuleChoice(default)(alternateImpls)
+  val io = IO(inst.cloneType)
+  io <> inst
+}
 
-      val inst =
-        ModuleChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget, Platform.ASIC -> new ASICTarget))
+class ModuleChoiceSpec extends ChiselFlatSpec with Utils with FileCheck {
+  it should "emit options and cases" in {
+    class ModuleWithValidChoices
+        extends ModuleWithChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget, Platform.ASIC -> new ASICTarget))
+
+    generateFirrtlAndFileCheck(new ModuleWithValidChoices)(
+      """|CHECK: option Platform :
+         |CHECK-NEXT: FPGA
+         |CHECK-NEXT: ASIC
+         |CHECK: instchoice inst of VerifTarget, Platform :
+         |CHECK-NEXT: FPGA => FPGATarget
+         |CHECK-NEXT: ASIC => ASICTarget""".stripMargin
+    )
+  }
 
-      inst.in := 42.U(8.W)
-      out := inst.out
+  it should "emit options and cases for Modules including definitions" in {
+    class ModuleWithValidChoices
+        extends ModuleWithChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget, Platform.ASIC -> new ASICTarget))
+    class TopWithDefinition extends Module {
+      val definitionWithChoice = Definition(new ModuleWithValidChoices)
     }
 
-    val chirrtl = ChiselStage.emitCHIRRTL(new ModuleWithChoice, Array("--full-stacktrace"))
-
-    info("CHIRRTL emission looks correct")
-    matchesAndOmits(chirrtl)(
-      "option Platform :",
-      "FPGA",
-      "ASIC",
-      "instchoice inst of VerifTarget, Platform :",
-      "FPGA => FPGATarget",
-      "ASIC => ASICTarget"
-    )()
+    generateFirrtlAndFileCheck(new TopWithDefinition)(
+      """|CHECK: option Platform :
+         |CHECK-NEXT: FPGA
+         |CHECK-NEXT: ASIC
+         |CHECK: instchoice inst of VerifTarget, Platform :
+         |CHECK-NEXT: FPGA => FPGATarget
+         |CHECK-NEXT: ASIC => ASICTarget""".stripMargin
+    )
   }
 
   it should "require that all cases are part of the same option" in {
 
-    class MixedOptions extends Module {
-      object Performance extends Group {
-        object Fast extends Case
-        object Small extends Case
-      }
-
-      val out = IO(UInt(8.W))
-
-      val inst =
-        ModuleChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget, Performance.Fast -> new ASICTarget))
-
-      inst.in := 42.U(8.W)
-      out := inst.out
+    object Performance extends Group {
+      object Fast extends Case
+      object Small extends Case
     }
 
+    class MixedOptions
+        extends ModuleWithChoice(new VerifTarget)(
+          Seq(Platform.FPGA -> new FPGATarget, Performance.Fast -> new ASICTarget)
+        )
+
     intercept[IllegalArgumentException] { ChiselStage.emitCHIRRTL(new MixedOptions) }.getMessage() should include(
       "cannot mix choices from different groups: Platform, Performance"
     )
@@ -73,33 +89,33 @@ class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits {
 
   it should "require that at least one alternative is present" in {
 
-    class MixedOptions extends Module {
-      val out = IO(UInt(8.W))
-
-      val inst =
-        ModuleChoice(new VerifTarget)(Seq())
+    class NoAlternatives extends ModuleWithChoice(new VerifTarget)(Seq())
 
-      inst.in := 42.U(8.W)
-      out := inst.out
-    }
-
-    intercept[IllegalArgumentException] { ChiselStage.emitCHIRRTL(new MixedOptions) }.getMessage() should include(
+    intercept[IllegalArgumentException] { ChiselStage.emitCHIRRTL(new NoAlternatives) }.getMessage() should include(
       "at least one alternative must be specified"
     )
 
   }
 
-  it should "require that all cases are distinct" in {
+  it should "allow a subset of options to be provided" in {
+
+    class SubsetOptions extends ModuleWithChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget))
 
-    class MixedOptions extends Module {
-      val out = IO(UInt(8.W))
+    // Note, because of a quirk in how [[Case]]s are registered, only those referenced
+    // in the Module here are going to be captured. This will be fixed in a forthcoming PR
+    // that implements an [[addLayer]] like feature for [[Group]]s
+    generateFirrtlAndFileCheck(new SubsetOptions)(
+      """|CHECK: option Platform :
+         |CHECK-NEXT: FPGA
+         |CHECK: instchoice inst of VerifTarget, Platform :
+         |CHECK-NEXT: FPGA => FPGATarget""".stripMargin
+    )
+  }
 
-      val inst =
-        ModuleChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget, Platform.FPGA -> new ASICTarget))
+  it should "require that all cases are distinct" in {
 
-      inst.in := 42.U(8.W)
-      out := inst.out
-    }
+    class MixedOptions
+        extends ModuleWithChoice(new VerifTarget)(Seq(Platform.FPGA -> new FPGATarget, Platform.FPGA -> new ASICTarget))
 
     intercept[IllegalArgumentException] { ChiselStage.emitCHIRRTL(new MixedOptions) }.getMessage() should include(
       "duplicate case 'FPGA'"
@@ -109,17 +125,9 @@ class ModuleChoiceSpec extends ChiselFlatSpec with Utils with MatchesAndOmits {
 
   it should "require that all IO bundles are type equivalent" in {
 
-    class MixedIO extends Module {
-      val out = IO(UInt(8.W))
+    class Target16 extends FixedIOExtModule[TargetIO](new TargetIO(16))
 
-      class Target16 extends FixedIOExtModule[TargetIO](new TargetIO(16))
-
-      val inst =
-        ModuleChoice(new VerifTarget)(Seq(Platform.FPGA -> new Target16))
-
-      inst.in := 42.U(8.W)
-      out := inst.out
-    }
+    class MixedIO extends ModuleWithChoice(new VerifTarget)(Seq(Platform.FPGA -> new Target16))
 
     intercept[ChiselException] { ChiselStage.emitCHIRRTL(new MixedIO, Array("--throw-on-first-error")) }
       .getMessage() should include(