Add --warn:reflective-naming (backport #2561) (#2565)

* Factor buildName into reusable function

The new function is chisel3.internal.buildName.

(cherry picked from commit 370ca8ac68f6d888dd99e1b9e63f0371add398cf)

* Add --warn:reflective-naming

This new argument (and associated annotation) will turn on a warning
whenever reflective naming changes the name of a signal. This is
provided to help migrate from Chisel 3.5 to 3.6 since reflective naming
is removed in Chisel 3.6.

(cherry picked from commit 97afd9b9a1155fa7cd5cedf19f9e0c15fbe899ec)

Co-authored-by: Jack Koenig <koenig@sifive.com>

7 files changed, 202 insertions, 17 deletions

diff --git a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala
index ba873c23..087bdae2 100644
--- a/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala
+++ b/src/main/scala/chisel3/aop/injecting/InjectingAspect.scala
@@ -58,7 +58,8 @@ abstract class InjectorAspect[T <: RawModule, M <: RawModule](
   final def toAnnotation(modules: Iterable[M], circuit: String, moduleNames: Seq[String]): AnnotationSeq = {
     RunFirrtlTransformAnnotation(new InjectingTransform) +: modules.map { module =>
       val chiselOptions = view[ChiselOptions](annotationsInAspect)
-      val dynamicContext = new DynamicContext(annotationsInAspect, chiselOptions.throwOnFirstError)
+      val dynamicContext =
+        new DynamicContext(annotationsInAspect, chiselOptions.throwOnFirstError, chiselOptions.warnReflectiveNaming)
       // Add existing module names into the namespace. If injection logic instantiates new modules
       //  which would share the same name, they will get uniquified accordingly
       moduleNames.foreach { n =>
diff --git a/src/main/scala/chisel3/stage/ChiselAnnotations.scala b/src/main/scala/chisel3/stage/ChiselAnnotations.scala
index e046319d..eda05a7d 100644
--- a/src/main/scala/chisel3/stage/ChiselAnnotations.scala
+++ b/src/main/scala/chisel3/stage/ChiselAnnotations.scala
@@ -79,6 +79,22 @@ case object ThrowOnFirstErrorAnnotation
 
 }
 
+/** Warn when reflective naming changes names of signals */
+case object WarnReflectiveNamingAnnotation
+    extends NoTargetAnnotation
+    with ChiselOption
+    with HasShellOptions
+    with Unserializable {
+
+  val options = Seq(
+    new ShellOption[Unit](
+      longOption = "warn:reflective-naming",
+      toAnnotationSeq = _ => Seq(this),
+      helpText = "Warn when reflective naming changes the name of signals (3.6 migration)"
+    )
+  )
+}
+
 /** An [[firrtl.annotations.Annotation]] storing a function that returns a Chisel module
   * @param gen a generator function
   */
diff --git a/src/main/scala/chisel3/stage/ChiselCli.scala b/src/main/scala/chisel3/stage/ChiselCli.scala
index f38bf50c..8c5eb79a 100644
--- a/src/main/scala/chisel3/stage/ChiselCli.scala
+++ b/src/main/scala/chisel3/stage/ChiselCli.scala
@@ -10,6 +10,7 @@ trait ChiselCli { this: Shell =>
     NoRunFirrtlCompilerAnnotation,
     PrintFullStackTraceAnnotation,
     ThrowOnFirstErrorAnnotation,
+    WarnReflectiveNamingAnnotation,
     ChiselOutputFileAnnotation,
     ChiselGeneratorAnnotation
   )
diff --git a/src/main/scala/chisel3/stage/ChiselOptions.scala b/src/main/scala/chisel3/stage/ChiselOptions.scala
index 7e4305fa..a03f3d7b 100644
--- a/src/main/scala/chisel3/stage/ChiselOptions.scala
+++ b/src/main/scala/chisel3/stage/ChiselOptions.scala
@@ -5,24 +5,27 @@ package chisel3.stage
 import chisel3.internal.firrtl.Circuit
 
 class ChiselOptions private[stage] (
-  val runFirrtlCompiler:   Boolean = true,
-  val printFullStackTrace: Boolean = false,
-  val throwOnFirstError:   Boolean = false,
-  val outputFile:          Option[String] = None,
-  val chiselCircuit:       Option[Circuit] = None) {
+  val runFirrtlCompiler:    Boolean = true,
+  val printFullStackTrace:  Boolean = false,
+  val throwOnFirstError:    Boolean = false,
+  val warnReflectiveNaming: Boolean = false,
+  val outputFile:           Option[String] = None,
+  val chiselCircuit:        Option[Circuit] = None) {
 
   private[stage] def copy(
-    runFirrtlCompiler:   Boolean = runFirrtlCompiler,
-    printFullStackTrace: Boolean = printFullStackTrace,
-    throwOnFirstError:   Boolean = throwOnFirstError,
-    outputFile:          Option[String] = outputFile,
-    chiselCircuit:       Option[Circuit] = chiselCircuit
+    runFirrtlCompiler:    Boolean = runFirrtlCompiler,
+    printFullStackTrace:  Boolean = printFullStackTrace,
+    throwOnFirstError:    Boolean = throwOnFirstError,
+    warnReflectiveNaming: Boolean = warnReflectiveNaming,
+    outputFile:           Option[String] = outputFile,
+    chiselCircuit:        Option[Circuit] = chiselCircuit
   ): ChiselOptions = {
 
     new ChiselOptions(
       runFirrtlCompiler = runFirrtlCompiler,
       printFullStackTrace = printFullStackTrace,
       throwOnFirstError = throwOnFirstError,
+      warnReflectiveNaming = warnReflectiveNaming,
       outputFile = outputFile,
       chiselCircuit = chiselCircuit
     )
diff --git a/src/main/scala/chisel3/stage/package.scala b/src/main/scala/chisel3/stage/package.scala
index b1064c05..10c8c524 100644
--- a/src/main/scala/chisel3/stage/package.scala
+++ b/src/main/scala/chisel3/stage/package.scala
@@ -15,11 +15,12 @@ package object stage {
     def view(options: AnnotationSeq): ChiselOptions = options.collect { case a: ChiselOption => a }
       .foldLeft(new ChiselOptions()) { (c, x) =>
         x match {
-          case NoRunFirrtlCompilerAnnotation => c.copy(runFirrtlCompiler = false)
-          case PrintFullStackTraceAnnotation => c.copy(printFullStackTrace = true)
-          case ThrowOnFirstErrorAnnotation   => c.copy(throwOnFirstError = true)
-          case ChiselOutputFileAnnotation(f) => c.copy(outputFile = Some(f))
-          case ChiselCircuitAnnotation(a)    => c.copy(chiselCircuit = Some(a))
+          case NoRunFirrtlCompilerAnnotation  => c.copy(runFirrtlCompiler = false)
+          case PrintFullStackTraceAnnotation  => c.copy(printFullStackTrace = true)
+          case ThrowOnFirstErrorAnnotation    => c.copy(throwOnFirstError = true)
+          case WarnReflectiveNamingAnnotation => c.copy(warnReflectiveNaming = true)
+          case ChiselOutputFileAnnotation(f)  => c.copy(outputFile = Some(f))
+          case ChiselCircuitAnnotation(a)     => c.copy(chiselCircuit = Some(a))
         }
       }
 
diff --git a/src/main/scala/chisel3/stage/phases/Elaborate.scala b/src/main/scala/chisel3/stage/phases/Elaborate.scala
index 55331cb4..ba29e5f2 100644
--- a/src/main/scala/chisel3/stage/phases/Elaborate.scala
+++ b/src/main/scala/chisel3/stage/phases/Elaborate.scala
@@ -29,8 +29,10 @@ class Elaborate extends Phase {
     case ChiselGeneratorAnnotation(gen) =>
       val chiselOptions = view[ChiselOptions](annotations)
       try {
+        val context =
+          new DynamicContext(annotations, chiselOptions.throwOnFirstError, chiselOptions.warnReflectiveNaming)
         val (circuit, dut) =
-          Builder.build(Module(gen()), new DynamicContext(annotations, chiselOptions.throwOnFirstError))
+          Builder.build(Module(gen()), context)
         Seq(ChiselCircuitAnnotation(circuit), DesignAnnotation(dut))
       } catch {
         /* if any throwable comes back and we're in "stack trace trimming" mode, then print an error and trim the stack trace
diff --git a/src/test/scala/chiselTests/naming/ReflectiveNamingSpec.scala b/src/test/scala/chiselTests/naming/ReflectiveNamingSpec.scala
new file mode 100644
index 00000000..baa991dd
--- /dev/null
+++ b/src/test/scala/chiselTests/naming/ReflectiveNamingSpec.scala
@@ -0,0 +1,161 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package chiselTests.naming
+
+import chisel3._
+import chiselTests.{ChiselFlatSpec, Utils}
+
+class ReflectiveNamingSpec extends ChiselFlatSpec with Utils {
+
+  behavior.of("Reflective naming")
+
+  private def emitChirrtl(gen: => RawModule): String = {
+    // Annoyingly need to emit files to use CLI
+    val targetDir = createTestDirectory(this.getClass.getSimpleName).toString
+    val args = Array("--warn:reflective-naming", "-td", targetDir)
+    (new chisel3.stage.ChiselStage).emitChirrtl(gen, args)
+  }
+
+  it should "NOT warn when no names are changed" in {
+    class Example extends Module {
+      val foo, bar = IO(Input(UInt(8.W)))
+      val out = IO(Output(UInt(8.W)))
+
+      val sum = foo +& bar
+      out := sum
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should equal("")
+    chirrtl should include("node sum = add(foo, bar)")
+  }
+
+  it should "warn when changing the name of a node" in {
+    class Example extends Module {
+      val foo, bar = IO(Input(UInt(8.W)))
+      val out = IO(Output(UInt(8.W)))
+
+      val sum = foo +& bar
+      val fuzz = sum
+      out := sum
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should include("'sum' is renamed by reflection to 'fuzz'")
+    chirrtl should include("node fuzz = add(foo, bar)")
+  }
+
+  // This also checks correct prefix reversing
+  it should "warn when changing the name of a node with a prefix in the name" in {
+    class Example extends Module {
+      val foo, bar = IO(Input(UInt(8.W)))
+      val out = IO(Output(UInt(8.W)))
+
+      // This is sketch, don't do this
+      var fuzz = 0.U
+      out := {
+        val sum = {
+          val node = foo +& bar
+          fuzz = node
+          node +% 0.U
+        }
+        sum
+      }
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should include("'out_sum_node' is renamed by reflection to 'fuzz'")
+    chirrtl should include("node fuzz = add(foo, bar)")
+  }
+
+  it should "warn when changing the name of a Module instance" in {
+    import chisel3.util._
+    class Example extends Module {
+      val enq = IO(Flipped(Decoupled(UInt(8.W))))
+      val deq = IO(Decoupled(UInt(8.W)))
+
+      val q = Module(new Queue(UInt(8.W), 4))
+      q.io.enq <> enq
+      deq <> q.io.deq
+
+      val fuzz = q
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should include("'q' is renamed by reflection to 'fuzz'")
+    chirrtl should include("inst fuzz of Queue")
+  }
+
+  it should "warn when changing the name of an Instance" in {
+    import chisel3.experimental.hierarchy.{Definition, Instance}
+    import chiselTests.experimental.hierarchy.Examples.AddOne
+    class Example extends Module {
+      val defn = Definition(new AddOne)
+      val inst = Instance(defn)
+      val fuzz = inst
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should include("'inst' is renamed by reflection to 'fuzz'")
+    chirrtl should include("inst fuzz of AddOne")
+  }
+
+  it should "warn when changing the name of a Mem" in {
+    class Example extends Module {
+      val mem = SyncReadMem(8, UInt(8.W))
+
+      val fuzz = mem
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should include("'mem' is renamed by reflection to 'fuzz'")
+    chirrtl should include("smem fuzz")
+  }
+
+  it should "NOT warn when changing the name of a verification statement" in {
+    class Example extends Module {
+      val in = IO(Input(UInt(8.W)))
+      val z = chisel3.assert(in =/= 123.U)
+      val fuzz = z
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should equal("")
+    // But the name is actually changed
+    (chirrtl should include).regex("assert.*: fuzz")
+  }
+
+  it should "NOT warn when \"naming\" a literal" in {
+    class Example extends Module {
+      val out = IO(Output(UInt(8.W)))
+
+      val sum = 0.U
+      val fuzz = sum
+      out := sum
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should equal("")
+    chirrtl should include("out <= UInt")
+  }
+
+  it should "NOT warn when \"naming\" a field of an Aggregate" in {
+    class Example extends Module {
+      val io = IO(new Bundle {
+        val in = Input(UInt(8.W))
+        val out = Output(UInt(8.W))
+      })
+      val in = io.in
+      val out = io.out
+      out := in
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should equal("")
+    chirrtl should include("io.out <= io.in")
+  }
+
+  it should "NOT warn when \"naming\" unbound Data" in {
+    class Example extends Module {
+      val in = IO(Input(UInt(8.W)))
+      val out = IO(Output(UInt(8.W)))
+      val z = UInt(8.W)
+      val a = z
+      out := in
+    }
+    val (log, chirrtl) = grabLog(emitChirrtl(new Example))
+    log should equal("")
+    chirrtl should include("out <= in")
+  }
+}