Make Module and Bundle properly use empty namespaces

Fix default suggested name of Module instances (now based on desired name
rather than actual assigned name).
Remove parent/child relationship from Namespace.

Previously, Module and Bundle namespaces were "children" of the Module
definition namespace. This could lead to collisions that would give unexpected
names for module instances or Bundle elements. In particular, otherwise
identical modules that instantiate other identical modules in such a way that
the instance cannot be named via reflection would not be deduplicated because
the names of the instances would collide with the names of the modules in the
Builder.globalNamespace.

3 files changed, 14 insertions, 10 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
index 732bf8fc..efb30326 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
@@ -374,7 +374,10 @@ abstract class Record extends Aggregate {
 
   // NOTE: This sets up dependent references, it can be done before closing the Module
   private[chisel3] override def _onModuleClose: Unit = { // scalastyle:ignore method.name
-    val _namespace = Builder.globalNamespace.child
+    // Since Bundle names this via reflection, it is impossible for two elements to have the same
+    // identifier; however, Namespace sanitizes identifiers to make them legal for Firrtl/Verilog
+    // which can cause collisions
+    val _namespace = Namespace.empty
     for ((name, elt) <- elements) { elt.setRef(this, _namespace.name(name)) }
   }
 
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
index ca7c8abd..de13c078 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
@@ -113,7 +113,8 @@ extends HasId {
     Port(iodef)
   }
 
-  private[core] val _namespace = Builder.globalNamespace.child
+  // Fresh Namespace because in Firrtl, Modules namespaces are disjoint with the global namespace
+  private[core] val _namespace = Namespace.empty
   private[chisel3] val _commands = ArrayBuffer[Command]()
   private[core] val _ids = ArrayBuffer[HasId]()
   Builder.currentModule = Some(this)
@@ -210,7 +211,7 @@ extends HasId {
 
     // For Module instances we haven't named, suggest the name of the Module
     _ids foreach {
-      case m: Module => m.suggestName(m.name)
+      case m: Module => m.suggestName(m.desiredName)
       case _ =>
     }
 
diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
index 5d841941..d8202da6 100644
--- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
+++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
@@ -9,7 +9,7 @@ import chisel3._
 import core._
 import firrtl._
 
-private[chisel3] class Namespace(parent: Option[Namespace], keywords: Set[String]) {
+private[chisel3] class Namespace(keywords: Set[String]) {
   private val names = collection.mutable.HashMap[String, Long]()
   for (keyword <- keywords)
     names(keyword) = 1
@@ -29,9 +29,7 @@ private[chisel3] class Namespace(parent: Option[Namespace], keywords: Set[String
     if (res.isEmpty || !legalStart(res.head)) s"_$res" else res
   }
 
-  def contains(elem: String): Boolean = {
-    names.contains(elem) || parent.map(_ contains elem).getOrElse(false)
-  }
+  def contains(elem: String): Boolean = names.contains(elem)
 
   def name(elem: String): String = {
     val sanitized = sanitize(elem)
@@ -42,9 +40,11 @@ private[chisel3] class Namespace(parent: Option[Namespace], keywords: Set[String
       sanitized
     }
   }
+}
 
-  def child(kws: Set[String]): Namespace = new Namespace(Some(this), kws)
-  def child: Namespace = child(Set())
+private[chisel3] object Namespace {
+  /** Constructs an empty Namespace */
+  def empty: Namespace = new Namespace(Set.empty[String])
 }
 
 private[chisel3] class IdGen {
@@ -143,7 +143,7 @@ private[chisel3] trait HasId extends InstanceId {
 
 private[chisel3] class DynamicContext() {
   val idGen = new IdGen
-  val globalNamespace = new Namespace(None, Set())
+  val globalNamespace = Namespace.empty
   val components = ArrayBuffer[Component]()
   val annotations = ArrayBuffer[ChiselAnnotation]()
   var currentModule: Option[Module] = None