Inlining uses "_", respects namespaces

Summary of changes:
  - Use "_" as an inlining delimiter instead of "$"
  - Makes inlining avoid namespace conflicts

This changes the delimiter used for inlining to "_" instead of "$". This
avoids problems with buggy parsers that may not handle "$" correctly. As
ClockListTransform relies on the explicit use of "$", the delimiter is a
FIRRTL-private val that the ClockListTransform overrides (to the original
"$").

Namespace conflicts could occur previously, but are very rare as users
will almost never use "$" in a name (even though it's allowed by both the
FIRRTL and Verilog specifications). Moving to "_" increases the
probability of namespace conflicts occurring. This adds explicit checking
that inlined names will not introduce namespace conflicts and that
generated names are prefix unique (as defined in the spec).

Note: inlined modules may not have unique prefixes. A test is included
that this is the case and an ignored test shows what prefix uniqueness
would look like.

MISC:
  - [skip chisel tests]: Changing the delimiter causes the Chisel
    InlineSpec to fail as this explicitly checks for "$".

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>

2 files changed, 159 insertions, 66 deletions

diff --git a/src/test/scala/firrtlTests/FlattenTests.scala b/src/test/scala/firrtlTests/FlattenTests.scala
index 570d03bf..77a221be 100644
--- a/src/test/scala/firrtlTests/FlattenTests.scala
+++ b/src/test/scala/firrtlTests/FlattenTests.scala
@@ -44,11 +44,11 @@ class FlattenTests extends LowTransformSpec {
           |  module Top :
           |    input a : UInt<32>
           |    output b : UInt<32>
-          |    wire i$a : UInt<32>
-          |    wire i$b : UInt<32>
-          |    i$b <= i$a
-          |    b <= i$b
-          |    i$a <= a
+          |    wire i_a : UInt<32>
+          |    wire i_b : UInt<32>
+          |    i_b <= i_a
+          |    b <= i_b
+          |    i_a <= a
           |  module Inline1 :
           |    input a : UInt<32>
           |    output b : UInt<32>
@@ -77,16 +77,16 @@ class FlattenTests extends LowTransformSpec {
           |  module Top :
           |    input a : UInt<32>
           |    output b : UInt<32>
-          |    wire i1$a : UInt<32>
-          |    wire i1$b : UInt<32>
-          |    i1$b <= i1$a
-          |    wire i2$a : UInt<32>
-          |    wire i2$b : UInt<32>
-          |    i2$b <= i2$a
-          |    node tmp = i1$b
-          |    b <= i2$b
-          |    i1$a <= a
-          |    i2$a <= tmp
+          |    wire i1_a : UInt<32>
+          |    wire i1_b : UInt<32>
+          |    i1_b <= i1_a
+          |    wire i2_a : UInt<32>
+          |    wire i2_b : UInt<32>
+          |    i2_b <= i2_a
+          |    node tmp = i1_b
+          |    b <= i2_b
+          |    i1_a <= a
+          |    i2_a <= tmp
           |  module Inline1 :
           |    input a : UInt<32>
           |    output b : UInt<32>
@@ -131,17 +131,17 @@ class FlattenTests extends LowTransformSpec {
           |    input na : UInt<32>
           |    output b : UInt<32>
           |    output nb : UInt<32>
-          |    wire i$a : UInt<32>
-          |    wire i$b : UInt<32>
-          |    wire i$i$a : UInt<32>
-          |    wire i$i$b : UInt<32>
-          |    i$i$b <= i$i$a
-          |    i$b <= i$i$a
-          |    i$i$a <= i$a
+          |    wire i_a : UInt<32>
+          |    wire i_b : UInt<32>
+          |    wire i_i_a : UInt<32>
+          |    wire i_i_b : UInt<32>
+          |    i_i_b <= i_i_a
+          |    i_b <= i_i_a
+          |    i_i_a <= i_a
           |    inst ni of NotInline1
-          |    b <= i$b
-          |    nb <= ni.b         
-          |    i$a <= a
+          |    b <= i_b
+          |    nb <= ni.b
+          |    i_a <= a
           |    ni.a <= na
           |  module NotInline1 :
           |    input a : UInt<32>
@@ -213,11 +213,11 @@ class FlattenTests extends LowTransformSpec {
           |  module Inline1 :
           |    input a : UInt<32>
           |    output b : UInt<32>
-          |    wire i$a : UInt<32>
-          |    wire i$b : UInt<32>
-          |    i$b <= i$a 
-          |    b <= i$a 
-          |    i$a <= a
+          |    wire i_a : UInt<32>
+          |    wire i_b : UInt<32>
+          |    i_b <= i_a
+          |    b <= i_a
+          |    i_a <= a
           |  module Inline2 :
           |    input a : UInt<32>
           |    output b : UInt<32>
diff --git a/src/test/scala/firrtlTests/InlineInstancesTests.scala b/src/test/scala/firrtlTests/InlineInstancesTests.scala
index 4398df48..6d386d48 100644
--- a/src/test/scala/firrtlTests/InlineInstancesTests.scala
+++ b/src/test/scala/firrtlTests/InlineInstancesTests.scala
@@ -46,11 +46,11 @@ class InlineInstancesTests extends LowTransformSpec {
            |  module Top :
            |    input a : UInt<32>
            |    output b : UInt<32>
-           |    wire i$a : UInt<32>
-           |    wire i$b : UInt<32>
-           |    i$b <= i$a
-           |    b <= i$b
-           |    i$a <= a""".stripMargin
+           |    wire i_a : UInt<32>
+           |    wire i_b : UInt<32>
+           |    i_b <= i_a
+           |    b <= i_b
+           |    i_a <= a""".stripMargin
       execute(input, check, Seq(inline("Inline")))
    }
 
@@ -74,15 +74,15 @@ class InlineInstancesTests extends LowTransformSpec {
            |  module Top :
            |    input a : UInt<32>
            |    output b : UInt<32>
-           |    wire i0$a : UInt<32>
-           |    wire i0$b : UInt<32>
-           |    i0$b <= i0$a
-           |    wire i1$a : UInt<32>
-           |    wire i1$b : UInt<32>
-           |    i1$b <= i1$a
-           |    b <= i1$b
-           |    i0$a <= a
-           |    i1$a <= i0$b""".stripMargin
+           |    wire i0_a : UInt<32>
+           |    wire i0_b : UInt<32>
+           |    i0_b <= i0_a
+           |    wire i1_a : UInt<32>
+           |    wire i1_b : UInt<32>
+           |    i1_b <= i1_a
+           |    b <= i1_b
+           |    i0_a <= a
+           |    i1_a <= i0_b""".stripMargin
       execute(input, check, Seq(inline("Simple")))
    }
 
@@ -106,13 +106,13 @@ class InlineInstancesTests extends LowTransformSpec {
            |  module Top :
            |    input a : UInt<32>
            |    output b : UInt<32>
-           |    wire i0$a : UInt<32>
-           |    wire i0$b : UInt<32>
-           |    i0$b <= i0$a
+           |    wire i0_a : UInt<32>
+           |    wire i0_b : UInt<32>
+           |    i0_b <= i0_a
            |    inst i1 of Simple
            |    b <= i1.b
-           |    i0$a <= a
-           |    i1.a <= i0$b
+           |    i0_a <= a
+           |    i1.a <= i0_b
            |  module Simple :
            |    input a : UInt<32>
            |    output b : UInt<32>
@@ -146,21 +146,21 @@ class InlineInstancesTests extends LowTransformSpec {
            |  module Top :
            |    input a : UInt<32>
            |    output b : UInt<32>
-           |    wire i0$a : UInt<32>
-           |    wire i0$b : UInt<32>
-           |    i0$b <= i0$a
+           |    wire i0_a : UInt<32>
+           |    wire i0_b : UInt<32>
+           |    i0_b <= i0_a
            |    inst i1 of B
            |    b <= i1.b
-           |    i0$a <= a
-           |    i1.a <= i0$b
+           |    i0_a <= a
+           |    i1.a <= i0_b
            |  module B :
            |    input a : UInt<32>
            |    output b : UInt<32>
-           |    wire i$a : UInt<32>
-           |    wire i$b : UInt<32>
-           |    i$b <= i$a
-           |    b <= i$b
-           |    i$a <= a""".stripMargin
+           |    wire i_a : UInt<32>
+           |    wire i_b : UInt<32>
+           |    i_b <= i_a
+           |    b <= i_b
+           |    i_a <= a""".stripMargin
       execute(input, check, Seq(inline("A")))
    }
 
@@ -188,13 +188,13 @@ class InlineInstancesTests extends LowTransformSpec {
            |  module Top :
            |    input a : UInt<32>
            |    output b : UInt<32>
-           |    wire i$a : UInt<32>
-           |    wire i$b : UInt<32>
-           |    inst i$i of B
-           |    i$b <= i$i.b
-           |    i$i.a <= i$a
-           |    b <= i$b
-           |    i$a <= a
+           |    wire i_a : UInt<32>
+           |    wire i_b : UInt<32>
+           |    inst i_i of B
+           |    i_b <= i_i.b
+           |    i_i.a <= i_a
+           |    b <= i_b
+           |    i_a <= a
            |  module B :
            |    input a : UInt<32>
            |    output b : UInt<32>
@@ -202,6 +202,99 @@ class InlineInstancesTests extends LowTransformSpec {
       execute(input, check, Seq(inline("A")))
    }
 
+  "A module with nested inlines" should "still prepend prefixes" in {
+    val input =
+      """|circuit Top:
+         |  module Top:
+         |    inst foo of Foo
+         |  module Foo:
+         |    inst bar of Bar
+         |    inst baz of Bar
+         |    node foo = UInt<1>("h0")
+         |  module Bar:
+         |    node bar = UInt<1>("h0")
+         |""".stripMargin
+    val check =
+      """|circuit Top:
+         |  module Top:
+         |    node foo_bar_bar = UInt<1>("h0")
+         |    inst foo_baz of Bar
+         |    node foo_foo = UInt<1>("h0")
+         |  module Bar:
+         |    node bar = UInt<1>("h0")
+         |""".stripMargin
+    execute(input, check, Seq(inline("Foo"), inline("Foo.bar")))
+  }
+
+  "An inlined module" should "NOT be prefix unique" in {
+    val input =
+      """|circuit Top:
+         |  module Top:
+         |    inst a of A
+         |    node a_foo = UInt<1>("h0")
+         |    node a__bar = UInt<1>("h0")
+         |  module A:
+         |    node bar = UInt<1>("h0")
+         |""".stripMargin
+    val check =
+      """|circuit Top:
+         |  module Top:
+         |    node a_bar = UInt<1>("h0")
+         |    node a_foo = UInt<1>("h0")
+         |    node a__bar = UInt<1>("h0")
+         |""".stripMargin
+    execute(input, check, Seq(inline("A")))
+  }
+
+  /* This test is mutually exclusive with the above */
+  ignore should "be prefix unique" in {
+    val input =
+      """|circuit Top:
+         |  module Top:
+         |    inst a of A
+         |    node a_foo = UInt<1>("h0")
+         |    node a__bar = UInt<1>("h0")
+         |  module A:
+         |    node bar = UInt<1>("h0")
+         |""".stripMargin
+    val check =
+      """|circuit Top:
+         |  module Top:
+         |    node a___bar = UInt<1>("h0")
+         |    node a_foo = UInt<1>("h0")
+         |    node a__bar = UInt<1>("h0")
+         |""".stripMargin
+    execute(input, check, Seq(inline("A")))
+  }
+
+  it should "uniquify sanely" in {
+    val input =
+      """|circuit Top:
+         |  module Top:
+         |    inst foo of Foo
+         |    node foo_ = UInt<1>("h0")
+         |    node foo__bar = UInt<1>("h0")
+         |  module Foo:
+         |    inst bar of Bar
+         |    inst baz of Bar
+         |    node foo = UInt<1>("h0")
+         |  module Bar:
+         |    node bar = UInt<1>("h0")
+         |""".stripMargin
+    val check =
+      """|circuit Top:
+         |  module Top:
+         |    node foo__bar_bar = UInt<1>("h0")
+         |    inst foo__baz of Bar
+         |    node foo__foo = UInt<1>("h0")
+         |    node foo_ = UInt<1>("h0")
+         |    node foo__bar = UInt<1>("h0")
+         |  module Bar:
+         |    node bar = UInt<1>("h0")
+         |""".stripMargin
+    execute(input, check, Seq(inline("Foo"), inline("Foo.bar")))
+  }
+
    // ---- Errors ----
    // 1) ext module
    "External module" should "not be inlined" in {