Merge branch '3.5.x' into 3.5-release

Author: jackkoenig

.github/PULL_REQUEST_TEMPLATE.md

@@ -44,7 +44,7 @@ Text from here to the end of the body will be considered for inclusion in the re
 
 ### Reviewer Checklist (only modified by reviewer)
 - [ ] Did you add the appropriate labels?
-- [ ] Did you mark the proper milestone (Bug fix: `3.3.x`, [small] API extension: `3.4.x`, API modification or big change: `3.5.0`)?
+- [ ] Did you mark the proper milestone (Bug fix: `3.4.x`, [small] API extension: `3.5.x`, API modification or big change: `3.6.0`)?
 - [ ] Did you review?
 - [ ] Did you check whether all relevant Contributor checkboxes have been checked?
 - [ ] Did you mark as `Please Merge`?

.github/workflows/test.yml

@@ -5,6 +5,7 @@ on:
   push:
     branches:
       - master
+      - 3.5.x
       - 3.4.x
       - 3.3.x
       - 3.2.x

README.md

@@ -121,10 +121,10 @@ For example, in SBT this could be expressed as:
 ```scala
 // build.sbt
 scalaVersion := "2.13.7"
-addCompilerPlugin("edu.berkeley.cs" % "chisel3-plugin" % "3.5.0-RC2" cross CrossVersion.full)
-libraryDependencies += "edu.berkeley.cs" %% "chisel3" % "3.5.0-RC2"
+addCompilerPlugin("edu.berkeley.cs" % "chisel3-plugin" % "3.5.0" cross CrossVersion.full)
+libraryDependencies += "edu.berkeley.cs" %% "chisel3" % "3.5.0"
 // We also recommend using chiseltest for writing unit tests 
-libraryDependencies += "edu.berkeley.cs" %% "chiseltest" % "0.5.0-RC2" % "test"
+libraryDependencies += "edu.berkeley.cs" %% "chiseltest" % "0.5.0" % "test"
 ```
 ### Guide For New Contributors
 If you are trying to make a contribution to this project, please read [CONTRIBUTING.md](https://github.com/Burnleydev1/chisel3/blob/recent_PR/CONTRIBUTING.md)
@@ -144,7 +144,7 @@ These simulation-based verification tools are available for Chisel:
 - [**ScalaDoc**](https://www.chisel-lang.org/api/latest/chisel3/index.html), a listing, description, and examples of the functionality exposed by Chisel
 - [**Gitter**](https://gitter.im/freechipsproject/chisel3), where you can ask questions or discuss anything Chisel
 - [**Website**](https://www.chisel-lang.org) ([source](https://github.com/freechipsproject/www.chisel-lang.org/))
-- [**Scastie (3.5.0-RC2)**](https://scastie.scala-lang.org/R6wV80vUTr2CwhvouEdidg)
+- [**Scastie (3.5.0)**](https://scastie.scala-lang.org/9ga9i2DvQymKlA5JjS1ieA)
 - [**asic-world**](http://www.asic-world.com/verilog/veritut.html) If you aren't familiar with verilog, this is a good tutorial.
 
 If you are migrating from Chisel2, see [the migration guide](https://www.chisel-lang.org/chisel3/chisel3-vs-chisel2.html).

build.sbt

@@ -1,5 +1,7 @@
 // See LICENSE for license details.
 
+import com.typesafe.tools.mima.core._
+
 enablePlugins(SiteScaladocPlugin)
 
 val defaultVersions = Map(
@@ -114,7 +116,8 @@ lazy val pluginScalaVersions = Seq(
   "2.13.4",
   "2.13.5",
   "2.13.6",
-  "2.13.7"
+  "2.13.7",
+  "2.13.8"
 )
 
 lazy val plugin = (project in file("plugin")).
@@ -134,7 +137,7 @@ lazy val plugin = (project in file("plugin")).
   ).
   settings(
     mimaPreviousArtifacts := {
-      Set()
+      Set("edu.berkeley.cs" % "chisel3-plugin" % "3.5.0" cross CrossVersion.full)
     }
   )
 
@@ -153,7 +156,7 @@ lazy val macros = (project in file("macros")).
   settings(name := "chisel3-macros").
   settings(commonSettings: _*).
   settings(publishSettings: _*).
-  settings(mimaPreviousArtifacts := Set())
+  settings(mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3-macros" % "3.5.0"))
 
 lazy val firrtlRef = ProjectRef(workspaceDirectory / "firrtl", "firrtl")
 
@@ -167,7 +170,7 @@ lazy val core = (project in file("core")).
     buildInfoKeys := Seq[BuildInfoKey](buildInfoPackage, version, scalaVersion, sbtVersion)
   ).
   settings(publishSettings: _*).
-  settings(mimaPreviousArtifacts := Set()).
+  settings(mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3-core" % "3.5.0")).
   settings(
     name := "chisel3-core",
     scalacOptions := scalacOptions.value ++ Seq(
@@ -196,8 +199,13 @@ lazy val chisel = (project in file(".")).
   dependsOn(core).
   aggregate(macros, core, plugin).
   settings(
-    mimaPreviousArtifacts := Set(),
+    mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3" % "3.5.0"),
+    mimaBinaryIssueFilters ++= Seq(
+      // Modified package private methods (https://github.com/lightbend/mima/issues/53)
+      ProblemFilters.exclude[DirectMissingMethodProblem]("chisel3.stage.ChiselOptions.this"),
+    ),
     libraryDependencies += defaultVersions("treadle") % "test",
+    Test / scalacOptions += "-P:chiselplugin:genBundleElements",
     scalacOptions in Test ++= Seq("-language:reflectiveCalls"),
     scalacOptions in Compile in doc ++= Seq(
       "-diagrams",

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",

core/src/main/scala/chisel3/Aggregate.scala

@@ -1207,7 +1207,66 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     *   assert(uint === "h12345678".U) // This will pass
     * }}}
     */
-  final lazy val elements: SeqMap[String, Data] = {
+  final lazy val elements: SeqMap[String, Data] =
+    // _elementsImpl is a method, only call it once
+    _elementsImpl match {
+      // Those not using plugin-generated _elementsImpl use the old reflective implementation
+      case oldElements: VectorMap[_, _] => oldElements.asInstanceOf[VectorMap[String, Data]]
+      // Plugin-generated _elementsImpl are incomplete and need some processing
+      case rawElements => _processRawElements(rawElements)
+    }
+
+  // The compiler plugin is imperfect at picking out elements statically so we process at runtime
+  // checking for errors and filtering out mistakes
+  private def _processRawElements(rawElements: Iterable[(String, Any)]): SeqMap[String, Data] = {
+    val hardwareFields = rawElements.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(rawElements.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))
+    }: _*)
+  }
+
+  /* The old, reflective implementation of Bundle.elements
+   * This method is optionally overwritten by the compiler plugin for much better performance
+   */
+  protected def _elementsImpl: Iterable[(String, Any)] = {
     val nameMap = LinkedHashMap[String, Data]()
     for (m <- getPublicFields(classOf[Bundle])) {
       getBundleField(m) match {

core/src/main/scala/chisel3/BlackBox.scala

@@ -145,27 +145,30 @@ abstract class BlackBox(
     extends BaseBlackBox {
 
   // Find a Record port named "io" for purposes of stripping the prefix
-  private[chisel3] lazy val _io: Record =
+  private[chisel3] lazy val _io: Option[Record] =
     this
       .findPort("io")
       .collect { case r: Record => r } // Must be a Record
-      .getOrElse(null) // null handling occurs in generateComponent
 
   // Allow access to bindings from the compatibility package
-  protected def _compatIoPortBound() = portsContains(_io)
+  protected def _compatIoPortBound() = _io.exists(portsContains(_))
 
   private[chisel3] override def generateComponent(): Option[Component] = {
     _compatAutoWrapPorts() // pre-IO(...) compatibility hack
 
     // Restrict IO to just io, clock, and reset
-    require(_io != null, "BlackBox must have a port named 'io' of type Record!")
-    require(portsContains(_io), "BlackBox must have io wrapped in IO(...)")
+    if (!_io.forall(portsContains)) {
+      throwException(s"BlackBox '$this' must have a port named 'io' of type Record wrapped in IO(...)!")
+    }
+
     require(portsSize == 1, "BlackBox must only have one IO, called `io`")
 
     require(!_closed, "Can't generate module more than once")
     _closed = true
 
-    val namedPorts = _io.elements.toSeq.reverse // ListMaps are stored in reverse order
+    val io = _io.get
+
+    val namedPorts = io.elements.toSeq.reverse // ListMaps are stored in reverse order
 
     // There is a risk of user improperly attempting to connect directly with io
     // Long term solution will be to define BlackBox IO differently as part of
@@ -181,18 +184,18 @@ abstract class BlackBox(
     // of the io bundle, but NOT on the io bundle itself.
     // Doing so would cause the wrong names to be assigned, since their parent
     // is now the module itself instead of the io bundle.
-    for (id <- getIds; if id ne _io) {
+    for (id <- getIds; if id ne io) {
       id._onModuleClose
     }
 
     val firrtlPorts = namedPorts.map { namedPort => Port(namedPort._2, namedPort._2.specifiedDirection) }
-    val component = DefBlackBox(this, name, firrtlPorts, _io.specifiedDirection, params)
+    val component = DefBlackBox(this, name, firrtlPorts, io.specifiedDirection, params)
     _component = Some(component)
     _component
   }
 
   private[chisel3] def initializeInParent(parentCompileOptions: CompileOptions): Unit = {
-    for ((_, port) <- _io.elements) {
+    for ((_, port) <- _io.map(_.elements).getOrElse(Nil)) {
       pushCommand(DefInvalid(UnlocatableSourceInfo, port.ref))
     }
   }

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)
   }