From b6addc702000c20a842fea91a7021c66a5d82a07 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 2 Jun 2022 09:11:23 -0700 Subject: [PATCH 01/28] suggestName: add some runtime deprecations * suggestName -- add some runtime deprecations * suggestName -- make it an error to call suggestName outside of Builder Context * SuggestName: create an internal version and use it internally to avoid runtime deprecations * Added a SuggestNameSpec to test suggestName behavior --- core/src/main/scala/chisel3/Module.scala | 4 +- .../scala/chisel3/experimental/package.scala | 2 +- .../main/scala/chisel3/internal/Builder.scala | 49 +++++++++- .../main/scala/chisel3/internal/Namer.scala | 2 +- .../chiselTests/naming/SuggestNameSpec.scala | 98 +++++++++++++++++++ 5 files changed, 148 insertions(+), 7 deletions(-) create mode 100644 src/test/scala/chiselTests/naming/SuggestNameSpec.scala diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index cf08bb2a945..a8e77a2eb65 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/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 diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index 730aae8be51..a95f134da85 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/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 } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index cefc3a798c9..a674f81b715 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -140,20 +140,49 @@ private[chisel3] trait HasId extends InstanceId { } else None } - /** Takes the first seed suggested. Multiple calls to this function will be ignored. + /** Takes the first seed suggested. Multiple calls to this function will be ignored. */ + private[chisel3] 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 = { +<<<<<<< HEAD + if (suggested_seed.isEmpty) suggested_seed = Some(seed) + naming_prefix = Builder.getPrefix + this +||||||| parent of 08934795... suggestName: add some runtime deprecations if (suggested_seed.isEmpty) suggested_seed = Some(seed) naming_prefix = Builder.getPrefix + for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } 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}) will become an error in Chisel 3.6" + ) + } + if (_computedName.isDefined) { + Builder.deprecated( + s"Calling suggestName ($seed, when the name was already computed as ${_computedName.get})) will become an error in Chisel 3.6" + ) + } + suggestNameInternal(seed) +>>>>>>> 08934795... suggestName: add some runtime deprecations } // Internal version of .suggestName that can override a user-suggested name @@ -163,10 +192,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) } } + private[chisel3] var _computedName: Option[Option[String]] = None + /** 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 +209,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 auto prefixed 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 diff --git a/core/src/main/scala/chisel3/internal/Namer.scala b/core/src/main/scala/chisel3/internal/Namer.scala index c36aafeff9c..c4fadec3497 100644 --- a/core/src/main/scala/chisel3/internal/Namer.scala +++ b/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)) { diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala new file mode 100644 index 00000000000..4ded7658475 --- /dev/null +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests.naming + +import chisel3._ +import chisel3.aop.Select +import chisel3.stage.{ChiselGeneratorAnnotation, ChiselStage} +import chisel3.experimental.{dump, noPrefix, prefix, treedump} +import chiselTests.{ChiselPropSpec, Utils} +import chisel3.stage.{DesignAnnotation, NoRunFirrtlCompilerAnnotation} +import firrtl.options.{Dependency, Phase, PhaseManager} +import firrtl.options.phases.DeletedWrapper + +class SuggestNameSpec extends ChiselPropSpec with Utils { + implicit val minimumMajorVersion: Int = 12 + + property("0. Calling suggestName 2x should be a runtime deprecation") { + class Test extends Module { + { + val wire = { + val x = WireInit(0.U(3.W)).suggestName("mywire") + dontTouch(x) + } + wire.suggestName("somethingElse") + } + } + val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) + log should include( + "Calling suggestName (somethingElse, when already called with Some(mywire)) will become an error in Chisel 3.6" + ) + } + + property("1. Calling suggestName outside of a Builder context should be an error") { + class Test extends Module { + val wire = { + val x = WireInit(0.U(3.W)) + dontTouch(x) + } + } + + val pm = new PhaseManager(Seq(Dependency[chisel3.stage.phases.Checks], Dependency[chisel3.stage.phases.Elaborate])) + val test = pm + .transform(Seq(ChiselGeneratorAnnotation(() => new Test()), NoRunFirrtlCompilerAnnotation)) + .collectFirst { + case d: DesignAnnotation[_] => d + } + .get + .design + val caught = intercept[IllegalArgumentException] { + test.asInstanceOf[Test].wire.suggestName("somethingElse") + } + caught.getMessage should include("suggestName (somethingElse) should only be called from a Builder context") + } + + property("2. Calling suggestName after module close should be a runtime deprecation") { + class Child extends Module { + val wire = { + val x = WireInit(0.U(3.W)) + dontTouch(x) + } + } + class Test extends Module { + val child = Module(new Child()) + child.wire.suggestName("somethingElse") + } + val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) + log should include( + "Calling suggestName (somethingElse, when the name was already computed " + ) + } + + property("3. Calling suggestName after toString should be a runtime deprecation") { + class Test extends Module { + val wire = { + val x = WireInit(0.U(3.W)) + val y = x.toString + x.suggestName("somethingElse") + } + } + val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) + log should include( + "Calling suggestName (somethingElse, when the name was already computed " + ) + } + + property("4. Calling suggestName with the same thing prefix would have given should be a runtime deprecation") { + class Test extends Module { + val wire = { + val x = WireInit(0.U(3.W)).suggestName("wire") + dontTouch(x) + } + } + val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) + log should include( + "calling suggestName(wire) had no effect as it is the same as the auto prefixed name" + ) + } +} From 08543f3946a3c38fc158b997062ce1015e1e7efc Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Sun, 19 Jun 2022 15:18:06 -0700 Subject: [PATCH 02/28] no more _computedName --- core/src/main/scala/chisel3/internal/Builder.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index a674f81b715..c3fca73351e 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -196,9 +196,8 @@ private[chisel3] trait HasId extends InstanceId { } } - private[chisel3] var _computedName: Option[Option[String]] = None - /** 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 */ From 236a4fd892bf4d0478a652cab9d6c7b2dced5b23 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Sun, 19 Jun 2022 15:23:38 -0700 Subject: [PATCH 03/28] post merge cleanups --- core/src/main/scala/chisel3/Module.scala | 4 +-- .../scala/chisel3/experimental/package.scala | 2 +- .../main/scala/chisel3/internal/Builder.scala | 34 ++----------------- .../main/scala/chisel3/internal/Namer.scala | 2 +- 4 files changed, 7 insertions(+), 35 deletions(-) diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index a8e77a2eb65..30e7338487b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/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())).suggestNameInternal("clock") - final val reset: Reset = IO(Input(mkReset)).suggestNameInternal("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 diff --git a/core/src/main/scala/chisel3/experimental/package.scala b/core/src/main/scala/chisel3/experimental/package.scala index a95f134da85..290a15877a4 100644 --- a/core/src/main/scala/chisel3/experimental/package.scala +++ b/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.suggestNameInternal(name) + p._suggestNameInternal(name) p } diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index c3fca73351e..828e3b9e89e 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -129,19 +129,7 @@ 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. */ - private[chisel3] def suggestNameInternal(seed: => String): this.type = { + def _suggestNameInternal(seed: => String): this.type = { if (suggested_seed.isEmpty) suggested_seed = Some(seed) naming_prefix = Builder.getPrefix this @@ -160,29 +148,13 @@ private[chisel3] trait HasId extends InstanceId { * @return this object */ def suggestName(seed: => String): this.type = { -<<<<<<< HEAD - if (suggested_seed.isEmpty) suggested_seed = Some(seed) - naming_prefix = Builder.getPrefix - this -||||||| parent of 08934795... suggestName: add some runtime deprecations - if (suggested_seed.isEmpty) suggested_seed = Some(seed) - naming_prefix = Builder.getPrefix - for (hook <- suggest_postseed_hooks.reverse) { hook(seed) } - 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}) will become an error in Chisel 3.6" ) } - if (_computedName.isDefined) { - Builder.deprecated( - s"Calling suggestName ($seed, when the name was already computed as ${_computedName.get})) will become an error in Chisel 3.6" - ) - } - suggestNameInternal(seed) ->>>>>>> 08934795... suggestName: add some runtime deprecations + _suggestNameInternal(seed) } // Internal version of .suggestName that can override a user-suggested name @@ -192,7 +164,7 @@ private[chisel3] trait HasId extends InstanceId { // This could be called with user prefixes, ignore them noPrefix { suggested_seed = Some(seed) - this.suggestNameInternal(seed) + this._suggestNameInternal(seed) } } diff --git a/core/src/main/scala/chisel3/internal/Namer.scala b/core/src/main/scala/chisel3/internal/Namer.scala index c4fadec3497..4f62480f6ee 100644 --- a/core/src/main/scala/chisel3/internal/Namer.scala +++ b/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.suggestNameInternal(name)) + chisel3.internal.Builder.nameRecursively(prefix + suffix, ref, (id, name) => id._suggestNameInternal(name)) // Then recurse into descendant contexts if (descendants.containsKey(ref)) { From eaedd38eb4cec6cce8ec4307fbeffe4b26149add Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Sun, 19 Jun 2022 16:12:00 -0700 Subject: [PATCH 04/28] Deleted a bunch of chiselName and TransitName to get tests to pass because they are hitting the 'you get the same name as the prefix' warnings --- .../main/scala/chisel3/internal/Builder.scala | 33 ++++- src/main/scala/chisel3/util/Arbiter.scala | 2 - src/main/scala/chisel3/util/CircuitMath.scala | 1 - src/main/scala/chisel3/util/Counter.scala | 3 - src/main/scala/chisel3/util/Decoupled.scala | 5 - src/test/scala/chiselTests/StrongEnum.scala | 1 - .../chiselTests/naming/SuggestNameSpec.scala | 133 ++++++++++++++++++ 7 files changed, 165 insertions(+), 13 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 828e3b9e89e..d5f8296b705 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -154,6 +154,18 @@ private[chisel3] trait HasId extends InstanceId { s"Calling suggestName ($seed, when already called with ${suggested_seed}) will become an error in Chisel 3.6" ) } + if (!HasId.canBeNamed(this)) { + Builder.deprecated( + s"Calling suggestName ($seed, on something that cannot actually be named: ${this}) will become an error in Chisel 3.6" + ) + } + /* + if (_computedName.isDefined) { + Builder.deprecated( + s"Calling suggestName ($seed, when the name was already computed as ${_computedName.get})) will become an error in Chisel 3.6" + ) + } + */ _suggestNameInternal(seed) } @@ -282,7 +294,26 @@ private[chisel3] trait HasId extends InstanceId { } } -/** Holds the implementation of toNamed for Data and MemBase */ +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 diff --git a/src/main/scala/chisel3/util/Arbiter.scala b/src/main/scala/chisel3/util/Arbiter.scala index e098fc09bbd..f4f4c9f3402 100644 --- a/src/main/scala/chisel3/util/Arbiter.scala +++ b/src/main/scala/chisel3/util/Arbiter.scala @@ -116,7 +116,6 @@ class LockingArbiter[T <: Data](gen: T, n: Int, count: Int, needsLock: Option[T * consumer.io.in <> arb.io.out * }}} */ -@chiselName class RRArbiter[T <: Data](val gen: T, val n: Int) extends LockingRRArbiter[T](gen, n, 1) /** Hardware module that is used to sequence n producers into 1 consumer. @@ -132,7 +131,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)) diff --git a/src/main/scala/chisel3/util/CircuitMath.scala b/src/main/scala/chisel3/util/CircuitMath.scala index df60f059f0c..58a8ba98331 100644 --- a/src/main/scala/chisel3/util/CircuitMath.scala +++ b/src/main/scala/chisel3/util/CircuitMath.scala @@ -22,7 +22,6 @@ object Log2 { /** Returns the base-2 integer logarithm of the least-significant `width` bits of an UInt. */ - @chiselName def apply(x: Bits, width: Int): UInt = { if (width < 2) { 0.U diff --git a/src/main/scala/chisel3/util/Counter.scala b/src/main/scala/chisel3/util/Counter.scala index ef1eff9f3e2..eabee3f5374 100644 --- a/src/main/scala/chisel3/util/Counter.scala +++ b/src/main/scala/chisel3/util/Counter.scala @@ -27,7 +27,6 @@ import chisel3.internal.naming.chiselName // can't use chisel3_ version because * } * }}} */ -@chiselName class Counter private (r: Range, oldN: Option[Int] = None) { require(r.length > 0, s"Counter range cannot be empty, got: $r") require(r.start >= 0 && r.end >= 0, s"Counter range must be positive, got: $r") @@ -113,7 +112,6 @@ object Counter { * @return tuple of the counter value and whether the counter will wrap (the value is at * maximum and the condition is true). */ - @chiselName def apply(cond: Bool, n: Int): (UInt, Bool) = { val c = new Counter(n) val wrap = WireInit(false.B) @@ -129,7 +127,6 @@ object Counter { * @return tuple of the counter value and whether the counter will wrap (the value is at * maximum and the condition is true). */ - @chiselName def apply(r: Range, enable: Bool = true.B, reset: Bool = false.B): (UInt, Bool) = { val c = new Counter(r) val wrap = WireInit(false.B) diff --git a/src/main/scala/chisel3/util/Decoupled.scala b/src/main/scala/chisel3/util/Decoupled.scala index ede907fad89..1adfdc57d2a 100644 --- a/src/main/scala/chisel3/util/Decoupled.scala +++ b/src/main/scala/chisel3/util/Decoupled.scala @@ -121,7 +121,6 @@ object Decoupled { * * @note unsafe (and will error) on the producer (input) side of an IrrevocableIO */ - @chiselName def apply[T <: Data](irr: IrrevocableIO[T]): DecoupledIO[T] = { require( DataMirror.directionOf(irr.bits) == Direction.Output, @@ -233,7 +232,6 @@ class QueueIO[T <: Data]( * consumer.io.in <> q.io.deq * }}} */ -@chiselName class Queue[T <: Data]( val gen: T, val entries: Int, @@ -344,8 +342,6 @@ object Queue { * consumer.io.in <> Queue(producer.io.out, 16) * }}} */ - @nowarn("cat=deprecation&msg=TransitName") - @chiselName def apply[T <: Data]( enq: ReadyValidIO[T], entries: Int = 2, @@ -390,7 +386,6 @@ object Queue { * consumer.io.in <> Queue(producer.io.out, 16) * }}} */ - @chiselName def irrevocable[T <: Data]( enq: ReadyValidIO[T], entries: Int = 2, diff --git a/src/test/scala/chiselTests/StrongEnum.scala b/src/test/scala/chiselTests/StrongEnum.scala index c43d832afb8..2496da834be 100644 --- a/src/test/scala/chiselTests/StrongEnum.scala +++ b/src/test/scala/chiselTests/StrongEnum.scala @@ -575,7 +575,6 @@ class StrongEnumAnnotator extends Module { val indexed2 = vec_of_bundles(cycle) } -@chiselName class StrongEnumAnnotatorWithChiselName extends Module { import EnumExample._ diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 4ded7658475..78a24e1a318 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -52,6 +52,8 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { caught.getMessage should include("suggestName (somethingElse) should only be called from a Builder context") } + /* + // TODO: I removed the _computedName code that would check for this... how to determine if we are after module close? property("2. Calling suggestName after module close should be a runtime deprecation") { class Child extends Module { val wire = { @@ -82,6 +84,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { "Calling suggestName (somethingElse, when the name was already computed " ) } + */ property("4. Calling suggestName with the same thing prefix would have given should be a runtime deprecation") { class Test extends Module { @@ -95,4 +98,134 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { "calling suggestName(wire) had no effect as it is the same as the auto prefixed name" ) } + + property("5a. Calling suggestName on a node should be allowed") { + class Example extends Module { + val foo, bar = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + + val sum = foo +& bar + sum.suggestName("fuzz") + out := sum + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("node fuzz = add(foo, bar)") + } + + property("5b. Calling suggestName on a prefixed node should be allowed") { + class Example extends Module { + val foo, bar = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + + out := { + val sum = { + val node = foo +& bar + node.suggestName("fuzz") + node +% 0.U + } + sum + } + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("node out_sum_fuzz = add(foo, bar)") + } + + property("5c. Calling suggestName on a Module instance should be allowed") { + 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 + q.suggestName("fuzz") + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("inst fuzz of Queue") + } + + /* + property("5d. Calling suggestName on an Instance instance should be allowed") { + 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) + inst.suggestName("fuzz") + val fuzz = inst + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("inst fuzz of AddOne") + } + */ + + property("5e. Calling suggestName on a Mem should be allowed") { + class Example extends Module { + val mem = SyncReadMem(8, UInt(8.W)) + + mem.suggestName("fuzz") + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("smem fuzz") + } + + property("5f. Calling suggestName on a verif statement should be a runtime deprecation") { + class Example extends Module { + val in = IO(Input(UInt(8.W))) + val z = chisel3.assert(in =/= 123.U) + z.suggestName("fuzz") + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should include("Calling suggestName (fuzz, on something that cannot actually be named: chisel3.assert$Assert") + (chirrtl should include).regex("assert.*: z") + } + + property("5g. Calling suggestName on a literal should be a runtime deprecation") { + class Example extends Module { + val out = IO(Output(UInt(8.W))) + + val sum = 0.U + sum.suggestName("fuzz") + out := sum + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should include("Calling suggestName (fuzz, on something that cannot actually be named: UInt<1>(0)") + chirrtl should include("out <= UInt") + } + + property("5h. Calling suggestName on a field of an Aggregate should be a runtime deprecation") { + class Example extends Module { + val io = IO(new Bundle { + val in = Input(UInt(8.W)) + val out = Output(UInt(8.W)) + }) + io.in.suggestName("fuzz") + io.out.suggestName("bar") + io.out := io.in + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should include("Calling suggestName (fuzz, on something that cannot actually be named: Example.io.in") + log should include("Calling suggestName (fuzz, on something that cannot actually be named: Example.io.in") + + chirrtl should include("io.out <= io.in") + } + + property("5i. Calling suggestName on unbound Data should be a runtime deprecation") { + class Example extends Module { + val in = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + val z = UInt(8.W) + z.suggestName("fuzz") + out := in + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should include("Calling suggestName (fuzz, on something that cannot actually be named: UInt<8>)") + chirrtl should include("out <= in") + } } From 87f0e41f83c3870c004acc43084e1fb0f652b542 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 24 Jun 2022 10:24:45 -0700 Subject: [PATCH 05/28] Add back the check for suggestName after module close --- .../main/scala/chisel3/internal/Builder.scala | 6 ++---- .../chiselTests/naming/SuggestNameSpec.scala | 21 ++----------------- 2 files changed, 4 insertions(+), 23 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index d5f8296b705..bd18260eee2 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -159,13 +159,11 @@ private[chisel3] trait HasId extends InstanceId { s"Calling suggestName ($seed, on something that cannot actually be named: ${this}) will become an error in Chisel 3.6" ) } - /* - if (_computedName.isDefined) { + if (_parent.map(_.isClosed).getOrElse(false)) { // not sure what it means to have no parent Builder.deprecated( - s"Calling suggestName ($seed, when the name was already computed as ${_computedName.get})) will become an error in Chisel 3.6" + s"Calling suggestName ($seed, on ${this}, when the containing module (${_parent.get.name}) completed elaboration already will become an error in Chisel 3.6" ) } - */ _suggestNameInternal(seed) } diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 78a24e1a318..12d8ca759c9 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -52,8 +52,6 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { caught.getMessage should include("suggestName (somethingElse) should only be called from a Builder context") } - /* - // TODO: I removed the _computedName code that would check for this... how to determine if we are after module close? property("2. Calling suggestName after module close should be a runtime deprecation") { class Child extends Module { val wire = { @@ -67,26 +65,11 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) log should include( - "Calling suggestName (somethingElse, when the name was already computed " - ) - } - - property("3. Calling suggestName after toString should be a runtime deprecation") { - class Test extends Module { - val wire = { - val x = WireInit(0.U(3.W)) - val y = x.toString - x.suggestName("somethingElse") - } - } - val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) - log should include( - "Calling suggestName (somethingElse, when the name was already computed " + "Calling suggestName (somethingElse, on Child.wire: Wire[UInt<3>], when the containing module (Child) completed elaboration already" ) } - */ - property("4. Calling suggestName with the same thing prefix would have given should be a runtime deprecation") { + property("3. Calling suggestName with the same thing prefix would have given should be a runtime deprecation") { class Test extends Module { val wire = { val x = WireInit(0.U(3.W)).suggestName("wire") From f0d24993f5cf2c4eea353cb8423c573993649a6f Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 24 Jun 2022 10:28:12 -0700 Subject: [PATCH 06/28] Add another case that we can name IOs --- .../chiselTests/naming/SuggestNameSpec.scala | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 12d8ca759c9..b09c149ea80 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -96,6 +96,22 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("node fuzz = add(foo, bar)") } + property("5a. Calling suggestName on an IO should be allowed") { + class Example extends Module { + val foo, bar = IO(Input(UInt(8.W))) + val out = IO(Output(UInt(8.W))) + + val sum = foo +& bar + foo.suggestName("FOO") + bar.suggestName("BAR") + out := sum + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("node sum = add(FOO, BAR)") + } + + property("5b. Calling suggestName on a prefixed node should be allowed") { class Example extends Module { val foo, bar = IO(Input(UInt(8.W))) @@ -194,7 +210,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) log should include("Calling suggestName (fuzz, on something that cannot actually be named: Example.io.in") - log should include("Calling suggestName (fuzz, on something that cannot actually be named: Example.io.in") + log should include("Calling suggestName (bar, on something that cannot actually be named: Example.io.out") chirrtl should include("io.out <= io.in") } From a175c1d634cf8c2b62e7a71b5d3aa426d6a7ab7a Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 24 Jun 2022 11:45:38 -0700 Subject: [PATCH 07/28] scalafmt --- src/test/scala/chiselTests/naming/SuggestNameSpec.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index b09c149ea80..565855dde90 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -96,7 +96,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("node fuzz = add(foo, bar)") } - property("5a. Calling suggestName on an IO should be allowed") { + property("5a. Calling suggestName on an IO should be allowed") { class Example extends Module { val foo, bar = IO(Input(UInt(8.W))) val out = IO(Output(UInt(8.W))) @@ -111,7 +111,6 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("node sum = add(FOO, BAR)") } - property("5b. Calling suggestName on a prefixed node should be allowed") { class Example extends Module { val foo, bar = IO(Input(UInt(8.W))) From aceaae419b499403b952cbe5361e2d922696283a Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Mon, 27 Jun 2022 13:46:36 -0700 Subject: [PATCH 08/28] scalafmt --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index bd18260eee2..c19fc98854f 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -179,7 +179,7 @@ private[chisel3] trait HasId extends InstanceId { } /** 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 */ From 3918123f42cd055d26f8e00472f7c42794f34beb Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 28 Jun 2022 12:55:56 -0700 Subject: [PATCH 09/28] Add back the @chiselName --- src/main/scala/chisel3/util/Arbiter.scala | 1 + src/main/scala/chisel3/util/CircuitMath.scala | 1 + src/main/scala/chisel3/util/Counter.scala | 3 +++ src/main/scala/chisel3/util/Decoupled.scala | 4 ++++ src/test/scala/chiselTests/StrongEnum.scala | 1 + 5 files changed, 10 insertions(+) diff --git a/src/main/scala/chisel3/util/Arbiter.scala b/src/main/scala/chisel3/util/Arbiter.scala index f4f4c9f3402..f503c4033fa 100644 --- a/src/main/scala/chisel3/util/Arbiter.scala +++ b/src/main/scala/chisel3/util/Arbiter.scala @@ -116,6 +116,7 @@ class LockingArbiter[T <: Data](gen: T, n: Int, count: Int, needsLock: Option[T * consumer.io.in <> arb.io.out * }}} */ +@chiselName class RRArbiter[T <: Data](val gen: T, val n: Int) extends LockingRRArbiter[T](gen, n, 1) /** Hardware module that is used to sequence n producers into 1 consumer. diff --git a/src/main/scala/chisel3/util/CircuitMath.scala b/src/main/scala/chisel3/util/CircuitMath.scala index 58a8ba98331..df60f059f0c 100644 --- a/src/main/scala/chisel3/util/CircuitMath.scala +++ b/src/main/scala/chisel3/util/CircuitMath.scala @@ -22,6 +22,7 @@ object Log2 { /** Returns the base-2 integer logarithm of the least-significant `width` bits of an UInt. */ + @chiselName def apply(x: Bits, width: Int): UInt = { if (width < 2) { 0.U diff --git a/src/main/scala/chisel3/util/Counter.scala b/src/main/scala/chisel3/util/Counter.scala index eabee3f5374..ef1eff9f3e2 100644 --- a/src/main/scala/chisel3/util/Counter.scala +++ b/src/main/scala/chisel3/util/Counter.scala @@ -27,6 +27,7 @@ import chisel3.internal.naming.chiselName // can't use chisel3_ version because * } * }}} */ +@chiselName class Counter private (r: Range, oldN: Option[Int] = None) { require(r.length > 0, s"Counter range cannot be empty, got: $r") require(r.start >= 0 && r.end >= 0, s"Counter range must be positive, got: $r") @@ -112,6 +113,7 @@ object Counter { * @return tuple of the counter value and whether the counter will wrap (the value is at * maximum and the condition is true). */ + @chiselName def apply(cond: Bool, n: Int): (UInt, Bool) = { val c = new Counter(n) val wrap = WireInit(false.B) @@ -127,6 +129,7 @@ object Counter { * @return tuple of the counter value and whether the counter will wrap (the value is at * maximum and the condition is true). */ + @chiselName def apply(r: Range, enable: Bool = true.B, reset: Bool = false.B): (UInt, Bool) = { val c = new Counter(r) val wrap = WireInit(false.B) diff --git a/src/main/scala/chisel3/util/Decoupled.scala b/src/main/scala/chisel3/util/Decoupled.scala index 1adfdc57d2a..9c8097eeb1b 100644 --- a/src/main/scala/chisel3/util/Decoupled.scala +++ b/src/main/scala/chisel3/util/Decoupled.scala @@ -121,6 +121,7 @@ object Decoupled { * * @note unsafe (and will error) on the producer (input) side of an IrrevocableIO */ + @chiselName def apply[T <: Data](irr: IrrevocableIO[T]): DecoupledIO[T] = { require( DataMirror.directionOf(irr.bits) == Direction.Output, @@ -232,6 +233,7 @@ class QueueIO[T <: Data]( * consumer.io.in <> q.io.deq * }}} */ +@chiselName class Queue[T <: Data]( val gen: T, val entries: Int, @@ -342,6 +344,7 @@ object Queue { * consumer.io.in <> Queue(producer.io.out, 16) * }}} */ + @chiselName def apply[T <: Data]( enq: ReadyValidIO[T], entries: Int = 2, @@ -386,6 +389,7 @@ object Queue { * consumer.io.in <> Queue(producer.io.out, 16) * }}} */ + @chiselName def irrevocable[T <: Data]( enq: ReadyValidIO[T], entries: Int = 2, diff --git a/src/test/scala/chiselTests/StrongEnum.scala b/src/test/scala/chiselTests/StrongEnum.scala index 2496da834be..c43d832afb8 100644 --- a/src/test/scala/chiselTests/StrongEnum.scala +++ b/src/test/scala/chiselTests/StrongEnum.scala @@ -575,6 +575,7 @@ class StrongEnumAnnotator extends Module { val indexed2 = vec_of_bundles(cycle) } +@chiselName class StrongEnumAnnotatorWithChiselName extends Module { import EnumExample._ From dbb9078977c47e1921ce96b372f3850f0c49f8d7 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 28 Jun 2022 12:59:40 -0700 Subject: [PATCH 10/28] SuggestNameSpec: use something other than Queue to avoid issues with chiselName --- .../chiselTests/naming/SuggestNameSpec.scala | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 565855dde90..e7c71d2c074 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -132,18 +132,26 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { property("5c. Calling suggestName on a Module instance should be allowed") { import chisel3.util._ + + class PassThrough extends Module { + val enq = IO(Flipped(Decoupled(UInt(8.W)))) + val deq = IO(Decoupled(UInt(8.W))) + deq <> enq + } + 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 q = Module(new PassThrough()) + q.enq <> enq + deq <> q.deq q.suggestName("fuzz") } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) log should equal("") - chirrtl should include("inst fuzz of Queue") + chirrtl should include("inst fuzz of PassThrough") } /* From cd07eded6d37ca75603b87c723a9c9c84a89e986 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 28 Jun 2022 13:38:40 -0700 Subject: [PATCH 11/28] I don't need to suggestName on clock/reset do I? --- core/src/main/scala/chisel3/Module.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 30e7338487b..789bcab62d9 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/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()))._suggestNameInternal("clock") - final val reset: Reset = IO(Input(mkReset))._suggestNameInternal("reset") + final val clock: Clock = IO(Input(Clock())) + final val reset: Reset = IO(Input(mkReset)) // 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 From a0d145d2873203e350baa5e4cf60b43117b51821 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Tue, 28 Jun 2022 14:03:53 -0700 Subject: [PATCH 12/28] Revert "I don't need to suggestName on clock/reset do I?" This reverts commit cd07eded6d37ca75603b87c723a9c9c84a89e986. --- core/src/main/scala/chisel3/Module.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 789bcab62d9..30e7338487b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/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())) - final val reset: Reset = IO(Input(mkReset)) + 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 From a8b296fb939c3e2d5f81e2e037668c3131cb194f Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Thu, 30 Jun 2022 13:55:44 -0700 Subject: [PATCH 13/28] Update core/src/main/scala/chisel3/internal/Builder.scala Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index c19fc98854f..c30b368b2ec 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -151,7 +151,7 @@ private[chisel3] trait HasId extends InstanceId { 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}) will become an error in Chisel 3.6" + s"Calling .suggestName(\"$seed\"), when already called with \"${suggested_seed.get}\", will become an error in Chisel 3.6" ) } if (!HasId.canBeNamed(this)) { From 412a8d13773a78fd16821c6fa8f144364a50985e Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 09:29:39 -0700 Subject: [PATCH 14/28] Arbiter restore chiselName --- src/main/scala/chisel3/util/Arbiter.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/scala/chisel3/util/Arbiter.scala b/src/main/scala/chisel3/util/Arbiter.scala index f503c4033fa..e098fc09bbd 100644 --- a/src/main/scala/chisel3/util/Arbiter.scala +++ b/src/main/scala/chisel3/util/Arbiter.scala @@ -132,6 +132,7 @@ 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)) From bbc0d0ae36cdef8e758d17b99a024345c552e0b2 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 09:31:08 -0700 Subject: [PATCH 15/28] restore nowarn annotation in Decoupled, that should be deleted in a standalone PR --- src/main/scala/chisel3/util/Decoupled.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/scala/chisel3/util/Decoupled.scala b/src/main/scala/chisel3/util/Decoupled.scala index 9c8097eeb1b..ede907fad89 100644 --- a/src/main/scala/chisel3/util/Decoupled.scala +++ b/src/main/scala/chisel3/util/Decoupled.scala @@ -344,6 +344,7 @@ object Queue { * consumer.io.in <> Queue(producer.io.out, 16) * }}} */ + @nowarn("cat=deprecation&msg=TransitName") @chiselName def apply[T <: Data]( enq: ReadyValidIO[T], From d7a1782db88f7105c09e07d41c5ab2c1e69fb87f Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 11:06:23 -0700 Subject: [PATCH 16/28] Update core/src/main/scala/chisel3/internal/Builder.scala Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index c30b368b2ec..e58d192a553 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -156,7 +156,7 @@ private[chisel3] trait HasId extends InstanceId { } if (!HasId.canBeNamed(this)) { Builder.deprecated( - s"Calling suggestName ($seed, on something that cannot actually be named: ${this}) will become an error in Chisel 3.6" + 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 From 898ae25d39ff1196cb71283035fe286664b89a9f Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 11:06:37 -0700 Subject: [PATCH 17/28] Update core/src/main/scala/chisel3/internal/Builder.scala Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index e58d192a553..f537111699f 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -161,7 +161,7 @@ private[chisel3] trait HasId extends InstanceId { } 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}) completed elaboration already will become an error in Chisel 3.6" + 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) From 5f25d909c109a53cb715368fcdf0b36ebba533d9 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 11:07:51 -0700 Subject: [PATCH 18/28] Update core/src/main/scala/chisel3/internal/Builder.scala Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index f537111699f..6c2433fc805 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -200,7 +200,7 @@ private[chisel3] trait HasId extends InstanceId { case (suggested, auto) => if (suggested == auto) { Builder.deprecated( - s"calling suggestName(${suggested}) had no effect as it is the same as the auto prefixed name, this will become an error in 3.6" + 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" ) } } From 5a119806ee11601100e5fcce9716a31b028cd30c Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 11:08:06 -0700 Subject: [PATCH 19/28] Update core/src/main/scala/chisel3/internal/Builder.scala Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 6c2433fc805..61108199fa5 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -292,7 +292,7 @@ private[chisel3] trait HasId extends InstanceId { } } -object HasId { +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 { From 0075c258341a1f5f3a78bc6b9f8681637156c0cf Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 11:20:28 -0700 Subject: [PATCH 20/28] Update src/test/scala/chiselTests/naming/SuggestNameSpec.scala --- src/test/scala/chiselTests/naming/SuggestNameSpec.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index e7c71d2c074..77872e7edfd 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -155,6 +155,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } /* + // This test is commented out until https://github.com/chipsalliance/chisel3/issues/2613 is resolved property("5d. Calling suggestName on an Instance instance should be allowed") { import chisel3.experimental.hierarchy.{Definition, Instance} import chiselTests.experimental.hierarchy.Examples.AddOne From db9152f492fc68ceeb66c687f1153848509d56f3 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 12:04:50 -0700 Subject: [PATCH 21/28] checkpoint: one test fails --- .../main/scala/chisel3/internal/Builder.scala | 12 +++--- .../chiselTests/naming/SuggestNameSpec.scala | 38 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 61108199fa5..dcf0fef7e17 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -148,20 +148,22 @@ private[chisel3] trait HasId extends InstanceId { * @return this object */ def suggestName(seed: => String): this.type = { - require(Builder.hasDynamicContext, s"suggestName (${seed}) should only be called from a Builder context.") + if (!Builder.hasDynamicContext){ + Builder.deprecated("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" + "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" + "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" + "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) @@ -200,7 +202,7 @@ private[chisel3] trait HasId extends InstanceId { 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" + "calling suggestName(\"" + suggested + "\") had no effect as it is the same as the automatically given name, this will become an error in 3.6" ) } } diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 77872e7edfd..91f8ee3a36e 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -26,11 +26,11 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) log should include( - "Calling suggestName (somethingElse, when already called with Some(mywire)) will become an error in Chisel 3.6" + "Calling suggestName(\"somethingElse\"), when already called with \"mywire\", will become an error in Chisel 3.6" ) } - property("1. Calling suggestName outside of a Builder context should be an error") { + property("1. Calling suggestName outside of a Builder context should be a runtime deprecation") { class Test extends Module { val wire = { val x = WireInit(0.U(3.W)) @@ -38,18 +38,17 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } } - val pm = new PhaseManager(Seq(Dependency[chisel3.stage.phases.Checks], Dependency[chisel3.stage.phases.Elaborate])) - val test = pm - .transform(Seq(ChiselGeneratorAnnotation(() => new Test()), NoRunFirrtlCompilerAnnotation)) - .collectFirst { - case d: DesignAnnotation[_] => d + + // Nasty use of var, only for this test purpose. Don't do stuff like this! + var test: Test = null + ChiselStage.elaborate { + test = new Test + test } - .get - .design - val caught = intercept[IllegalArgumentException] { - test.asInstanceOf[Test].wire.suggestName("somethingElse") + val (log, _) = grabLog{ +test.wire.suggestName("somethingElse") } - caught.getMessage should include("suggestName (somethingElse) should only be called from a Builder context") + log should include("suggestName(\"somethingElse\") should only be called from a Builder context") } property("2. Calling suggestName after module close should be a runtime deprecation") { @@ -65,8 +64,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) log should include( - "Calling suggestName (somethingElse, on Child.wire: Wire[UInt<3>], when the containing module (Child) completed elaboration already" - ) + "Calling suggestName(\"somethingElse\") on \"Child.wire: Wire[UInt<3>]\" when the containing module \"Child\" has already completed elaboration") } property("3. Calling suggestName with the same thing prefix would have given should be a runtime deprecation") { @@ -78,7 +76,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) log should include( - "calling suggestName(wire) had no effect as it is the same as the auto prefixed name" + "calling suggestName(\"wire\") had no effect as it is the same as the automatically given name" ) } @@ -189,7 +187,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { z.suggestName("fuzz") } val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) - log should include("Calling suggestName (fuzz, on something that cannot actually be named: chisel3.assert$Assert") + log should include("Calling suggestName(\"fuzz\") on \"chisel3.assert$Assert") (chirrtl should include).regex("assert.*: z") } @@ -202,7 +200,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { out := sum } val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) - log should include("Calling suggestName (fuzz, on something that cannot actually be named: UInt<1>(0)") + log should include("Calling suggestName(\"fuzz\") on \"UInt<1>(0)\" (which cannot actually be named)") chirrtl should include("out <= UInt") } @@ -217,8 +215,8 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { io.out := io.in } val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) - log should include("Calling suggestName (fuzz, on something that cannot actually be named: Example.io.in") - log should include("Calling suggestName (bar, on something that cannot actually be named: Example.io.out") + log should include("Calling suggestName(\"fuzz\") on \"Example.io.in: IO[UInt<8>]\" (which cannot actually be named)") + log should include("Calling suggestName(\"bar\") on \"Example.io.out: IO[UInt<8>]\" (which cannot actually be named)") chirrtl should include("io.out <= io.in") } @@ -232,7 +230,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { out := in } val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) - log should include("Calling suggestName (fuzz, on something that cannot actually be named: UInt<8>)") + log should include("Calling suggestName(\"fuzz\") on \"UInt<8>\" (which cannot actually be named)") chirrtl should include("out <= in") } } From 7cf0c42414eb38339cebb07c068fae7783f10123 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 12:05:13 -0700 Subject: [PATCH 22/28] scalafmt --- .../main/scala/chisel3/internal/Builder.scala | 6 ++-- .../chiselTests/naming/SuggestNameSpec.scala | 28 +++++++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index dcf0fef7e17..f306189414f 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -148,8 +148,8 @@ private[chisel3] trait HasId extends InstanceId { * @return this object */ def suggestName(seed: => String): this.type = { - if (!Builder.hasDynamicContext){ - Builder.deprecated("suggestName(\"" + seed + "\") should only be called from a Builder context.") + if (!Builder.hasDynamicContext) { + Builder.deprecated("suggestName(\"" + seed + "\") should only be called from a Builder context.") } if (suggested_seed.isDefined) { Builder.deprecated( @@ -158,7 +158,7 @@ private[chisel3] trait HasId extends InstanceId { } if (!HasId.canBeNamed(this)) { Builder.deprecated( - "Calling suggestName(\""+ seed +"\") on \"" + this + "\" (which cannot actually be named) will become an error in Chisel 3.6" + "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 diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 91f8ee3a36e..906cf006d18 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -38,15 +38,14 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } } - - // Nasty use of var, only for this test purpose. Don't do stuff like this! - var test: Test = null - ChiselStage.elaborate { - test = new Test - test - } - val (log, _) = grabLog{ -test.wire.suggestName("somethingElse") + // Nasty use of var, only for this test purpose. Don't do stuff like this! + var test: Test = null + ChiselStage.elaborate { + test = new Test + test + } + val (log, _) = grabLog { + test.wire.suggestName("somethingElse") } log should include("suggestName(\"somethingElse\") should only be called from a Builder context") } @@ -64,7 +63,8 @@ test.wire.suggestName("somethingElse") } val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) log should include( - "Calling suggestName(\"somethingElse\") on \"Child.wire: Wire[UInt<3>]\" when the containing module \"Child\" has already completed elaboration") + "Calling suggestName(\"somethingElse\") on \"Child.wire: Wire[UInt<3>]\" when the containing module \"Child\" has already completed elaboration" + ) } property("3. Calling suggestName with the same thing prefix would have given should be a runtime deprecation") { @@ -215,8 +215,12 @@ test.wire.suggestName("somethingElse") io.out := io.in } val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) - log should include("Calling suggestName(\"fuzz\") on \"Example.io.in: IO[UInt<8>]\" (which cannot actually be named)") - log should include("Calling suggestName(\"bar\") on \"Example.io.out: IO[UInt<8>]\" (which cannot actually be named)") + log should include( + "Calling suggestName(\"fuzz\") on \"Example.io.in: IO[UInt<8>]\" (which cannot actually be named)" + ) + log should include( + "Calling suggestName(\"bar\") on \"Example.io.out: IO[UInt<8>]\" (which cannot actually be named)" + ) chirrtl should include("io.out <= io.in") } From ce183890e765b985f0b67e8abe6fd7a3dc8a0c3f Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 14:26:27 -0700 Subject: [PATCH 23/28] fix numbering --- .../chiselTests/naming/SuggestNameSpec.scala | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index 906cf006d18..cf445229802 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -80,7 +80,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { ) } - property("5a. Calling suggestName on a node should be allowed") { + property("4a. Calling suggestName on a node should be allowed") { class Example extends Module { val foo, bar = IO(Input(UInt(8.W))) val out = IO(Output(UInt(8.W))) @@ -94,7 +94,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("node fuzz = add(foo, bar)") } - property("5a. Calling suggestName on an IO should be allowed") { + property("4b. Calling suggestName on an IO should be allowed") { class Example extends Module { val foo, bar = IO(Input(UInt(8.W))) val out = IO(Output(UInt(8.W))) @@ -109,7 +109,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("node sum = add(FOO, BAR)") } - property("5b. Calling suggestName on a prefixed node should be allowed") { + property("4c. Calling suggestName on a prefixed node should be allowed") { class Example extends Module { val foo, bar = IO(Input(UInt(8.W))) val out = IO(Output(UInt(8.W))) @@ -128,7 +128,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("node out_sum_fuzz = add(foo, bar)") } - property("5c. Calling suggestName on a Module instance should be allowed") { + property("4d. Calling suggestName on a Module instance should be allowed") { import chisel3.util._ class PassThrough extends Module { @@ -152,24 +152,9 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("inst fuzz of PassThrough") } - /* - // This test is commented out until https://github.com/chipsalliance/chisel3/issues/2613 is resolved - property("5d. Calling suggestName on an Instance instance should be allowed") { - 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) - inst.suggestName("fuzz") - val fuzz = inst - } - val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) - log should equal("") - chirrtl should include("inst fuzz of AddOne") - } - */ + - property("5e. Calling suggestName on a Mem should be allowed") { + property("4e. Calling suggestName on a Mem should be allowed") { class Example extends Module { val mem = SyncReadMem(8, UInt(8.W)) @@ -180,7 +165,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("smem fuzz") } - property("5f. Calling suggestName on a verif statement should be a runtime deprecation") { + property("4f. Calling suggestName on a verif statement should be a runtime deprecation") { class Example extends Module { val in = IO(Input(UInt(8.W))) val z = chisel3.assert(in =/= 123.U) @@ -191,7 +176,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { (chirrtl should include).regex("assert.*: z") } - property("5g. Calling suggestName on a literal should be a runtime deprecation") { + property("4g. Calling suggestName on a literal should be a runtime deprecation") { class Example extends Module { val out = IO(Output(UInt(8.W))) @@ -204,7 +189,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("out <= UInt") } - property("5h. Calling suggestName on a field of an Aggregate should be a runtime deprecation") { + property("4h. Calling suggestName on a field of an Aggregate should be a runtime deprecation") { class Example extends Module { val io = IO(new Bundle { val in = Input(UInt(8.W)) @@ -225,7 +210,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("io.out <= io.in") } - property("5i. Calling suggestName on unbound Data should be a runtime deprecation") { + property("4i. Calling suggestName on unbound Data should be a runtime deprecation") { class Example extends Module { val in = IO(Input(UInt(8.W))) val out = IO(Output(UInt(8.W))) @@ -237,4 +222,21 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { log should include("Calling suggestName(\"fuzz\") on \"UInt<8>\" (which cannot actually be named)") chirrtl should include("out <= in") } + + /* + // This test is commented out until https://github.com/chipsalliance/chisel3/issues/2366 is resolved + property("4j. Calling suggestName on an Instance instance should be allowed") { + 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) + inst.suggestName("fuzz") + val fuzz = inst + } + val (log, chirrtl) = grabLog(ChiselStage.emitChirrtl(new Example)) + log should equal("") + chirrtl should include("inst fuzz of AddOne") + } + */ } From 7158448728ee5bc9545a6b1f846ac3f9a50c7743 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 14:26:51 -0700 Subject: [PATCH 24/28] scalafmt --- src/test/scala/chiselTests/naming/SuggestNameSpec.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index cf445229802..a64f1f8d379 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -152,8 +152,6 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { chirrtl should include("inst fuzz of PassThrough") } - - property("4e. Calling suggestName on a Mem should be allowed") { class Example extends Module { val mem = SyncReadMem(8, UInt(8.W)) From e60b9bdad18f342a7e90359230f621a985d1f1bc Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 16:51:54 -0700 Subject: [PATCH 25/28] get the builder context check to work --- .../src/main/scala/chisel3/internal/Builder.scala | 15 ++++++++++++--- .../chiselTests/naming/SuggestNameSpec.scala | 2 +- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index f306189414f..227c1d9d4a9 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -149,7 +149,16 @@ private[chisel3] trait HasId extends InstanceId { */ def suggestName(seed: => String): this.type = { if (!Builder.hasDynamicContext) { - Builder.deprecated("suggestName(\"" + seed + "\") should only be called from a Builder context.") + // This is super hacky but this is just for a short term deprecation. + // This access is detected outside of Chisel elaboration so we cannot use the normal + // Builder.deprecated mechanism, we have to create our own one off ErrorLog and print the + // warning right away. + val errors = new ErrorLog + val logger = new _root_.logger.Logger(this.getClass.getName) + val msg = "suggestName(\"" + seed + "\") should only be called from a Builder context." + + "This will become an error in Chisel 3.6." + errors.deprecated(msg, None) + errors.checkpoint(logger) } if (suggested_seed.isDefined) { Builder.deprecated( @@ -202,8 +211,8 @@ private[chisel3] trait HasId extends InstanceId { case (suggested, auto) => if (suggested == auto) { Builder.deprecated( - "calling suggestName(\"" + suggested + "\") had no effect as it is the same as the automatically given name, this will become an error in 3.6" - ) + "calling suggestName(\"" + suggested + "\") on \"" + this._parent.get.name + '.' + suggested + "\" had no effect as it is the same as the automatically given name, this will become an error in 3.6", + Some("(unknown)")) } } suggested_seed.orElse(auto_seed) diff --git a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala index a64f1f8d379..f04ae6a8486 100644 --- a/src/test/scala/chiselTests/naming/SuggestNameSpec.scala +++ b/src/test/scala/chiselTests/naming/SuggestNameSpec.scala @@ -76,7 +76,7 @@ class SuggestNameSpec extends ChiselPropSpec with Utils { } val (log, _) = grabLog(ChiselStage.emitVerilog(new Test())) log should include( - "calling suggestName(\"wire\") had no effect as it is the same as the automatically given name" + "calling suggestName(\"wire\") on \"Test.wire\" had no effect as it is the same as the automatically given name" ) } From 9772b29dbfd358ea346565c3b52525c2adf4d6dc Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 16:52:31 -0700 Subject: [PATCH 26/28] scalafmt --- core/src/main/scala/chisel3/internal/Builder.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 227c1d9d4a9..7d46f3ca420 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -212,7 +212,8 @@ private[chisel3] trait HasId extends InstanceId { if (suggested == auto) { Builder.deprecated( "calling suggestName(\"" + suggested + "\") on \"" + this._parent.get.name + '.' + suggested + "\" had no effect as it is the same as the automatically given name, this will become an error in 3.6", - Some("(unknown)")) + Some("(unknown)") + ) } } suggested_seed.orElse(auto_seed) From 67ce7d82cf51f0dc13a0d25ce321ffd615a0c8c7 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 16:55:21 -0700 Subject: [PATCH 27/28] avoid any allocations in seedOpt check --- .../src/main/scala/chisel3/internal/Builder.scala | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 7d46f3ca420..57da96201dc 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -207,14 +207,13 @@ private[chisel3] trait HasId extends InstanceId { * @return the current calculation of a name, if it exists */ private[chisel3] def seedOpt: Option[String] = { - suggested_seed.zip(auto_seed).foreach { - case (suggested, auto) => - if (suggested == auto) { - Builder.deprecated( - "calling suggestName(\"" + suggested + "\") on \"" + this._parent.get.name + '.' + suggested + "\" had no effect as it is the same as the automatically given name, this will become an error in 3.6", - Some("(unknown)") - ) - } + if (suggested_seed.isDefined && auto_seed.isDefined) { + if (suggested_seed.get == auto_seed.get) { + Builder.deprecated( + "calling suggestName(\"" + suggested_seed.get + "\") on \"" + this._parent.get.name + '.' + suggested_seed.get + "\" had no effect as it is the same as the automatically given name, this will become an error in 3.6", + Some("(unknown)") + ) + } } suggested_seed.orElse(auto_seed) } From 47d88ce27dd673528e50cd78f3d731123b763252 Mon Sep 17 00:00:00 2001 From: Megan Wachs Date: Fri, 1 Jul 2022 20:16:27 -0700 Subject: [PATCH 28/28] Update core/src/main/scala/chisel3/internal/Builder.scala --- core/src/main/scala/chisel3/internal/Builder.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 57da96201dc..bd437ceab2b 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -322,7 +322,7 @@ private[chisel3] object HasId { } } -/** Holds the implementation of to for Data and MemBase */ +/** Holds the implementation of toNamed for Data and MemBase */ private[chisel3] trait NamedComponent extends HasId { /** Returns a FIRRTL ComponentName that references this object