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.

10 files changed, 36 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