Make some usages of suggestName runtime deprecated

core/src/main/scala/chisel3/Module.scala

@@ -117,8 +117,8 @@ object Module extends SourceInfoDoc {
   */
 abstract class Module(implicit moduleCompileOptions: CompileOptions) extends RawModule {
   // Implicit clock and reset pins
-  final val clock: Clock = IO(Input(Clock())).suggestName("clock")
-  final val reset: Reset = IO(Input(mkReset)).suggestName("reset")
+  final val clock: Clock = IO(Input(Clock()))._suggestNameInternal("clock")
+  final val reset: Reset = IO(Input(mkReset))._suggestNameInternal("reset")
 
   // TODO It's hard to remove these deprecated override methods because they're used by
   //   Chisel.QueueCompatibility which extends chisel3.Queue which extends chisel3.Module

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

@@ -73,7 +73,7 @@ package object experimental {
       gen.elements.toSeq.reverse.map {
         case (name, data) =>
           val p = IO(coerceDirection(chiselTypeClone(data).asInstanceOf[Data]))
-          p.suggestName(name)
+          p._suggestNameInternal(name)
           p
 
       }

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

@@ -129,31 +129,42 @@ 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.
+  def _suggestNameInternal(seed: => String): this.type = {
+    if (suggested_seed.isEmpty) suggested_seed = Some(seed)
+    naming_prefix = Builder.getPrefix
+    this
+  }
+
+  /** Takes the first seed suggested. Multiple calls to this function will be ignored (and will become an error in future).
     * If the final computed name conflicts with another name, it may get uniquified by appending
     * a digit at the end.
     *
     * Is a higher priority than [[autoSeed]], in that regardless of whether [[autoSeed]]
     * was called, [[suggestName]] will always take precedence.
     *
+    * @note calling this after the name has already been computed will become an error in the future.
+    *
     * @param seed The seed for the name of this component
     * @return this object
     */
   def suggestName(seed: => String): this.type = {
-    if (suggested_seed.isEmpty) suggested_seed = Some(seed)
-    naming_prefix = Builder.getPrefix
-    this
+    require(Builder.hasDynamicContext, s"suggestName (${seed}) should only be called from a Builder context.")
+    if (suggested_seed.isDefined) {
+      Builder.deprecated(
+        s"Calling .suggestName(\"$seed\"), when already called with \"${suggested_seed.get}\", will become an error in Chisel 3.6"
+      )
+    }
+    if (!HasId.canBeNamed(this)) {
+      Builder.deprecated(
+        s"Calling .suggestName(\"$seed\") on \"$this\" (which cannot actually be named) will become an error in Chisel 3.6"
+      )
+    }
+    if (_parent.map(_.isClosed).getOrElse(false)) { // not sure what it means to have no parent
+      Builder.deprecated(
+        s"Calling .suggestName(\"$seed\") on ${this} when the containing module \"${_parent.get.name}\" has already completed elaboration will become an error in Chisel 3.6"
+      )
+    }
+    _suggestNameInternal(seed)
   }
 
   // Internal version of .suggestName that can override a user-suggested name
@@ -163,11 +174,12 @@ private[chisel3] trait HasId extends InstanceId {
     // This could be called with user prefixes, ignore them
     noPrefix {
       suggested_seed = Some(seed)
-      this.suggestName(seed)
+      this._suggestNameInternal(seed)
     }
   }
 
   /** Computes the name of this HasId, if one exists
+    *
     * @param defaultSeed Optionally provide default seed for computing the name
     * @return the name, if it can be computed
     */
@@ -178,10 +190,22 @@ private[chisel3] trait HasId extends InstanceId {
   }
 
   /** This resolves the precedence of [[autoSeed]] and [[suggestName]]
+    *
+    * @note It will become an error in the future to suggestName the same thing that autoSeed would have assigned
     *
     * @return the current calculation of a name, if it exists
     */
-  private[chisel3] def seedOpt: Option[String] = suggested_seed.orElse(auto_seed)
+  private[chisel3] def seedOpt: Option[String] = {
+    suggested_seed.zip(auto_seed).foreach {
+      case (suggested, auto) =>
+        if (suggested == auto) {
+          Builder.deprecated(
+            s"calling .suggestName(\"${suggested}\") had no effect as it is the same as the automatically given name, this will become an error in 3.6"
+          )
+        }
+    }
+    suggested_seed.orElse(auto_seed)
+  }
 
   /** @return Whether either autoName or suggestName has been called */
   def hasSeed: Boolean = seedOpt.isDefined
@@ -268,7 +292,26 @@ private[chisel3] trait HasId extends InstanceId {
   }
 }
 
-/** Holds the implementation of toNamed for Data and MemBase */
+private[chisel3] object HasId {
+
+  /** Utility for things that (currently) appear to be nameable but actually cannot be */
+  private def canBeNamed(id: HasId): Boolean = id match {
+    case d: Data =>
+      d.binding match {
+        case Some(_: ConstrainedBinding) => true
+        case _ => false
+      }
+    case b: BaseModule => true
+    case m: MemBase[_] => true
+    // These names don't affect hardware
+    case _: VerificationStatement => false
+    // While the above should be comprehensive, since this is used in warning we want to be careful
+    // to never accidentally have a match error
+    case _ => false
+  }
+}
+
+/** Holds the implementation of to for Data and MemBase */
 private[chisel3] trait NamedComponent extends HasId {
 
   /** Returns a FIRRTL ComponentName that references this object

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

@@ -108,7 +108,7 @@ class NamingContext extends NamingContextInterface {
     closed = true
     for ((ref, suffix) <- items) {
       // First name the top-level object
-      chisel3.internal.Builder.nameRecursively(prefix + suffix, ref, (id, name) => id.suggestName(name))
+      chisel3.internal.Builder.nameRecursively(prefix + suffix, ref, (id, name) => id._suggestNameInternal(name))
 
       // Then recurse into descendant contexts
       if (descendants.containsKey(ref)) {

src/main/scala/chisel3/util/Arbiter.scala

@@ -132,7 +132,6 @@ class RRArbiter[T <: Data](val gen: T, val n: Int) extends LockingRRArbiter[T](g
   * consumer.io.in <> arb.io.out
   * }}}
   */
-@chiselName
 class Arbiter[T <: Data](val gen: T, val n: Int) extends Module {
   val io = IO(new ArbiterIO(gen, n))
 

src/main/scala/chisel3/util/Decoupled.scala

@@ -344,7 +344,6 @@ object Queue {
     *   consumer.io.in <> Queue(producer.io.out, 16)
     * }}}
     */
-  @nowarn("cat=deprecation&msg=TransitName")
   @chiselName
   def apply[T <: Data](
     enq:            ReadyValidIO[T],