Empty prefixes should be ignored in emission (#5057)

Previously they would result in an extra '_' in the same of the signal.
Also delete duplicate implementation of prefix.

Author: jackkoenig

core/src/main/scala/chisel3/experimental/prefix.scala

@@ -20,8 +20,9 @@ import chisel3.internal.{Builder, HasId}
   */
 object prefix {
 
-  /** Use to add a prefix to any components generated in the provided scope
-    * The prefix is the name of the provided which, which may not be known yet.
+  /** Use to add a prefix to any components generated in the provided scope.
+    *
+    * The prefix is the name of the provided `HasId`, which may not be known yet.
     *
     * @param name The signal/instance whose name will be the prefix
     * @param f a function for which any generated components are given the prefix
@@ -37,10 +38,11 @@ object prefix {
     ret
   }
 
-  /** Use to add a prefix to any components generated in the provided scope
+  /** Use to add a prefix to any components generated in the provided scope.
+    *
     * The prefix is a string, which must be known when this function is used.
     *
-    * @param name The name which will be the prefix
+    * @param name The name which will be the prefix (and empty name is ignored)
     * @param f a function for which any generated components are given the prefix
     * @tparam T The return type of the provided function
     * @return The return value of the provided function
@@ -60,12 +62,7 @@ object prefix {
   // TODO(adkian-sifive) This is factored out because the Scala 3
   // compiler fails to diambiguate the apply overloads using the type
   // parameter
-  private[chisel3] def applyString[T](name: String)(f: => T): T = {
-    Builder.pushPrefix(name)
-    val ret = f
-    if (Builder.getPrefix.nonEmpty) Builder.popPrefix()
-    ret
-  }
+  private[chisel3] def applyString[T](name: String)(f: => T): T = apply(name)(f)
 }
 
 /** Use to eliminate any existing prefixes within the provided scope.

core/src/main/scala/chisel3/internal/package.scala

@@ -44,8 +44,11 @@ package object internal {
         builder.append('_')
       }
       prefix.foreach { p =>
-        builder.append(p)
-        builder.append('_')
+        // Don't append _ if the prefix is empty
+        if (p.nonEmpty) {
+          builder.append(p)
+          builder.append('_')
+        }
       }
       if (temp) {
         // We've moved the leading _ to the front, drop it here

src/test/scala-2/chiselTests/naming/PrefixSpec.scala

@@ -594,4 +594,46 @@ class PrefixSpec extends AnyPropSpec with Matchers with FileCheck {
     }
     ChiselStage.emitCHIRRTL(new Test) should include("wire prefixed_wire :")
   }
+
+  property("Empty prefixes should be a no-op") {
+    class MyModule extends Module {
+      prefix("") {
+        val w = Wire(UInt(3.W))
+      }
+      val x = prefix("") {
+        val y = Wire(UInt(3.W))
+        val z = Wire(UInt(3.W))
+        y
+      }
+    }
+    println(ChiselStage.emitCHIRRTL(new MyModule))
+    ChiselStage
+      .emitCHIRRTL(new MyModule)
+      .fileCheck()(
+        """|CHECK: wire w :
+           |CHECK: wire x :
+           |CHECK: wire x_z :
+           |""".stripMargin
+      )
+  }
+
+  property("skipPrefix should apply to empty prefixes") {
+    class MyModule extends Module {
+      // skipPrefix should remove the empty prefix, not x_
+      val x = prefix("") {
+        skipPrefix {
+          val y = Wire(UInt(3.W))
+          val z = Wire(UInt(3.W))
+          y
+        }
+      }
+    }
+    ChiselStage
+      .emitCHIRRTL(new MyModule)
+      .fileCheck()(
+        """|CHECK: wire x :
+           |CHECK: wire x_z :
+           |""".stripMargin
+      )
+  }
 }