Chisel plugin bundle elements handler (#2306) (#2380)

Adds generation of `Bundle.elements` method to the chores done by the compiler plugin
For each `Bundle` find the relevant visible Chisel field members and construct a
hard-coded list of the elements and their names implemented as `_elementsImpl`

For more details: See plugins/README.md

- Should be no change in API
- Handles inheritance and mixins
- Handles Seq[Data]
- Tests in BundleElementSpec

Co-authored-by: chick <chick.markley@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
(cherry picked from commit 237200a420581519f29149cbae9b3e968c0d01fc)

Co-authored-by: Chick Markley <chick@qrhino.com>

9 files changed, 887 insertions, 45 deletions

diff --git a/build.sbt b/build.sbt
index b9406c8f..f5f178fd 100644
--- a/build.sbt
+++ b/build.sbt
@@ -198,6 +198,7 @@ lazy val chisel = (project in file(".")).
   settings(
     mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3" % "3.5.0"),
     libraryDependencies += defaultVersions("treadle") % "test",
+    Test / scalacOptions += "-P:chiselplugin:genBundleElements",
     scalacOptions in Test ++= Seq("-language:reflectiveCalls"),
     scalacOptions in Compile in doc ++= Seq(
       "-diagrams",
diff --git a/build.sc b/build.sc
index 1e665a9f..d7dbc6d7 100644
--- a/build.sc
+++ b/build.sc
@@ -107,6 +107,10 @@ class chisel3CrossModule(val crossScalaVersion: String) extends CommonModule wit
   object test extends Tests {
     override def scalacPluginClasspath = m.scalacPluginClasspath
 
+    override  def scalacOptions = T {
+      super.scalacOptions() ++ Agg("-P:chiselplugin:genBundleElements")
+    }
+
     override def ivyDeps = m.ivyDeps() ++ Agg(
       ivy"org.scalatest::scalatest:3.2.10",
       ivy"org.scalatestplus::scalacheck-1-14:3.2.2.0",
diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
index 15cdf428..75966a16 100644
--- a/core/src/main/scala/chisel3/Aggregate.scala
+++ b/core/src/main/scala/chisel3/Aggregate.scala
@@ -1208,12 +1208,58 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     * }}}
     */
   final lazy val elements: SeqMap[String, Data] = {
+    val hardwareFields = _elementsImpl.flatMap {
+      case (name, data: Data) =>
+        if (data.isSynthesizable) {
+          Some(s"$name: $data")
+        } else {
+          None
+        }
+      case (name, Some(data: Data)) =>
+        if (data.isSynthesizable) {
+          Some(s"$name: $data")
+        } else {
+          None
+        }
+      case (name, s: scala.collection.Seq[Any]) if s.nonEmpty =>
+        s.head match {
+          // Ignore empty Seq()
+          case d: Data =>
+            throwException(
+              "Public Seq members cannot be used to define Bundle elements " +
+                s"(found public Seq member '${name}'). " +
+                "Either use a Vec if all elements are of the same type, or MixedVec if the elements " +
+                "are of different types. If this Seq member is not intended to construct RTL, mix in the trait " +
+                "IgnoreSeqInBundle."
+            )
+          case _ => // don't care about non-Data Seq
+        }
+        None
+
+      case _ => None
+    }
+    if (hardwareFields.nonEmpty) {
+      throw ExpectedChiselTypeException(s"Bundle: $this contains hardware fields: " + hardwareFields.mkString(","))
+    }
+    VectorMap(_elementsImpl.toSeq.flatMap {
+      case (name, data: Data) =>
+        Some(name -> data)
+      case (name, Some(data: Data)) =>
+        Some(name -> data)
+      case _ => None
+    }.sortWith {
+      case ((an, a), (bn, b)) => (a._id > b._id) || ((a eq b) && (an > bn))
+    }: _*)
+  }
+  /*
+   * This method will be overwritten by the Chisel-Plugin
+   */
+  protected def _elementsImpl: SeqMap[String, Any] = {
     val nameMap = LinkedHashMap[String, Data]()
     for (m <- getPublicFields(classOf[Bundle])) {
       getBundleField(m) match {
         case Some(d: Data) =>
-          requireIsChiselType(d)
-
+          // Checking for chiselType is done in elements method
           if (nameMap contains m.getName) {
             require(nameMap(m.getName) eq d)
           } else {
diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
index 9e9f5dbb..ef43a3b0 100644
--- a/core/src/main/scala/chisel3/Data.scala
+++ b/core/src/main/scala/chisel3/Data.scala
@@ -480,7 +480,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
   protected[chisel3] def binding: Option[Binding] = _binding
   protected def binding_=(target: Binding) {
     if (_binding.isDefined) {
-      throw RebindingException(s"Attempted reassignment of binding to $this")
+      throw RebindingException(s"Attempted reassignment of binding to $this, from: ${target}")
     }
     _binding = Some(target)
   }
diff --git a/plugin/README.md b/plugin/README.md
new file mode 100644
index 00000000..762f4822
--- /dev/null
+++ b/plugin/README.md
@@ -0,0 +1,99 @@
+# Notes on the Compiler Plug-in
+
+The Chisel plugin provides some operations that are too difficult, or not possbile,
+to implement through regular Scala code. 
+
+# This documentation is for developers working on chisel internals.
+
+## Compiler plugin operations
+These are the two things that the compile plugin does.
+
+1. Automatically generates the `cloneType` methods of Bundle
+2. Changes the underlying mechanics of the `Bundle`s `elements` method in a way 
+that does not require the use of **reflection** 
+3. Future work: Make having a Seq[Data] in a bundle be a compiler error. See "Detecting Bundles with Seq[Data]" below.
+
+### 1. Generating `cloneType` method
+As of  Mar 18, 2021, PR #1826, generating the `cloneType` method (1. above) is now the default behavior.
+The cloneType method used to be a tricky thing to write for chisel developers.
+For historical purposes, here is the flag was used to control that prior to full adoption.
+```
+-P:chiselplugin:useBundlePlugin
+```
+
+### 2. Changing `Bundle#elements` method
+
+A `Bundle` has a default `elements` method that relies on **reflection**, which is slow and brittle, to access the list of
+*fields* the bundle contains.
+When enabled this second operation of the plugin examines
+the `Bundle`s AST in order to determine the fields and then re-writes the underlying code of `elements`.
+Technically, rewriting a lower level private method `_elementsImpl`.
+It is expected that the using this feature will shortly become the default.
+
+>The plugin should not be enabled for the `main` chisel3 project because of internal considerations.
+> It is enabled for the `Test` section.
+
+In the meantime, advanced users can try using the feature by adding the following flag to the scalac options in their
+chisel projects.
+
+```
+-P:chiselplugin:buildElementAccessor
+```
+
+For example in an `build.sbt` file adding the line 
+```
+scalacOptions += "-P:chiselplugin:genBundleElements",
+```
+in the appropriate place.
+
+## Future work
+### Detecting Bundles with Seq[Data]
+Trying to have a `val Seq[Data]` (as opposed to a `val Vec[Data]` in a `Bundle` is a run time error.
+Here is a block of code that could be added to the plugin to detect this case at compile time (with some refinement in
+the detection mechanism):
+```scala
+  if (member.isAccessor && typeIsSeqOfData(member.tpe) && !isIgnoreSeqInBundle(bundleSymbol)) {
+    global.reporter.error(
+      member.pos,
+      s"Bundle.field ${bundleSymbol.name}.${member.name} cannot be a Seq[Data]. " +
+        "Use Vec or MixedVec or mix in trait IgnoreSeqInBundle"
+    )
+  }
+```
+### Notes about working on the `_elementsImpl` generator for the plugin in `BundleComponent.scala`
+In general the easiest way to develop and debug new code in the plugin is to use `println` statements.
+Naively this can result in reams of text that can be very hard to look through.
+
+What I found to be useful was creating some wrappers for `println` that only printed when the `Bundles` had a particular name pattern.
+- Create a regular expression string in the `BundleComponent` class
+- Add a printf wrapper name `show` that checks the `Bundle`'s name against the regex
+- For recursive code in `getAllBundleFields` create a different wrapper `indentShow` that indents debug lines
+- Sprinkle calls to these wrappers as needed for debugging
+
+#### Bundle Regex
+```scala
+    val bundleNameDebugRegex = "MyBundle.*"
+```
+#### Add `show` wrapper
+`show` should be inside `case bundle` block of the `transform` method in order to have access to the current `Bundle`
+
+```scala
+def show(string: => String): Unit = {
+  if (bundle.symbol.name.toString.matches(bundleNameDebugRegex)) {
+    println(string)
+  }
+}
+```
+#### Add `indentShow` wrapper
+This method can be added into `BundleComponent.scala` in the `transform` method after `case Bundle`
+Inside of `getAllBundleFields` I added the following code that indented for each recursion up the current
+`Bundle`'s hierarchy.
+```scala
+def indentShow(s: => String): Unit = {
+  val indentString = ("-" * depth) * 2 + ">  "
+  s.split("\n").foreach { line =>
+    show(indentString + line)
+  }
+}
+```
+
diff --git a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala
index 2d3a2cae..e92bbb23 100644
--- a/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala
+++ b/plugin/src/main/scala/chisel3/internal/plugin/BundleComponent.scala
@@ -9,7 +9,16 @@ import scala.tools.nsc.plugins.PluginComponent
 import scala.tools.nsc.symtab.Flags
 import scala.tools.nsc.transform.TypingTransformers
 
-// TODO This component could also implement val elements in Bundles
+/** Performs three operations
+  * 1) Records that this plugin ran on a bundle by adding a method
+  *    `override protected def _usingPlugin: Boolean = true`
+  * 2) Constructs a cloneType method
+  * 3) Builds a `def elements` that is computed once in this plugin
+  *    Eliminates needing reflection to discover the hardware fields of a `Bundle`
+  *
+  * @param global     the environment
+  * @param arguments  run time parameters to code
+  */
 private[plugin] class BundleComponent(val global: Global, arguments: ChiselPluginArguments)
     extends PluginComponent
     with TypingTransformers {
@@ -32,14 +41,41 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi
 
     def inferType(t: Tree): Type = localTyper.typed(t, nsc.Mode.TYPEmode).tpe
 
-    val bundleTpe = inferType(tq"chisel3.Bundle")
-    val dataTpe = inferType(tq"chisel3.Data")
+    val bundleTpe:     Type = inferType(tq"chisel3.Bundle")
+    val dataTpe:       Type = inferType(tq"chisel3.Data")
+    val ignoreSeqTpe:  Type = inferType(tq"chisel3.IgnoreSeqInBundle")
+    val seqOfDataTpe:  Type = inferType(tq"scala.collection.Seq[chisel3.Data]")
+    val someOfDataTpe: Type = inferType(tq"scala.Option[chisel3.Data]")
+    val seqMapTpe:     Type = inferType(tq"scala.collection.immutable.SeqMap[String,Any]")
 
     // Not cached because it should only be run once per class (thus once per Type)
-    def isBundle(sym: Symbol): Boolean = sym.tpe <:< bundleTpe
+    def isBundle(sym: Symbol): Boolean = { sym.tpe <:< bundleTpe }
+
+    def isIgnoreSeqInBundle(sym: Symbol): Boolean = { sym.tpe <:< ignoreSeqTpe }
+
+    def isSeqOfData(sym: Symbol): Boolean = {
+      val tpe = sym.tpe
+      tpe match {
+        case NullaryMethodType(resultType) =>
+          resultType <:< seqOfDataTpe
+        case _ =>
+          false
+      }
+    }
+
+    def isOptionOfData(symbol: Symbol): Boolean = {
+      val tpe = symbol.tpe
+      tpe match {
+        case NullaryMethodType(resultType) =>
+          resultType <:< someOfDataTpe
+        case _ =>
+          false
+      }
+    }
+    def isExactBundle(sym: Symbol): Boolean = { sym.tpe =:= bundleTpe }
 
-    val isDataCache = new mutable.HashMap[Type, Boolean]
     // Cached because this is run on every argument to every Bundle
+    val isDataCache = new mutable.HashMap[Type, Boolean]
     def isData(sym: Symbol): Boolean = isDataCache.getOrElseUpdate(sym.tpe, sym.tpe <:< dataTpe)
 
     def cloneTypeFull(tree: Tree): Tree =
@@ -63,7 +99,7 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi
           val msg = "Users cannot override _usingPlugin, it is for the compiler plugin's use only."
           global.globalError(d.pos, msg)
         case d: DefDef if isNullaryMethodNamed("cloneType", d) =>
-          val msg = "Users cannot override cloneType. Let the compiler plugin generate it."
+          val msg = "Users cannot override cloneType.  Let the compiler plugin generate it."
           global.globalError(d.pos, msg)
         case _ =>
       }
@@ -72,52 +108,136 @@ private[plugin] class BundleComponent(val global: Global, arguments: ChiselPlugi
 
     override def transform(tree: Tree): Tree = tree match {
 
-      case bundle: ClassDef if isBundle(bundle.symbol) && !bundle.mods.hasFlag(Flag.ABSTRACT) =>
+      case bundle: ClassDef if isBundle(bundle.symbol) =>
         // ==================== Generate _cloneTypeImpl ====================
         val (con, params) = getConstructorAndParams(bundle.impl.body)
         if (con.isEmpty) {
           global.reporter.warning(bundle.pos, "Unable to determine primary constructor!")
           return super.transform(tree)
         }
-        val constructor = con.get
 
+        val constructor = con.get
         val thiz = gen.mkAttributedThis(bundle.symbol)
 
         // The params have spaces after them (Scalac implementation detail)
         val paramLookup: String => Symbol = params.map(sym => sym.name.toString.trim -> sym).toMap
 
-        // Create a this.<ref> for each field matching order of constructor arguments
-        // List of Lists because we can have multiple parameter lists
-        val conArgs: List[List[Tree]] =
-          constructor.vparamss.map(_.map { vp =>
-            val p = paramLookup(vp.name.toString)
-            // Make this.<ref>
-            val select = gen.mkAttributedSelect(thiz, p)
-            // Clone any Data parameters to avoid field aliasing, need full clone to include direction
-            if (isData(vp.symbol)) cloneTypeFull(select) else select
-          })
-
-        val tparamList = bundle.tparams.map { t => Ident(t.symbol) }
-        val ttpe = if (tparamList.nonEmpty) AppliedTypeTree(Ident(bundle.symbol), tparamList) else Ident(bundle.symbol)
-        val newUntyped = New(ttpe, conArgs)
-        val neww = localTyper.typed(newUntyped)
-
-        // Create the symbol for the method and have it be associated with the Bundle class
-        val cloneTypeSym =
-          bundle.symbol.newMethod(TermName("_cloneTypeImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED)
-        // Handwritten cloneTypes don't have the Method flag set, unclear if it matters
-        cloneTypeSym.resetFlag(Flags.METHOD)
-        // Need to set the type to chisel3.Bundle for the override to work
-        cloneTypeSym.setInfo(NullaryMethodType(bundleTpe))
-
-        val cloneTypeImpl = localTyper.typed(DefDef(cloneTypeSym, neww))
+        val cloneTypeImplOpt = if (!bundle.mods.hasFlag(Flag.ABSTRACT)) {
+          // Create a this.<ref> for each field matching order of constructor arguments
+          // List of Lists because we can have multiple parameter lists
+          val conArgs: List[List[Tree]] =
+            constructor.vparamss.map(_.map { vp =>
+              val p = paramLookup(vp.name.toString)
+              // Make this.<ref>
+              val select = gen.mkAttributedSelect(thiz.asInstanceOf[Tree], p)
+              // Clone any Data parameters to avoid field aliasing, need full clone to include direction
+              if (isData(vp.symbol)) cloneTypeFull(select.asInstanceOf[Tree]) else select
+            })
+
+          val tparamList = bundle.tparams.map { t => Ident(t.symbol) }
+          val ttpe =
+            if (tparamList.nonEmpty) AppliedTypeTree(Ident(bundle.symbol), tparamList) else Ident(bundle.symbol)
+          val newUntyped = New(ttpe, conArgs)
+          val neww = localTyper.typed(newUntyped)
+
+          // Create the symbol for the method and have it be associated with the Bundle class
+          val cloneTypeSym =
+            bundle.symbol.newMethod(TermName("_cloneTypeImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED)
+          // Handwritten cloneTypes don't have the Method flag set, unclear if it matters
+          cloneTypeSym.resetFlag(Flags.METHOD)
+          // Need to set the type to chisel3.Bundle for the override to work
+          cloneTypeSym.setInfo(NullaryMethodType(bundleTpe))
+
+          Some(localTyper.typed(DefDef(cloneTypeSym, neww)))
+        } else {
+          // Don't create if this Bundle is abstract
+          None
+        }
+
+        // ==================== Generate val elements ====================
+
+        /* Test to see if the bundle found is amenable to having it's elements
+         * converted to an immediate form that will not require reflection
+         */
+        def isSupportedBundleType: Boolean = {
+          arguments.genBundleElements && !bundle.mods.hasFlag(Flag.ABSTRACT)
+        }
+
+        val elementsImplOpt = if (isSupportedBundleType) {
+          /* extract the true fields from the super classes a given bundle
+           * depth argument can be helpful for debugging
+           */
+          def getAllBundleFields(bundleSymbol: Symbol, depth: Int = 0): List[(String, Tree)] = {
+
+            def isBundleField(member: Symbol): Boolean = {
+              if (!member.isAccessor) {
+                false
+              } else if (isData(member.tpe.typeSymbol)) {
+                true
+              } else if (isOptionOfData(member)) {
+                true
+              } else if (isSeqOfData(member)) {
+                // This field is passed along, even though it is illegal
+                // An error for this will be generated in `Bundle.elements`
+                // It would be possible here to check for Seq[Data] and make a compiler error, but
+                // that would be a API error difference. See reference in docs/chisel-plugin.md
+                // If Bundle is subclass of IgnoreSeqInBundle then don't pass this field along
+
+                !isIgnoreSeqInBundle(bundleSymbol)
+              } else {
+                // none of the above
+                false
+              }
+            }
+
+            val currentFields = bundleSymbol.info.members.flatMap {
+
+              case member if member.isPublic =>
+                if (isBundleField(member)) {
+                  // The params have spaces after them (Scalac implementation detail)
+                  Some(member.name.toString.trim -> gen.mkAttributedSelect(thiz.asInstanceOf[Tree], member))
+                } else {
+                  None
+                }
+
+              case _ => None
+            }.toList
+
+            val allParentFields = bundleSymbol.parentSymbols.flatMap { parentSymbol =>
+              val fieldsFromParent = if (depth < 1 && !isExactBundle(bundleSymbol)) {
+                val foundFields = getAllBundleFields(parentSymbol, depth + 1)
+                foundFields
+              } else {
+                List()
+              }
+              fieldsFromParent
+            }
+            allParentFields ++ currentFields
+          }
+
+          val elementArgs = getAllBundleFields(bundle.symbol)
+
+          val elementsImplSym =
+            bundle.symbol.newMethod(TermName("_elementsImpl"), bundle.symbol.pos.focus, Flag.OVERRIDE | Flag.PROTECTED)
+          elementsImplSym.resetFlag(Flags.METHOD)
+          elementsImplSym.setInfo(NullaryMethodType(seqMapTpe))
+
+          val elementsImpl = localTyper.typed(
+            DefDef(elementsImplSym, q"scala.collection.immutable.SeqMap.apply[String, Any](..$elementArgs)")
+          )
+
+          Some(elementsImpl)
+        } else {
+          // No code generated for elements accessor
+          None
+        }
 
         // ==================== Generate _usingPlugin ====================
         // Unclear why quasiquotes work here but didn't for cloneTypeSym, maybe they could.
-        val usingPlugin = localTyper.typed(q"override protected def _usingPlugin: Boolean = true")
+        val usingPluginOpt = Some(localTyper.typed(q"override protected def _usingPlugin: Boolean = true"))
 
         val withMethods = deriveClassDef(bundle) { t =>
-          deriveTemplate(t)(_ :+ cloneTypeImpl :+ usingPlugin)
+          deriveTemplate(t)(_ ++ cloneTypeImplOpt ++ usingPluginOpt ++ elementsImplOpt)
         }
 
         super.transform(localTyper.typed(withMethods))
diff --git a/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala b/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala
index bd02d50c..9bf8c657 100644
--- a/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala
+++ b/plugin/src/main/scala/chisel3/internal/plugin/ChiselPlugin.scala
@@ -8,10 +8,13 @@ import nsc.plugins.{Plugin, PluginComponent}
 import scala.reflect.internal.util.NoPosition
 import scala.collection.mutable
 
-private[plugin] case class ChiselPluginArguments(val skipFiles: mutable.HashSet[String] = mutable.HashSet.empty) {
+private[plugin] case class ChiselPluginArguments(
+  val skipFiles:         mutable.HashSet[String] = mutable.HashSet.empty,
+  var genBundleElements: Boolean = false) {
   def useBundlePluginOpt = "useBundlePlugin"
   def useBundlePluginFullOpt = s"-P:${ChiselPlugin.name}:$useBundlePluginOpt"
-
+  def genBundleElementsOpt = "genBundleElements"
+  def genBundleElementsFullOpt = s"-P:${ChiselPlugin.name}:$genBundleElementsOpt"
   // Annoying because this shouldn't be used by users
   def skipFilePluginOpt = "INTERNALskipFile:"
   def skipFilePluginFullOpt = s"-P:${ChiselPlugin.name}:$skipFilePluginOpt"
@@ -20,7 +23,7 @@ private[plugin] case class ChiselPluginArguments(val skipFiles: mutable.HashSet[
 object ChiselPlugin {
   val name = "chiselplugin"
 
-  // Also logs why the compoennt was not run
+  // Also logs why the component was not run
   private[plugin] def runComponent(
     global:    Global,
     arguments: ChiselPluginArguments
@@ -67,11 +70,12 @@ class ChiselPlugin(val global: Global) extends Plugin {
         // Be annoying and warn because users are not supposed to use this
         val msg = s"Option -P:${ChiselPlugin.name}:$option should only be used for internal chisel3 compiler purposes!"
         global.reporter.warning(NoPosition, msg)
+      } else if (option == arguments.genBundleElementsOpt) {
+        arguments.genBundleElements = true
       } else {
         error(s"Option not understood: '$option'")
       }
     }
     true
   }
-
 }
diff --git a/src/test/scala/chiselTests/BundleElementsSpec.scala b/src/test/scala/chiselTests/BundleElementsSpec.scala
new file mode 100644
index 00000000..fab2e733
--- /dev/null
+++ b/src/test/scala/chiselTests/BundleElementsSpec.scala
@@ -0,0 +1,564 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package chiselTests
+
+import chisel3._
+import chisel3.experimental.{ChiselEnum, FixedPoint}
+import chisel3.stage.ChiselStage
+import chisel3.util.Decoupled
+import org.scalatest.exceptions.TestFailedException
+import org.scalatest.freespec.AnyFreeSpec
+import org.scalatest.matchers.should.Matchers
+
+import scala.language.reflectiveCalls
+
+/* Rich and complicated bundle examples
+ */
+trait BpipSuperTraitWithField {
+  val bpipSuperTraitGood = SInt(17.W)
+  def bpipSuperTraitBad = SInt(22.W)
+}
+
+trait BpipTraitWithField extends BpipSuperTraitWithField {
+  val bpipTraitGood = SInt(17.W)
+  def bpipTraitBad = SInt(22.W)
+}
+
+class BpipOneField extends Bundle with BpipTraitWithField {
+  val bpipOneFieldOne = SInt(8.W)
+  val bpipOneFieldTwo = SInt(8.W)
+}
+
+class BpipTwoField extends BpipOneField {
+  val bpipTwoFieldOne = SInt(8.W)
+  val bpipTwoFieldTwo = Vec(4, UInt(12.W))
+  val myInt = 7
+  val baz = Decoupled(UInt(16.W))
+}
+
+class BpipDecoupled extends BpipOneField {
+  val bpipDecoupledSInt = SInt(8.W)
+  val bpipDecoupledVec = Vec(4, UInt(12.W))
+  val bpipDecoupledDecoupled = Decoupled(UInt(16.W))
+}
+
+class HasDecoupledBundleByInheritance extends Module {
+  val out1 = IO(Output(new BpipDecoupled))
+  assertElementsMatchExpected(out1)(
+    "bpipDecoupledDecoupled" -> _.bpipDecoupledDecoupled,
+    "bpipDecoupledVec" -> _.bpipDecoupledVec,
+    "bpipDecoupledSInt" -> _.bpipDecoupledSInt,
+    "bpipOneFieldTwo" -> _.bpipOneFieldTwo,
+    "bpipOneFieldOne" -> _.bpipOneFieldOne,
+    "bpipTraitGood" -> _.bpipTraitGood,
+    "bpipSuperTraitGood" -> _.bpipSuperTraitGood
+  )
+}
+
+/* plugin should not affect the seq detection */
+class DebugProblem3 extends Module {
+  val out1 = IO(Output(new BpipTwoField))
+  assertElementsMatchExpected(out1)(
+    "baz" -> _.baz,
+    "bpipTwoFieldTwo" -> _.bpipTwoFieldTwo,
+    "bpipTwoFieldOne" -> _.bpipTwoFieldOne,
+    "bpipOneFieldTwo" -> _.bpipOneFieldTwo,
+    "bpipOneFieldOne" -> _.bpipOneFieldOne,
+    "bpipTraitGood" -> _.bpipTraitGood,
+    "bpipSuperTraitGood" -> _.bpipSuperTraitGood
+  )
+}
+
+class BpipBadSeqBundle extends Bundle {
+  val bpipBadSeqBundleGood = UInt(999.W)
+  val bpipBadSeqBundleBad = Seq(UInt(16.W), UInt(8.W), UInt(4.W))
+}
+
+class HasBadSeqBundle extends Module {
+  val out1 = IO(Output(new BpipBadSeqBundle))
+}
+
+class BpipBadSeqBundleWithIgnore extends Bundle with IgnoreSeqInBundle {
+  val goodFieldWithIgnore = UInt(999.W)
+  val badSeqFieldWithIgnore = Seq(UInt(16.W), UInt(8.W), UInt(4.W))
+}
+
+class UsesIgnoreSeqInBundle extends Module {
+  val out1 = IO(Output(new BpipBadSeqBundleWithIgnore))
+}
+
+/* This is mostly a test of the field order */
+class BpipP8_1 extends Bundle {
+  val field_1_1 = UInt(11.W)
+  val field_1_2 = UInt(12.W)
+}
+
+class BpipP8_2 extends BpipP8_1 {
+  val field_2_1 = UInt(11.W)
+  val field_2_2 = UInt(12.W)
+}
+
+class BpipP8_3 extends BpipP8_2 {
+  val field_3_1 = UInt(11.W)
+  val field_3_2 = UInt(12.W)
+}
+
+/* plugin should not affect the seq detection */
+class ForFieldOrderingTest extends Module {
+  val out1 = IO(Output(new BpipP8_3))
+  out1 := DontCare
+  assertElementsMatchExpected(out1)(
+    "field_3_2" -> _.field_3_2,
+    "field_3_1" -> _.field_3_1,
+    "field_2_2" -> _.field_2_2,
+    "field_2_1" -> _.field_2_1,
+    "field_1_2" -> _.field_1_2,
+    "field_1_1" -> _.field_1_1
+  )
+}
+
+/* plugin should allow parameter var fields */
+class HasValParamsToBundle extends Module {
+  // The following block does not work, suggesting that ParamIsField is not a case we need to solve
+  class BpipParamIsField0(val paramField0: UInt) extends Bundle
+  class BpipParamIsField1(val paramField1: UInt) extends BpipParamIsField0(UInt(66.W))
+
+  val out3 = IO(Output(new BpipParamIsField1(UInt(10.W))))
+  val out4 = IO(Output(new BpipParamIsField1(UInt(10.W))))
+  out3 := DontCare
+  assertElementsMatchExpected(out3)("paramField0" -> _.paramField0, "paramField1" -> _.paramField1)
+  assertElementsMatchExpected(out4)("paramField0" -> _.paramField0, "paramField1" -> _.paramField1)
+}
+
+class HasGenParamsPassedToSuperclasses extends Module {
+
+  class OtherBundle extends Bundle {
+    val otherField = UInt(55.W)
+  }
+
+  class BpipWithGen[T <: Data, TT <: Data](gen: T, gen2: => TT) extends Bundle {
+    val superFoo = gen
+    val superQux = gen2
+  }
+
+//  class BpipDemoBundle[T <: Data](gen: T, gen2: => T) extends BpipTwoField with BpipVarmint {
+  class BpipUsesWithGen[T <: Data](gen: T, gen2: => T) extends BpipWithGen(gen, gen2) {
+    //    val foo = gen
+    val bar = Bool()
+    val qux = gen2
+    val bad = 444
+    val baz = Decoupled(UInt(16.W))
+  }
+
+  val out1 = IO(Output(new BpipUsesWithGen(UInt(4.W), new OtherBundle)))
+  val out2 = IO(Output(new BpipUsesWithGen(UInt(4.W), FixedPoint(10.W, 4.BP))))
+
+  out1 := DontCare
+
+  assertElementsMatchExpected(out1)(
+    "baz" -> _.baz,
+    "qux" -> _.qux,
+    "bar" -> _.bar,
+    "superQux" -> _.superQux,
+    "superFoo" -> _.superFoo
+  )
+  assertElementsMatchExpected(out2)(
+    "baz" -> _.baz,
+    "qux" -> _.qux,
+    "bar" -> _.bar,
+    "superQux" -> _.superQux,
+    "superFoo" -> _.superFoo
+  )
+}
+
+/* Testing whether gen fields superFoo and superQux can be found when they are
+ * declared in a superclass
+ *
+ */
+class UsesGenFiedldsInSuperClass extends Module {
+  class BpipWithGen[T <: Data](gen: T) extends Bundle {
+    val superFoo = gen
+    val superQux = gen
+  }
+
+  class BpipUsesWithGen[T <: Data](gen: T) extends BpipWithGen(gen) {}
+
+  val out = IO(Output(new BpipUsesWithGen(UInt(4.W))))
+
+  out := DontCare
+
+  assertElementsMatchExpected(out)()
+}
+
+/* Testing whether gen fields superFoo and superQux can be found when they are
+ * declared in a superclass
+ *
+ */
+class BpipBadBundleWithHardware extends Bundle {
+  val bpipWithHardwareGood = UInt(8.W)
+  val bpipWithHardwareBad = 244.U(16.W)
+}
+
+class HasHardwareFieldsInBundle extends Module {
+  val out = IO(Output(new BpipBadBundleWithHardware))
+  assertElementsMatchExpected(out)()
+}
+
+/* This is legal because of =>
+ */
+class UsesBundleWithGeneratorField extends Module {
+  class BpipWithGen[T <: Data](gen: => T) extends Bundle {
+    val superFoo = gen
+    val superQux = gen
+  }
+
+  class BpipUsesWithGen[T <: Data](gen: => T) extends BpipWithGen(gen)
+
+  val out = IO(Output(new BpipUsesWithGen(UInt(4.W))))
+
+  out := DontCare
+
+  assertElementsMatchExpected(out)("superQux" -> _.superQux, "superFoo" -> _.superFoo)
+}
+
+/* Testing whether gen fields superFoo and superQux can be found when they are
+ * declared in a superclass
+ *
+ */
+class BundleElementsSpec extends AnyFreeSpec with Matchers {
+
+  /** Tests a whole bunch of different Bundle constructions
+    */
+  class BpipIsComplexBundle extends Module {
+
+    trait BpipVarmint {
+      val varmint = Bool()
+
+      def vermin = Bool()
+
+      private val puppy = Bool()
+    }
+
+    abstract class BpipAbstractBundle extends Bundle {
+      def doNothing: Unit
+
+      val fromAbstractBundle = UInt(22.W)
+    }
+
+    class BpipOneField extends Bundle {
+      val fieldOne = SInt(8.W)
+    }
+
+    class BpipTwoField extends BpipOneField {
+      val fieldTwo = SInt(8.W)
+      val fieldThree = Vec(4, UInt(12.W))
+    }
+
+    class BpipAnimalBundle(w1: Int, w2: Int) extends Bundle {
+      val dog = SInt(w1.W)
+      val fox = UInt(w2.W)
+    }
+
+    class BpipDemoBundle[T <: Data](gen: T, gen2: => T) extends BpipTwoField with BpipVarmint {
+      val foo = gen
+      val bar = Bool()
+      val qux = gen2
+      val bad = 44
+      val baz = Decoupled(UInt(16.W))
+      val animals = new BpipAnimalBundle(4, 8)
+    }
+
+    val out = IO(Output(new BpipDemoBundle(UInt(4.W), FixedPoint(10.W, 4.BP))))
+
+    val out2 = IO(Output(new BpipAbstractBundle {
+      override def doNothing: Unit = ()
+
+      val notAbstract: Bool = Bool()
+    }))
+
+    val out4 = IO(Output(new BpipAnimalBundle(99, 100)))
+    val out5 = IO(Output(new BpipTwoField))
+
+    out := DontCare
+    out5 := DontCare
+
+    assertElementsMatchExpected(out)(
+      "animals" -> _.animals,
+      "baz" -> _.baz,
+      "qux" -> _.qux,
+      "bar" -> _.bar,
+      "varmint" -> _.varmint,
+      "fieldThree" -> _.fieldThree,
+      "fieldTwo" -> _.fieldTwo,
+      "fieldOne" -> _.fieldOne,
+      "foo" -> _.foo
+    )
+    assertElementsMatchExpected(out5)("fieldThree" -> _.fieldThree, "fieldTwo" -> _.fieldTwo, "fieldOne" -> _.fieldOne)
+    assertElementsMatchExpected(out2)("notAbstract" -> _.notAbstract, "fromAbstractBundle" -> _.fromAbstractBundle)
+    assertElementsMatchExpected(out4)("fox" -> _.fox, "dog" -> _.dog)
+  }
+
+  "Complex Bundle with inheritance, traits and params. DebugProblem1" in {
+    ChiselStage.emitFirrtl(new BpipIsComplexBundle)
+  }
+
+  "Decoupled Bundle with inheritance" in {
+    ChiselStage.emitFirrtl(new HasDecoupledBundleByInheritance)
+  }
+
+  "Simple bundle inheritance. DebugProblem3" in {
+    ChiselStage.emitFirrtl(new DebugProblem3)
+  }
+
+  "Bundles containing Seq[Data] should be compile erorr. HasBadSeqBundle" in {
+    intercept[ChiselException] {
+      ChiselStage.emitFirrtl(new HasBadSeqBundle)
+    }
+  }
+
+  "IgnoreSeqInBundle allows Seq[Data] in bundle" in {
+    ChiselStage.emitFirrtl(new UsesIgnoreSeqInBundle)
+  }
+
+  "Simple field ordering test." in {
+    ChiselStage.emitFirrtl(new ForFieldOrderingTest)
+  }
+
+  "Val params to Bundle should be an Exception." in {
+    ChiselStage.emitFirrtl(new HasValParamsToBundle)
+  }
+
+  "Should handle gen params passed to superclasses" in {
+    ChiselStage.emitFirrtl(new HasGenParamsPassedToSuperclasses)
+  }
+
+  "Aliased fields should be detected and throw an exception, because gen: Data, with no =>" in {
+    intercept[AliasedAggregateFieldException] {
+      ChiselStage.emitFirrtl(new UsesGenFiedldsInSuperClass)
+    }
+  }
+
+  "Error when bundle fields are hardware, such as literal values. HasHardwareFieldsInBundle" in {
+    val e = intercept[ExpectedChiselTypeException] {
+      ChiselStage.emitFirrtl(new HasHardwareFieldsInBundle)
+    }
+    e.getMessage should include(
+      "Bundle: BpipBadBundleWithHardware contains hardware fields: bpipWithHardwareBad: UInt<16>(244)"
+    )
+  }
+
+  "Aliased fields not created when using gen: => Data" in {
+    ChiselStage.emitFirrtl(new UsesBundleWithGeneratorField)
+  }
+
+  class OptionBundle(val hasIn: Boolean) extends Bundle {
+    val in = if (hasIn) {
+      Some(Input(Bool()))
+    } else {
+      None
+    }
+    val out = Output(Bool())
+  }
+
+  class UsesBundleWithOptionFields extends Module {
+    val outTrue = IO(Output(new OptionBundle(hasIn = true)))
+    val outFalse = IO(Output(new OptionBundle(hasIn = false)))
+    //NOTE: The _.in.get _.in is an optional field
+    assertElementsMatchExpected(outTrue)("out" -> _.out, "in" -> _.in.get)
+    assertElementsMatchExpected(outFalse)("out" -> _.out)
+  }
+
+  "plugin should work with Bundles with Option fields" in {
+    ChiselStage.emitFirrtl(new UsesBundleWithOptionFields)
+  }
+
+  "plugin should work with enums in bundles" in {
+    object Enum0 extends ChiselEnum {
+      val s0, s1, s2 = Value
+    }
+
+    ChiselStage.emitFirrtl(new Module {
+      val out = IO(Output(new Bundle {
+        val a = UInt(8.W)
+        val b = Bool()
+        val c = Enum0.Type
+      }))
+      assertElementsMatchExpected(out)("b" -> _.b, "a" -> _.a)
+    })
+  }
+
+  "plugin will NOT see fields that are Data but declared in some way as Any" in {
+    //This is not incompatible with chisel not using the plugin, but this code is considered bad practice
+
+    ChiselStage.emitFirrtl(new Module {
+      val out = IO(Output(new Bundle {
+        val a = UInt(8.W)
+        val b: Any = Bool()
+      }))
+
+      intercept[TestFailedException] {
+        assert(out.elements.keys.exists(_ == "b"))
+      }
+    })
+  }
+
+  "plugin tests should fail properly in the following cases" - {
+
+    class BundleAB extends Bundle {
+      val a = Output(UInt(8.W))
+      val b = Output(Bool())
+    }
+
+    def checkAssertion(checks: (BundleAB => (String, Data))*)(expectedMessage: String): Unit = {
+      intercept[AssertionError] {
+        ChiselStage.emitFirrtl(new Module {
+          val out = IO(new BundleAB)
+          assertElementsMatchExpected(out)(checks: _*)
+        })
+      }.getMessage should include(expectedMessage)
+    }
+
+    "one of the expected data values is wrong" in {
+      checkAssertion("b" -> _.b, "a" -> _.b)("field 'a' data field BundleElementsSpec_Anon.out.a")
+    }
+
+    "one of the expected field names in wrong" in {
+      checkAssertion("b" -> _.b, "z" -> _.a)("field: 'a' did not match expected 'z'")
+    }
+
+    "fields that are expected are not returned by the elements method" in {
+      checkAssertion("b" -> _.b, "a" -> _.a, "c" -> _.a)("#elements is missing the 'c' field")
+    }
+
+    "fields returned by the element are not specified in the expected fields" in {
+      checkAssertion("b" -> _.b)("expected fields did not include 'a' field found in #elements")
+    }
+
+    "multiple errors between elements method and expected fields are shown in the assertion error message" in {
+      checkAssertion()(
+        "expected fields did not include 'b' field found in #elements," +
+          " expected fields did not include 'a' field found in #elements"
+      )
+    }
+  }
+
+  "plugin should error correctly when bundles contain only a Option field" in {
+    ChiselStage.emitFirrtl(new Module {
+      val io = IO(new Bundle {
+        val foo = Input(UInt(8.W))
+        val x = new Bundle {
+          val y = if (false) Some(Input(UInt(8.W))) else None
+        }
+      })
+      assertElementsMatchExpected(io)("x" -> _.x, "foo" -> _.foo)
+      assertElementsMatchExpected(io.x)()
+    })
+  }
+
+  "plugin should handle fields using the boolean to option construct" in {
+    case class ALUConfig(
+      xLen: Int,
+      mul:  Boolean,
+      b:    Boolean)
+
+    class OptionalBundle extends Bundle {
+      val optionBundleA = Input(UInt(3.W))
+      val optionBundleB = Input(UInt(7.W))
+    }
+
+    class ALU(c: ALUConfig) extends Module {
+
+      class BpipOptionBundle extends Bundle with IgnoreSeqInBundle {
+        val bpipUIntVal = Input(UInt(8.W))
+        lazy val bpipUIntLazyVal = Input(UInt(8.W))
+        var bpipUIntVar = Input(UInt(8.W))
+
+        def bpipUIntDef = Input(UInt(8.W))
+
+        val bpipOptionUInt = Some(Input(UInt(16.W)))
+        val bpipOptionOfBundle = if (c.b) Some(new OptionalBundle) else None
+        val bpipSeqData = Seq(UInt(8.W), UInt(8.W))
+      }
+
+      val io = IO(new BpipOptionBundle)
+      assertElementsMatchExpected(io)(
+        "bpipUIntLazyVal" -> _.bpipUIntLazyVal,
+        "bpipOptionUInt" -> _.bpipOptionUInt.get,
+        "bpipUIntVar" -> _.bpipUIntVar,
+        "bpipUIntVal" -> _.bpipUIntVal
+      )
+    }
+
+    ChiselStage.emitFirrtl(new ALU(ALUConfig(10, mul = true, b = false)))
+  }
+
+  "TraceSpec test, different version found in TraceSpec.scala" in {
+    class Bundle0 extends Bundle {
+      val a = UInt(8.W)
+      val b = Bool()
+      val c = Enum0.Type
+    }
+
+    class Bundle1 extends Bundle {
+      val a = new Bundle0
+      val b = Vec(4, Vec(4, Bool()))
+    }
+
+    class Module0 extends Module {
+      val i = IO(Input(new Bundle1))
+      val o = IO(Output(new Bundle1))
+      val r = Reg(new Bundle1)
+      o := r
+      r := i
+
+      assertElementsMatchExpected(i)("b" -> _.b, "a" -> _.a)
+      assertElementsMatchExpected(o)("b" -> _.b, "a" -> _.a)
+      assertElementsMatchExpected(r)("b" -> _.b, "a" -> _.a)
+    }
+
+    class Module1 extends Module {
+      val i = IO(Input(new Bundle1))
+      val m0 = Module(new Module0)
+      m0.i := i
+      m0.o := DontCare
+      assertElementsMatchExpected(i)("b" -> _.b, "a" -> _.a)
+    }
+
+    object Enum0 extends ChiselEnum {
+      val s0, s1, s2 = Value
+    }
+
+    ChiselStage.emitFirrtl(new Module1)
+  }
+}
+/* Checks that the elements method of a bundle matches the testers idea of what the bundle field names and their
+ * associated data match and that they have the same number of fields in the same order
+ */
+object assertElementsMatchExpected {
+  def apply[T <: Bundle](bun: T)(checks: (T => (String, Data))*): Unit = {
+    val expected = checks.map { fn => fn(bun) }
+    val elements = bun.elements
+    val missingMsg = "missing field in #elements"
+    val extraMsg = "extra field in #elements"
+    val paired = elements.toSeq.zipAll(expected, missingMsg -> UInt(1.W), extraMsg -> UInt(1.W))
+    val errorsStrings = paired.flatMap {
+      case (element, expected) =>
+        val (elementName, elementData) = element
+        val (expectedName, expectedData) = expected
+        if (elementName == missingMsg) {
+          Some(s"#elements is missing the '$expectedName' field")
+        } else if (expectedName == extraMsg) {
+          Some(s"expected fields did not include '$elementName' field found in #elements")
+        } else if (elementName != expectedName) {
+          Some(s"field: '$elementName' did not match expected '$expectedName'")
+        } else if (elementData != expectedData) {
+          Some(
+            s"field '$elementName' data field ${elementData}(${elementData.hashCode}) did not match expected $expectedData(${expectedData.hashCode})"
+          )
+        } else {
+          None
+        }
+    }
+    assert(errorsStrings.isEmpty, s"Bundle: ${bun.getClass.getName}: " + errorsStrings.mkString(", "))
+  }
+}
diff --git a/src/test/scala/chiselTests/BundleSpec.scala b/src/test/scala/chiselTests/BundleSpec.scala
index 720f877f..5dcbbefa 100644
--- a/src/test/scala/chiselTests/BundleSpec.scala
+++ b/src/test/scala/chiselTests/BundleSpec.scala
@@ -26,6 +26,10 @@ trait BundleSpecUtils {
     val bar = Seq(UInt(16.W), UInt(8.W), UInt(4.W))
   }
 
+  class BadSeqBundleWithIgnoreSeqInBundle extends Bundle with IgnoreSeqInBundle {
+    val bar = Seq(UInt(16.W), UInt(8.W), UInt(4.W))
+  }
+
   class MyModule(output: Bundle, input: Bundle) extends Module {
     val io = IO(new Bundle {
       val in = Input(input)
@@ -87,7 +91,7 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils {
       new BasicTester {
         val m = Module(new Module {
           val io = IO(new Bundle {
-            val b = new BadSeqBundle with IgnoreSeqInBundle
+            val b = new BadSeqBundleWithIgnoreSeqInBundle
           })
         })
         stop()
@@ -141,7 +145,7 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils {
           out := in
         }
       }
-    }).getMessage should include("must be a Chisel type, not hardware")
+    }).getMessage should include("MyBundle contains hardware fields: foo: UInt<7>(123)")
   }
   "Bundles" should "not recursively contain aggregates with bound hardware" in {
     (the[ChiselException] thrownBy extractCause[ChiselException] {
@@ -153,7 +157,7 @@ class BundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils {
           out := in
         }
       }
-    }).getMessage should include("must be a Chisel type, not hardware")
+    }).getMessage should include("Bundle: MyBundle contains hardware fields: foo: BundleSpec_Anon.out")
   }
   "Unbound bundles sharing a field" should "not error" in {
     ChiselStage.elaborate {