From 53b620478ddab1faa96512e473fa198f7f1fcf50 Mon Sep 17 00:00:00 2001
From: Jack Koenig
Date: Tue, 9 Feb 2021 17:12:37 -0800
Subject: Add no-plugin-tests for testing Chisel without the compiler plugin

This is a new SBT build unit that symlinks in some files from the normal
chisel project tests, but builds them without the compiler plugin.
---
 .github/workflows/test.yml                         |  2 +-
 build.sbt                                          |  5 ++
 .../src/test/scala/chisel3/testers/TestUtils.scala |  1 +
 .../src/test/scala/chiselTests/ChiselSpec.scala    |  1 +
 .../test/scala/chiselTests/InstanceNameSpec.scala  |  1 +
 src/test/scala/chiselTests/ChiselSpec.scala        | 56 +-------------------
 .../chiselTests/ChiselTestUtilitiesSpec.scala      | 60 ++++++++++++++++++++++
 7 files changed, 70 insertions(+), 56 deletions(-)
 create mode 120000 no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala
 create mode 120000 no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala
 create mode 120000 no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala
 create mode 100644 src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index a9322492..6d9fe475 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -35,7 +35,7 @@ jobs:
         if: matrix.scala == '2.12.13'
         run: sbt ++${{ matrix.scala }} docs/mdoc
       - name: Test
-        run: sbt ++${{ matrix.scala }} test
+        run: sbt ++${{ matrix.scala }} test noPluginTests/test
       - name: Binary compatibility
         run: sbt ++${{ matrix.scala }} mimaReportBinaryIssues
 
diff --git a/build.sbt b/build.sbt
index fc19d2ad..5c051ff5 100644
--- a/build.sbt
+++ b/build.sbt
@@ -201,6 +201,11 @@ lazy val chisel = (project in file(".")).
     )
   )
 
+lazy val noPluginTests = (project in file ("no-plugin-tests")).
+  dependsOn(chisel).
+  settings(commonSettings: _*).
+  settings(chiselSettings: _*)
+
 lazy val docs = project       // new documentation project
   .in(file("docs-target")) // important: it must not be docs/
   .dependsOn(chisel)
