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>

2 files changed, 25 insertions, 19 deletions

diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index 97c3bc49..6f02b57c 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -137,6 +137,17 @@ private[chisel3] trait HasId extends InstanceId {
     this
   }
 
+  // Private internal version of suggestName that tells you if the name changed
+  // Returns Some(old name, old prefix) if name changed, None otherwise
+  private[chisel3] def _suggestNameCheck(seed: => String): Option[(String, Prefix)] = {
+    val oldSeed = this.seedOpt
+    val oldPrefix = this.naming_prefix
+    suggestName(seed)
+    if (oldSeed.nonEmpty && (oldSeed != this.seedOpt || oldPrefix != this.naming_prefix)) {
+      Some(oldSeed.get -> oldPrefix)
+    } else None
+  }
+
   /** Takes the first seed suggested. Multiple calls to this function will be ignored.
     * If the final computed name conflicts with another name, it may get uniquified by appending
     * a digit at the end.
@@ -171,22 +182,6 @@ private[chisel3] trait HasId extends InstanceId {
     * @return the name, if it can be computed
     */
   private[chisel3] def _computeName(defaultPrefix: Option[String], defaultSeed: Option[String]): Option[String] = {
-
-    /** Computes a name of this signal, given the seed and prefix
-      * @param seed
-      * @param prefix
-      * @return
-      */
-    def buildName(seed: String, prefix: Prefix): String = {
-      val builder = new StringBuilder()
-      prefix.foreach { p =>
-        builder ++= p
-        builder += '_'
-      }
-      builder ++= seed
-      builder.toString
-    }
-
     if (hasSeed) {
       Some(buildName(seedOpt.get, naming_prefix.reverse))
     } else {
@@ -374,7 +369,10 @@ private[chisel3] class ChiselContext() {
   val viewNamespace = Namespace.empty
 }
 
-private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq, val throwOnFirstError: Boolean) {
+private[chisel3] class DynamicContext(
+  val annotationSeq:        AnnotationSeq,
+  val throwOnFirstError:    Boolean,
+  val warnReflectiveNaming: Boolean) {
   val importDefinitionAnnos = annotationSeq.collect { case a: ImportDefinitionAnnotation[_] => a }
 
   // Ensure there are no repeated names for imported Definitions
@@ -649,6 +647,8 @@ private[chisel3] object Builder extends LazyLogging {
     throwException("Error: No implicit reset.")
   )
 
+  def warnReflectiveNaming: Boolean = dynamicContext.warnReflectiveNaming
+
   // TODO(twigg): Ideally, binding checks and new bindings would all occur here
   // However, rest of frontend can't support this yet.
   def pushCommand[T <: Command](c: T): T = {
@@ -690,8 +690,9 @@ private[chisel3] object Builder extends LazyLogging {
       throwException(m)
     }
   }
-  def warning(m:    => String): Unit = if (dynamicContextVar.value.isDefined) errors.warning(m)
-  def deprecated(m: => String, location: Option[String] = None): Unit =
+  def warning(m:      => String): Unit = if (dynamicContextVar.value.isDefined) errors.warning(m)
+  def warningNoLoc(m: => String): Unit = if (dynamicContextVar.value.isDefined) errors.warningNoLoc(m)
+  def deprecated(m:   => String, location: Option[String] = None): Unit =
     if (dynamicContextVar.value.isDefined) errors.deprecated(m, location)
 
   /** Record an exception as an error, and throw it.
diff --git a/core/src/main/scala/chisel3/internal/Error.scala b/core/src/main/scala/chisel3/internal/Error.scala
index 62086870..3b0846eb 100644
--- a/core/src/main/scala/chisel3/internal/Error.scala
+++ b/core/src/main/scala/chisel3/internal/Error.scala
@@ -186,6 +186,11 @@ private[chisel3] class ErrorLog {
   def warning(m: => String): Unit =
     errors += new Warning(m, getUserLineNumber)
 
+  /** Log a warning message without a source locator */
+  def warningNoLoc(m: => String): Unit = {
+    errors += new Warning(m, None)
+  }
+
   /** Emit an informational message */
   @deprecated("This method will be removed in 3.5", "3.4")
   def info(m: String): Unit =