diff --git a/no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala b/no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala
new file mode 120000
index 00000000..f3ebcbce
--- /dev/null
+++ b/no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala
@@ -0,0 +1 @@
+../../../../../../src/test/scala/chisel3/testers/TestUtils.scala
\ No newline at end of file
diff --git a/no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala
new file mode 120000
index 00000000..9b257eeb
--- /dev/null
+++ b/no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala
@@ -0,0 +1 @@
+../../../../../src/test/scala/chiselTests/ChiselSpec.scala
\ No newline at end of file
diff --git a/no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala
new file mode 120000
index 00000000..e8eca65f
--- /dev/null
+++ b/no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala
@@ -0,0 +1 @@
+../../../../../src/test/scala/chiselTests/InstanceNameSpec.scala
\ No newline at end of file
diff --git a/src/test/scala/chiselTests/ChiselSpec.scala b/src/test/scala/chiselTests/ChiselSpec.scala
index 843b3192..8df680d6 100644
--- a/src/test/scala/chiselTests/ChiselSpec.scala
+++ b/src/test/scala/chiselTests/ChiselSpec.scala
@@ -90,62 +90,8 @@ trait ChiselRunners extends Assertions with BackendCompilationUtilities {
 /** Spec base class for BDD-style testers. */
 abstract class ChiselFlatSpec extends AnyFlatSpec with ChiselRunners with Matchers
 
-class ChiselTestUtilitiesSpec extends ChiselFlatSpec {
-  import org.scalatest.exceptions.TestFailedException
-  // Who tests the testers?
-  "assertKnownWidth" should "error when the expected width is wrong" in {
-    val caught = intercept[ChiselException] {
-      assertKnownWidth(7) {
-        Wire(UInt(8.W))
-      }
-    }
-    assert(caught.getCause.isInstanceOf[TestFailedException])
-  }
-
-  it should "error when the width is unknown" in {
-    a [ChiselException] shouldBe thrownBy {
-      assertKnownWidth(7) {
-        Wire(UInt())
-      }
-    }
-  }
-
-  it should "work if the width is correct" in {
-    assertKnownWidth(8) {
-      Wire(UInt(8.W))
-    }
-  }
-
-  "assertInferredWidth" should "error if the width is known" in {
-    val caught = intercept[ChiselException] {
-      assertInferredWidth(8) {
-        Wire(UInt(8.W))
-      }
-    }
-    assert(caught.getCause.isInstanceOf[TestFailedException])
-  }
-
-  it should "error if the expected width is wrong" in {
-    a [TestFailedException] shouldBe thrownBy {
-      assertInferredWidth(8) {
-        val w = Wire(UInt())
-        w := 2.U(2.W)
-        w
-      }
-    }
-  }
-
-  it should "pass if the width is correct" in {
-    assertInferredWidth(4) {
-      val w = Wire(UInt())
-      w := 2.U(4.W)
-      w
-    }
-  }
-}
-
 /** Spec base class for property-based testers. */
-class ChiselPropSpec extends PropSpec with ChiselRunners with ScalaCheckPropertyChecks with Matchers {
+abstract class ChiselPropSpec extends PropSpec with ChiselRunners with ScalaCheckPropertyChecks with Matchers {
 
   // Constrain the default number of instances generated for every use of forAll.
   implicit override val generatorDrivenConfig: PropertyCheckConfiguration =
diff --git a/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala b/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala
new file mode 100644
index 00000000..6d3d526c
--- /dev/null
+++ b/src/test/scala/chiselTests/ChiselTestUtilitiesSpec.scala
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package chiselTests
+
+import chisel3._
+import org.scalatest.exceptions.TestFailedException
+
+class ChiselTestUtilitiesSpec extends ChiselFlatSpec {
+  // Who tests the testers?
+  "assertKnownWidth" should "error when the expected width is wrong" in {
+    val caught = intercept[ChiselException] {
+      assertKnownWidth(7) {
+        Wire(UInt(8.W))
+      }
+    }
+    assert(caught.getCause.isInstanceOf[TestFailedException])
+  }
+
+  it should "error when the width is unknown" in {
+    a [ChiselException] shouldBe thrownBy {
+      assertKnownWidth(7) {
+        Wire(UInt())
+      }
+    }
+  }
+
+  it should "work if the width is correct" in {
+    assertKnownWidth(8) {
+      Wire(UInt(8.W))
+    }
+  }
+
+  "assertInferredWidth" should "error if the width is known" in {
+    val caught = intercept[ChiselException] {
+      assertInferredWidth(8) {
+        Wire(UInt(8.W))
+      }
+    }
+    assert(caught.getCause.isInstanceOf[TestFailedException])
+  }
+
+  it should "error if the expected width is wrong" in {
+    a [TestFailedException] shouldBe thrownBy {
+      assertInferredWidth(8) {
+        val w = Wire(UInt())
+        w := 2.U(2.W)
+        w
+      }
+    }
+  }
+
+  it should "pass if the width is correct" in {
+    assertInferredWidth(4) {
+      val w = Wire(UInt())
+      w := 2.U(4.W)
+      w
+    }
+  }
+}
+
-- 
cgit v1.2.3


From 0a0d7c6aac4326f2127d6d95efa5a4e10c81946c Mon Sep 17 00:00:00 2001
From: Jack Koenig
Date: Mon, 12 Oct 2020 21:02:27 -0700
Subject: Make it possible to GC Data instances

No longer create a pointer from parent to every HasId, only do it by
default for BaseModules and MemBases. Add pointer from parent to Data
upon binding the Data.

* Add MemTypeBinding for port types of Mems

This binding is similar to the SampleElementBinding for Vecs in that
these Data are not truly hardware, but are represented in the FIRRTL IR
and thus need some representation.

* Call _onModuleClose on unbound Records

This maintains some corner-case behavior that is nevertheless relied
upon. It ensures that refs are set for the elements of Records, even if
they are not bound to any real hardware.
---
 core/src/main/scala/chisel3/Aggregate.scala           |  6 ++++--
 core/src/main/scala/chisel3/BlackBox.scala            |  2 ++
 core/src/main/scala/chisel3/Data.scala                |  5 +++--
 core/src/main/scala/chisel3/Element.scala             |  3 ++-
 core/src/main/scala/chisel3/Mem.scala                 |  4 ++++
 core/src/main/scala/chisel3/Module.scala              | 13 +++++++++++++
 core/src/main/scala/chisel3/RawModule.scala           |  2 ++
 core/src/main/scala/chisel3/experimental/Analog.scala |  3 ++-
 core/src/main/scala/chisel3/internal/Binding.scala    |  4 ++++
 core/src/main/scala/chisel3/internal/Builder.scala    |  1 -
 src/test/scala/chiselTests/CompatibilitySpec.scala    | 12 ++++++++++++
 11 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala
index 403fcdba..f07b2543 100644
--- a/core/src/main/scala/chisel3/Aggregate.scala
+++ b/core/src/main/scala/chisel3/Aggregate.scala
@@ -20,7 +20,8 @@ class AliasedAggregateFieldException(message: String) extends ChiselException(me
   * of) other Data objects.
   */
 sealed abstract class Aggregate extends Data {
-  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) {
+  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
+    _parent.foreach(_.addId(this))
     binding = target
 
     val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
@@ -166,7 +167,8 @@ sealed class Vec[T <: Data] private[chisel3] (gen: => T, val length: Int)
     case _ => false
   }
 
-  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) {
+  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
+    _parent.foreach(_.addId(this))
     binding = target
 
     val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
diff --git a/core/src/main/scala/chisel3/BlackBox.scala b/core/src/main/scala/chisel3/BlackBox.scala
index 03543790..8ba4b612 100644
--- a/core/src/main/scala/chisel3/BlackBox.scala
+++ b/core/src/main/scala/chisel3/BlackBox.scala
@@ -81,6 +81,8 @@ package experimental {
         id._onModuleClose
       }
 
+      closeUnboundIds(names)
+
       val firrtlPorts = getModulePorts map {port => Port(port, port.specifiedDirection)}
       val component = DefBlackBox(this, name, firrtlPorts, SpecifiedDirection.Unspecified, params)
       _component = Some(component)
diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
index 332527bf..377a94e6 100644
--- a/core/src/main/scala/chisel3/Data.scala
+++ b/core/src/main/scala/chisel3/Data.scala
@@ -339,13 +339,14 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
   private[chisel3] final def isSynthesizable: Boolean = _binding.map {
     case ChildBinding(parent) => parent.isSynthesizable
     case _: TopBinding => true
-    case _: SampleElementBinding[_] => false
+    case (_: SampleElementBinding[_] | _: MemTypeBinding[_]) => false
   }.getOrElse(false)
 
   private[chisel3] def topBindingOpt: Option[TopBinding] = _binding.flatMap {
     case ChildBinding(parent) => parent.topBindingOpt
     case bindingVal: TopBinding => Some(bindingVal)
     case SampleElementBinding(parent) => parent.topBindingOpt
+    case _: MemTypeBinding[_] => None
   }
 
   private[chisel3] def topBinding: TopBinding = topBindingOpt.get
@@ -355,7 +356,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
     * node is the top-level.
     * binding and direction are valid after this call completes.
     */
-  private[chisel3] def bind(target: Binding, parentDirection: SpecifiedDirection = SpecifiedDirection.Unspecified)
+  private[chisel3] def bind(target: Binding, parentDirection: SpecifiedDirection = SpecifiedDirection.Unspecified): Unit
 
   // Both _direction and _resolvedUserDirection are saved versions of computed variables (for
   // efficiency, avoid expensive recomputation of frequent operations).
diff --git a/core/src/main/scala/chisel3/Element.scala b/core/src/main/scala/chisel3/Element.scala
index 55415f3d..0c99ff70 100644
--- a/core/src/main/scala/chisel3/Element.scala
+++ b/core/src/main/scala/chisel3/Element.scala
@@ -17,7 +17,8 @@ abstract class Element extends Data {
   def widthKnown: Boolean = width.known
   def name: String = getRef.name
 
-  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) {
+  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
+    _parent.foreach(_.addId(this))
     binding = target
     val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
     direction = ActualDirection.fromSpecified(resolvedDirection)
diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala
index a60b31ac..90525bfa 100644
--- a/core/src/main/scala/chisel3/Mem.scala
+++ b/core/src/main/scala/chisel3/Mem.scala
@@ -35,6 +35,7 @@ object Mem {
     }
     val mt  = t.cloneTypeFull
     val mem = new Mem(mt, size)
+    mt.bind(MemTypeBinding(mem))
     pushCommand(DefMemory(sourceInfo, mem, mt, size))
     mem
   }
@@ -45,6 +46,8 @@ object Mem {
 }
 
 sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) extends HasId with NamedComponent with SourceInfoDoc {
+  _parent.foreach(_.addId(this))
+
   // REVIEW TODO: make accessors (static/dynamic, read/write) combinations consistent.
 
   /** Creates a read accessor into the memory with static addressing. See the
@@ -174,6 +177,7 @@ object SyncReadMem {
     }
     val mt  = t.cloneTypeFull
     val mem = new SyncReadMem(mt, size, ruw)
+    mt.bind(MemTypeBinding(mem))
     pushCommand(DefSeqMemory(sourceInfo, mem, mt, size, ruw))
     mem
   }
diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala
index d34211f1..9f8087bf 100644
--- a/core/src/main/scala/chisel3/Module.scala
+++ b/core/src/main/scala/chisel3/Module.scala
@@ -209,6 +209,8 @@ package experimental {
     */
   // TODO: seal this?
   abstract class BaseModule extends HasId {
+    _parent.foreach(_.addId(this))
+
     //
     // Builder Internals - this tracks which Module RTL construction belongs to.
     //
@@ -382,6 +384,17 @@ package experimental {
       names
     }
 
+    /** Invokes _onModuleClose on HasIds found via reflection but not bound to hardware
+      * (thus not part of _ids)
+      * This maintains old naming behavior for non-hardware Data
+      */
+    private[chisel3] def closeUnboundIds(names: HashMap[HasId, String]): Unit = {
+      val idLookup = _ids.toSet
+      for ((id, _) <- names if !idLookup(id)) {
+        id._onModuleClose
+      }
+    }
+
     /** Compatibility function. Allows Chisel2 code which had ports without the IO wrapper to
       * compile under Bindings checks. Does nothing in non-compatibility mode.
       *
diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala
index 0adacedb..d2ba6e84 100644
--- a/core/src/main/scala/chisel3/RawModule.scala
+++ b/core/src/main/scala/chisel3/RawModule.scala
@@ -98,6 +98,8 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions)
       id._onModuleClose
     }
 
+    closeUnboundIds(names)
+
     val firrtlPorts = getModulePorts map { port: Data =>
       // Special case Vec to make FIRRTL emit the direction of its
       // element.
diff --git a/core/src/main/scala/chisel3/experimental/Analog.scala b/core/src/main/scala/chisel3/experimental/Analog.scala
index df76fd70..2ce2c86d 100644
--- a/core/src/main/scala/chisel3/experimental/Analog.scala
+++ b/core/src/main/scala/chisel3/experimental/Analog.scala
@@ -45,7 +45,8 @@ final class Analog private (private[chisel3] val width: Width) extends Element {
 
   // Define setter/getter pairing
   // Analog can only be bound to Ports and Wires (and Unbound)
-  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection) {
+  private[chisel3] override def bind(target: Binding, parentDirection: SpecifiedDirection): Unit = {
+    _parent.foreach(_.addId(this))
     SpecifiedDirection.fromParent(parentDirection, specifiedDirection) match {
       case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip =>
       case x => throwException(s"Analog may not have explicit direction, got '$x'")
diff --git a/core/src/main/scala/chisel3/internal/Binding.scala b/core/src/main/scala/chisel3/internal/Binding.scala
index 4442c62e..9e17aded 100644
--- a/core/src/main/scala/chisel3/internal/Binding.scala
+++ b/core/src/main/scala/chisel3/internal/Binding.scala
@@ -110,6 +110,10 @@ case class ChildBinding(parent: Data) extends Binding {
 case class SampleElementBinding[T <: Data](parent: Vec[T]) extends Binding {
   def location = parent.topBinding.location
 }
+/** Special binding for Mem types */
+case class MemTypeBinding[T <: Data](parent: MemBase[T]) extends Binding {
+  def location: Option[BaseModule] = parent._parent
+}
 // A DontCare element has a specific Binding, somewhat like a literal.
 // It is a source (RHS). It may only be connected/applied to sinks.
 case class DontCareBinding() extends UnconstrainedBinding
diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index b7772aea..31d4666c 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -84,7 +84,6 @@ trait InstanceId {
 private[chisel3] trait HasId extends InstanceId {
   private[chisel3] def _onModuleClose: Unit = {}
   private[chisel3] val _parent: Option[BaseModule] = Builder.currentModule
-  _parent.foreach(_.addId(this))
 
   private[chisel3] val _id: Long = Builder.idGen.next
 
diff --git a/src/test/scala/chiselTests/CompatibilitySpec.scala b/src/test/scala/chiselTests/CompatibilitySpec.scala
index c7a68e7c..2d4ad517 100644
--- a/src/test/scala/chiselTests/CompatibilitySpec.scala
+++ b/src/test/scala/chiselTests/CompatibilitySpec.scala
@@ -451,6 +451,18 @@ class CompatibiltySpec extends ChiselFlatSpec with ScalaCheckDrivenPropertyCheck
     ChiselStage.elaborate(new Foo)
   }
 
+  it should "support data-types of mixed directionality" in {
+    class Foo extends Module {
+      val io = IO(new Bundle {})
+      val tpe = new Bundle { val foo = UInt(OUTPUT, width = 4); val bar = UInt(width = 4) }
+      // NOTE for some reason, the old bug this hit did not occur when `tpe` is inlined
+      val mem = SeqMem(tpe, 8)
+      mem(3.U)
+
+    }
+    ChiselStage.elaborate((new Foo))
+  }
+
   behavior of "debug"
 
   it should "still exist" in {
-- 
cgit v1.2.3