diff options
19 files changed, 128 insertions, 543 deletions
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5f3252cc..ce18fab0 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,7 +47,7 @@ jobs: if: startsWith(matrix.scala, '2.12') run: sbt ++${{ matrix.scala }} docs/mdoc - name: Test - run: sbt ++${{ matrix.scala }} test noPluginTests/test + run: sbt ++${{ matrix.scala }} test - name: Binary compatibility run: sbt ++${{ matrix.scala }} mimaReportBinaryIssues @@ -189,13 +189,6 @@ If the compilation succeeded and the dependencies noted above are installed, you sbt test ``` -If you would like to run the tests without the compiler plugin (less common), you can do so by first launching `sbt`, -then running `noPluginTests / test`: -``` -sbt -> noPluginTests / test -``` - ### Running Projects Against Local Chisel To use the development version of Chisel (`master` branch), you will need to build from source and `publishLocal`. @@ -221,15 +221,6 @@ lazy val chisel = (project in file(".")). ) ) -lazy val noPluginTests = (project in file ("no-plugin-tests")). - dependsOn(chisel). - settings(commonSettings: _*). - settings(chiselSettings: _*). - settings(Seq( - // Totally don't know why GitHub Action won't introduce FIRRTL to dependency. - libraryDependencies += defaultVersions("firrtl"), - )) - // tests elaborating and executing/formally verifying a Chisel circuit with chiseltest lazy val integrationTests = (project in file ("integration-tests")). dependsOn(chisel). diff --git a/core/src/main/scala/chisel3/Aggregate.scala b/core/src/main/scala/chisel3/Aggregate.scala index 4cf427ff..7c7db1c6 100644 --- a/core/src/main/scala/chisel3/Aggregate.scala +++ b/core/src/main/scala/chisel3/Aggregate.scala @@ -1033,6 +1033,10 @@ package experimental { * }}} */ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record { + assert(_usingPlugin, "The Chisel compiler plugin is now required for compiling Chisel code. " + + "Please see https://github.com/chipsalliance/chisel3#build-your-own-chisel-projects." + ) + override def className: String = this.getClass.getSimpleName match { case name if name.startsWith("$anon$") => "AnonymousBundle" // fallback for anonymous Bundle case case "" => "AnonymousBundle" // ditto, but on other platforms @@ -1109,16 +1113,6 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record { */ protected def _usingPlugin: Boolean = false - // Memoize the outer instance for autoclonetype, especially where this is context-dependent - // (like the outer module or enclosing Bundles). - private var _outerInst: Option[Object] = None - - // For reflective autoclonetype, record possible candidates for outer instance. - // _outerInst should always take precedence, since it should be propagated from the original - // object which has the most accurate context. - private val _containingModule: Option[BaseModule] = if (_usingPlugin) None else Builder.currentModule - private val _containingBundles: Seq[Bundle] = if (_usingPlugin) Nil else Builder.updateBundleStack(this) - private def checkClone(clone: Bundle): Unit = { for ((name, field) <- elements) { if (clone.elements(name) eq field) { @@ -1142,224 +1136,10 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record { /** Implementation of cloneType using runtime reflection. This should _never_ be overridden or called in user-code * - * @note This is overridden by the compiler plugin (it is never called when using the plugin) + * @note This is overridden by the compiler plugin (this implementation is never called) */ protected def _cloneTypeImpl: Bundle = { - assert(Builder.allowReflectiveAutoCloneType, "reflective autoclonetype is disallowed, this should only happen in testing") - Builder.deprecated( - "The runtime reflection inference for cloneType (_cloneTypeImpl) is deprecated as of Chisel 3.5: " + - "Use the Compiler Plugin to infer cloneType" - ) - // This attempts to infer constructor and arguments to clone this Bundle subtype without - // requiring the user explicitly overriding cloneType. - import scala.language.existentials - import scala.reflect.runtime.universe._ - - val clazz = this.getClass - - def autoClonetypeError(desc: String): Nothing = - throw new AutoClonetypeException( - s"Unable to automatically infer cloneType on $clazz. " + - "cloneType is now implemented by the Chisel compiler plugin so please ensure you are using it in your build. " + - "If you cannot use the compiler plugin or you are using it and you still see this message, please file an issue and let us know. " + - s"For those not using the plugin, here is the 'runtime reflection' cloneType error message: $desc" - ) - - def validateClone(clone: Bundle, equivDiagnostic: String): Unit = { - if (!clone.typeEquivalent(this)) { - autoClonetypeError(s"Automatically cloned $clone not type-equivalent to base $this. " + equivDiagnostic) - } - checkClone(clone) - } - - val mirror = runtimeMirror(clazz.getClassLoader) - - val classSymbolOption = try { - Some(mirror.reflect(this).symbol) - } catch { - case e: scala.reflect.internal.Symbols#CyclicReference => None // Workaround for a scala bug - } - - val enclosingClassOption = (clazz.getEnclosingClass, classSymbolOption) match { - case (null, _) => None - case (_, Some(classSymbol)) if classSymbol.isStatic => None // allows support for members of companion objects - case (outerClass, _) => Some(outerClass) - } - - // For compatibility with pre-3.1, where null is tried as an argument to the constructor. - // This stores potential error messages which may be used later. - var outerClassError: Option[String] = None - - // Check if this is an inner class, and if so, try to get the outer instance - val outerClassInstance = enclosingClassOption.map { outerClass => - def canAssignOuterClass(x: Object) = outerClass.isAssignableFrom(x.getClass) - - val outerInstance = _outerInst match { - case Some(outerInstance) => outerInstance // use _outerInst if defined - case None => // determine outer instance if not already recorded - try { - // Prefer this if it works, but doesn't work in all cases, namely anonymous inner Bundles - val outer = clazz.getDeclaredField("$outer").get(this) - _outerInst = Some(outer) - outer - } catch { - case (_: NoSuchFieldException | _: IllegalAccessException) => - // Fallback using guesses based on common patterns - val allOuterCandidates = Seq( - _containingModule.toSeq, - _containingBundles - ).flatten.distinct - allOuterCandidates.filter(canAssignOuterClass(_)) match { - case outer :: Nil => - _outerInst = Some(outer) // record the guess for future use - outer - case Nil => // TODO: replace with fatal autoClonetypeError once compatibility period is dropped - outerClassError = Some(s"Unable to determine instance of outer class $outerClass," + - s" no candidates assignable to outer class types; examined $allOuterCandidates") - null - case candidates => // TODO: replace with fatal autoClonetypeError once compatibility period is dropped - outerClassError = Some(s"Unable to determine instance of outer class $outerClass," + - s" multiple possible candidates $candidates assignable to outer class type") - null - } - } - } - (outerClass, outerInstance) - } - - // If possible (constructor with no arguments), try Java reflection first - // This handles two cases that Scala reflection doesn't: - // 1. getting the ClassSymbol of a class with an anonymous outer class fails with a - // CyclicReference exception - // 2. invoking the constructor of an anonymous inner class seems broken (it expects the outer - // class as an argument, but fails because the number of arguments passed in is incorrect) - if (clazz.getConstructors.size == 1) { - var ctor = clazz.getConstructors.head - val argTypes = ctor.getParameterTypes.toList - val clone = (argTypes, outerClassInstance) match { - case (Nil, None) => // no arguments, no outer class, invoke constructor directly - Some(ctor.newInstance().asInstanceOf[this.type]) - case (argType :: Nil, Some((_, outerInstance))) => - if (outerInstance == null) { - Builder.deprecated(s"chisel3.1 autoclonetype failed, falling back to 3.0 behavior using null as the outer instance." + - s" Autoclonetype failure reason: ${outerClassError.get}", - Some(s"$clazz")) - Some(ctor.newInstance(outerInstance).asInstanceOf[this.type]) - } else if (argType isAssignableFrom outerInstance.getClass) { - Some(ctor.newInstance(outerInstance).asInstanceOf[this.type]) - } else { - None - } - case _ => None - - } - clone match { - case Some(clone) => - clone._outerInst = this._outerInst - validateClone(clone, "Constructor argument values were not inferred, ensure constructor is deterministic.") - return clone.asInstanceOf[this.type] - case None => - } - } - - // Get constructor parameters and accessible fields - val classSymbol = classSymbolOption.getOrElse(autoClonetypeError(s"scala reflection failed." + - " This is known to occur with inner classes on anonymous outer classes." + - " In those cases, autoclonetype only works with no-argument constructors, or you can define a custom cloneType.")) - - val decls = classSymbol.typeSignature.decls - val ctors = decls.collect { case meth: MethodSymbol if meth.isConstructor => meth } - if (ctors.size != 1) { - autoClonetypeError(s"found multiple constructors ($ctors)." + - " Either remove all but the default constructor, or define a custom cloneType method.") - } - val ctor = ctors.head - val ctorParamss = ctor.paramLists - val ctorParams = ctorParamss match { - case Nil => List() - case ctorParams :: Nil => ctorParams - case ctorParams :: ctorImplicits :: Nil => ctorParams ++ ctorImplicits - case _ => autoClonetypeError(s"internal error, unexpected ctorParamss = $ctorParamss") - } - val ctorParamsNames = ctorParams.map(_.name.toString) - - // Special case for anonymous inner classes: their constructor consists of just the outer class reference - // Scala reflection on anonymous inner class constructors seems broken - if (ctorParams.size == 1 && outerClassInstance.isDefined && - ctorParams.head.typeSignature == mirror.classSymbol(outerClassInstance.get._1).toType) { - // Fall back onto Java reflection - val ctors = clazz.getConstructors - require(ctors.size == 1) // should be consistent with Scala constructors - try { - val clone = ctors.head.newInstance(outerClassInstance.get._2).asInstanceOf[this.type] - clone._outerInst = this._outerInst - - validateClone(clone, "Outer class instance was inferred, ensure constructor is deterministic.") - return clone - } catch { - case e @ (_: java.lang.reflect.InvocationTargetException | _: IllegalArgumentException) => - autoClonetypeError(s"unexpected failure at constructor invocation, got $e.") - } - } - - // Get all the class symbols up to (but not including) Bundle and get all the accessors. - // (each ClassSymbol's decls only includes those declared in the class itself) - val bundleClassSymbol = mirror.classSymbol(classOf[Bundle]) - val superClassSymbols = classSymbol.baseClasses.takeWhile(_ != bundleClassSymbol) - val superClassDecls = superClassSymbols.map(_.typeSignature.decls).flatten - val accessors = superClassDecls.collect { case meth: MethodSymbol if meth.isParamAccessor => meth } - - // Get constructor argument values - // Check that all ctor params are immutable and accessible. Immutability is required to avoid - // potential subtle bugs (like values changing after cloning). - // This also generates better error messages (all missing elements shown at once) instead of - // failing at the use site one at a time. - val accessorsName = accessors.filter(_.isStable).map(_.name.toString) - val paramsDiff = ctorParamsNames.toSet -- accessorsName.toSet - if (!paramsDiff.isEmpty) { - autoClonetypeError(s"constructor has parameters (${paramsDiff.toList.sorted.mkString(", ")}) that are not both immutable and accessible." + - " Either make all parameters immutable and accessible (vals) so cloneType can be inferred, or define a custom cloneType method.") - } - - // Get all the argument values - val accessorsMap = accessors.map(accessor => accessor.name.toString -> accessor).toMap - val instanceReflect = mirror.reflect(this) - val ctorParamsNameVals = ctorParamsNames.map { - paramName => paramName -> instanceReflect.reflectMethod(accessorsMap(paramName)).apply() - } - - // Opportunistic sanity check: ensure any arguments of type Data is not bound - // (which could lead to data conflicts, since it's likely the user didn't know to re-bind them). - // This is not guaranteed to catch all cases (for example, Data in Tuples or Iterables). - val boundDataParamNames = ctorParamsNameVals.collect { - case (paramName, paramVal: Data) if paramVal.topBindingOpt.isDefined => paramName - } - if (boundDataParamNames.nonEmpty) { - autoClonetypeError(s"constructor parameters (${boundDataParamNames.sorted.mkString(", ")}) have values that are hardware types, which is likely to cause subtle errors." + - " Use chisel types instead: use the value before it is turned to a hardware type (with Wire(...), Reg(...), etc) or use chiselTypeOf(...) to extract the chisel type.") - } - - // Clone unbound parameters in case they are being used as bundle fields. - val ctorParamsVals = ctorParamsNameVals.map { - case (_, paramVal: Data) => paramVal.cloneTypeFull - case (_, paramVal) => paramVal - } - - // Invoke ctor - val classMirror = outerClassInstance match { - case Some((_, null)) => autoClonetypeError(outerClassError.get) // deals with the null hack for 3.0 compatibility - case Some((_, outerInstance)) => mirror.reflect(outerInstance).reflectClass(classSymbol) - case _ => mirror.reflectClass(classSymbol) - } - val clone = classMirror.reflectConstructor(ctor).apply(ctorParamsVals:_*).asInstanceOf[this.type] - clone._outerInst = this._outerInst - - validateClone(clone, - "Constructor argument values were inferred:" + - " ensure that variable names are consistent and have the same value throughout the constructor chain," + - " and that the constructor is deterministic." - ) - clone + throwException(s"Internal Error! This should have been implemented by the chisel3-plugin. Please file an issue against chisel3") } /** Default "pretty-print" implementation diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala index 966e60d6..71894887 100644 --- a/core/src/main/scala/chisel3/internal/Builder.scala +++ b/core/src/main/scala/chisel3/internal/Builder.scala @@ -343,9 +343,6 @@ private[chisel3] trait NamedComponent extends HasId { private[chisel3] class ChiselContext() { val idGen = new IdGen - // Record the Bundle instance, class name, method name, and reverse stack trace position of open Bundles - val bundleStack: ArrayBuffer[(Bundle, String, String, Int)] = ArrayBuffer() - // Records the different prefixes which have been scoped at this point in time var prefixStack: Prefix = Nil @@ -360,8 +357,6 @@ private[chisel3] class DynamicContext(val annotationSeq: AnnotationSeq) { val components = ArrayBuffer[Component]() val annotations = ArrayBuffer[ChiselAnnotation]() var currentModule: Option[BaseModule] = None - // This is only used for testing, it can be removed if the plugin becomes mandatory - var allowReflectiveAutoCloneType = true /** Contains a mapping from a elaborated module to their aspect * Set by [[ModuleAspect]] @@ -603,16 +598,6 @@ private[chisel3] object Builder extends LazyLogging { .getOrElse(false) } - // This should only be used for testing, must be true outside of Builder context - def allowReflectiveAutoCloneType: Boolean = { - dynamicContextVar.value - .map(_.allowReflectiveAutoCloneType) - .getOrElse(true) - } - def allowReflectiveAutoCloneType_=(value: Boolean): Unit = { - dynamicContext.allowReflectiveAutoCloneType = value - } - def forcedClock: Clock = currentClock.getOrElse( throwException("Error: No implicit clock.") ) @@ -632,40 +617,6 @@ private[chisel3] object Builder extends LazyLogging { pushCommand(cmd).id } - // Called when Bundle construction begins, used to record a stack of open Bundle constructors to - // record candidates for Bundle autoclonetype. This is a best-effort guess. - // Returns the current stack of open Bundles - // Note: elt will NOT have finished construction, its elements cannot be accessed - def updateBundleStack(elt: Bundle): Seq[Bundle] = { - val stackElts = Thread.currentThread().getStackTrace() - .reverse // so stack frame numbers are deterministic across calls - .dropRight(2) // discard Thread.getStackTrace and updateBundleStack - - // Determine where we are in the Bundle stack - val eltClassName = elt.getClass.getName - val eltStackPos = stackElts.map(_.getClassName).lastIndexOf(eltClassName) - - // Prune the existing Bundle stack of closed Bundles - // If we know where we are in the stack, discard frames above that - val stackEltsTop = if (eltStackPos >= 0) eltStackPos else stackElts.size - val pruneLength = chiselContext.get.bundleStack.reverse.prefixLength { case (_, cname, mname, pos) => - pos >= stackEltsTop || stackElts(pos).getClassName != cname || stackElts(pos).getMethodName != mname - } - chiselContext.get.bundleStack.trimEnd(pruneLength) - - // Return the stack state before adding the most recent bundle - val lastStack = chiselContext.get.bundleStack.map(_._1).toSeq - - // Append the current Bundle to the stack, if it's on the stack trace - if (eltStackPos >= 0) { - val stackElt = stackElts(eltStackPos) - chiselContext.get.bundleStack.append((elt, eltClassName, stackElt.getMethodName, eltStackPos)) - } - // Otherwise discard the stack frame, this shouldn't fail noisily - - lastStack - } - /** Recursively suggests names to supported "container" classes * Arbitrary nestings of supported classes are allowed so long as the * innermost element is of type HasId diff --git a/no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala b/no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala deleted file mode 120000 index f3ebcbce..00000000 --- a/no-plugin-tests/src/test/scala/chisel3/testers/TestUtils.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../../src/test/scala/chisel3/testers/TestUtils.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/AutoClonetypeSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/AutoClonetypeSpec.scala deleted file mode 120000 index ae3a7597..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/AutoClonetypeSpec.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/AutoClonetypeSpec.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/AutoNestedCloneSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/AutoNestedCloneSpec.scala deleted file mode 120000 index 4ff3aa4f..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/AutoNestedCloneSpec.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/AutoNestedCloneSpec.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/BetterNamingTests.scala b/no-plugin-tests/src/test/scala/chiselTests/BetterNamingTests.scala deleted file mode 120000 index 0d362f3c..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/BetterNamingTests.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/BetterNamingTests.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/CatSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/CatSpec.scala deleted file mode 120000 index 2fb2d6a7..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/CatSpec.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/util/CatSpec.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala deleted file mode 120000 index 9b257eeb..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/ChiselSpec.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/ChiselSpec.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/CustomBundle.scala b/no-plugin-tests/src/test/scala/chiselTests/CustomBundle.scala deleted file mode 120000 index 006a2dbd..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/CustomBundle.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/CustomBundle.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala deleted file mode 120000 index e8eca65f..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/InstanceNameSpec.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/InstanceNameSpec.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala deleted file mode 100644 index 28673495..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala +++ /dev/null @@ -1,55 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -package chiselTests -import Chisel.ChiselException -import chisel3.stage.ChiselStage -import org.scalatest._ -import org.scalatest.matchers.should.Matchers - -class MissingCloneBindingExceptionSpec extends ChiselFlatSpec with Matchers with Utils { - behavior of "missing cloneType in Chisel3" - ( the [ChiselException] thrownBy extractCause[ChiselException] { - import chisel3._ - - class Test extends Module { - class TestIO(w: Int) extends Bundle { - val a = Input(Vec(4, UInt(w.W))) - } - - val io = IO(new TestIO(32)) - } - - class TestTop extends Module { - val io = IO(new Bundle {}) - - val subs = VecInit(Seq.fill(2) { - Module(new Test).io - }) - } - - ChiselStage.elaborate(new TestTop) - }).getMessage should include("make all parameters immutable") - - behavior of "missing cloneType in Chisel2" - ( the [ChiselException] thrownBy extractCause[ChiselException] { - import Chisel._ - - class Test extends Module { - class TestIO(w: Int) extends Bundle { - val a = Vec(4, UInt(width = w)).asInput - } - - val io = IO(new TestIO(32)) - } - - class TestTop extends Module { - val io = IO(new Bundle {}) - - val subs = Vec.fill(2) { - Module(new Test).io - } - } - - ChiselStage.elaborate(new TestTop) - }).getMessage should include("make all parameters immutable") -} diff --git a/no-plugin-tests/src/test/scala/chiselTests/NamingAnnotationTest.scala b/no-plugin-tests/src/test/scala/chiselTests/NamingAnnotationTest.scala deleted file mode 120000 index 6933a290..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/NamingAnnotationTest.scala +++ /dev/null @@ -1 +0,0 @@ -../../../../../src/test/scala/chiselTests/NamingAnnotationTest.scala
\ No newline at end of file diff --git a/no-plugin-tests/src/test/scala/chiselTests/NoPluginBundleSpec.scala b/no-plugin-tests/src/test/scala/chiselTests/NoPluginBundleSpec.scala deleted file mode 100644 index b73be483..00000000 --- a/no-plugin-tests/src/test/scala/chiselTests/NoPluginBundleSpec.scala +++ /dev/null @@ -1,36 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 - -package chiselTests - -import chisel3._ -import chisel3.stage.ChiselStage -import chisel3.testers.BasicTester - - - -trait BundleSpecUtils { - - class BundleBaz(val w: Int) extends Bundle { - val baz = UInt(w.W) - // (if we don't have the val on the val w: Int then it is an Exception) - // Check that we get a runtime deprecation warning if we don't have this: - // override def cloneType = (new BundleBaz(w)).asInstanceOf[this.type] - } - -} - -class NoPluginBundleSpec extends ChiselFlatSpec with BundleSpecUtils with Utils { - - "No override def cloneType" should "give a runtime deprecation warning without compiler plugin" in { - class MyModule extends MultiIOModule { - val in = IO(Input(new BundleBaz(w = 3))) - val out = IO(Output(in.cloneType)) - } - val (log, _) = grabLog( - ChiselStage.elaborate(new MyModule()) - ) - log should include ("warn") - log should include ("deprecated") - log should include ("The runtime reflection inference for cloneType") - } -} diff --git a/src/test/scala/chisel3/testers/TestUtils.scala b/src/test/scala/chisel3/testers/TestUtils.scala index c72c779a..338f9cd4 100644 --- a/src/test/scala/chisel3/testers/TestUtils.scala +++ b/src/test/scala/chisel3/testers/TestUtils.scala @@ -12,13 +12,4 @@ object TestUtils { // Useful because TesterDriver.Backend is chisel3 package private def containsBackend(annos: AnnotationSeq): Boolean = annos.collectFirst { case b: Backend => b }.isDefined - - // Allows us to check that the compiler plugin cloneType is actually working - val usingPlugin: Boolean = (new Bundle { def check = _usingPlugin }).check - def elaborateNoReflectiveAutoCloneType(f: => RawModule): Circuit = { - ChiselStage.elaborate { - chisel3.internal.Builder.allowReflectiveAutoCloneType = !usingPlugin - f - } - } } diff --git a/src/test/scala/chiselTests/AutoClonetypeSpec.scala b/src/test/scala/chiselTests/AutoClonetypeSpec.scala index 3f33fda8..ef58f1ed 100644 --- a/src/test/scala/chiselTests/AutoClonetypeSpec.scala +++ b/src/test/scala/chiselTests/AutoClonetypeSpec.scala @@ -5,6 +5,7 @@ package chiselTests import chisel3._ import chisel3.testers.TestUtils import chisel3.util.QueueIO +import chisel3.stage.ChiselStage.elaborate class BundleWithIntArg(val i: Int) extends Bundle { val out = UInt(i.W) @@ -71,14 +72,11 @@ class InheritingBundle extends QueueIO(UInt(8.W), 8) { val error = Output(Bool()) } -// TODO all `.suggestNames` are due to https://github.com/chipsalliance/chisel3/issues/1802 class AutoClonetypeSpec extends ChiselFlatSpec with Utils { - val usingPlugin: Boolean = TestUtils.usingPlugin - val elaborate = TestUtils.elaborateNoReflectiveAutoCloneType _ "Bundles with Scala args" should "not need clonetype" in { elaborate { new Module { - val io = IO(new Bundle{}).suggestName("io") + val io = IO(new Bundle{}) val myWire = Wire(new BundleWithIntArg(8)) assert(myWire.i == 8) @@ -87,7 +85,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { "Bundles with Scala implicit args" should "not need clonetype" in { elaborate { new Module { - val io = IO(new Bundle{}).suggestName("io") + val io = IO(new Bundle{}) implicit val implicitInt: Int = 4 val myWire = Wire(new BundleWithImplicit()) @@ -98,7 +96,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { "Bundles with Scala explicit and impicit args" should "not need clonetype" in { elaborate { new Module { - val io = IO(new Bundle{}).suggestName("io") + val io = IO(new Bundle{}) implicit val implicitInt: Int = 4 val myWire = Wire(new BundleWithArgAndImplicit(8)) @@ -110,7 +108,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { "Subtyped Bundles" should "not need clonetype" in { elaborate { new Module { - val io = IO(new Bundle{}).suggestName("io") + val io = IO(new Bundle{}) val myWire = Wire(new SubBundle(8, 4)) @@ -118,7 +116,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { assert(myWire.i2 == 4) } } elaborate { new Module { - val io = IO(new Bundle{}).suggestName("io") + val io = IO(new Bundle{}) val myWire = Wire(new SubBundleVal(8, 4)) @@ -131,23 +129,12 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { new BundleWithIntArg(8).cloneType } - def checkSubBundleInvalid() = { + "Subtyped Bundles that don't clone well" should "be now be supported!" in { elaborate { new Module { - val io = IO(new Bundle{}).suggestName("io") + val io = IO(new Bundle{}) val myWire = Wire(new SubBundleInvalid(8, 4)) } } } - if (usingPlugin) { - "Subtyped Bundles that don't clone well" should "be now be supported!" in { - checkSubBundleInvalid() - } - } else { - "Subtyped Bundles that don't clone well" should "be caught" in { - a [ChiselException] should be thrownBy extractCause[ChiselException] { - checkSubBundleInvalid() - } - } - } "Inner bundles with Scala args" should "not need clonetype" in { elaborate { new ModuleWithInner } @@ -155,7 +142,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { "Bundles with arguments as fields" should "not need clonetype" in { elaborate { new Module { - val io = IO(Output(new BundleWithArgumentField(UInt(8.W), UInt(8.W)))).suggestName("io") + val io = IO(Output(new BundleWithArgumentField(UInt(8.W), UInt(8.W)))) io.x := 1.U io.y := 1.U } } @@ -163,28 +150,28 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { it should "also work when giving directions to the fields" in { elaborate { new Module { - val io = IO(new BundleWithArgumentField(Input(UInt(8.W)), Output(UInt(8.W)))).suggestName("io") + val io = IO(new BundleWithArgumentField(Input(UInt(8.W)), Output(UInt(8.W)))) io.y := io.x } } } "Bundles inside companion objects" should "not need clonetype" in { elaborate { new Module { - val io = IO(Output(new CompanionObjectWithBundle.Inner)).suggestName("io") + val io = IO(Output(new CompanionObjectWithBundle.Inner)) io.data := 1.U } } } "Parameterized bundles inside companion objects" should "not need clonetype" in { elaborate { new Module { - val io = IO(Output(new CompanionObjectWithBundle.ParameterizedInner(8))).suggestName("io") + val io = IO(Output(new CompanionObjectWithBundle.ParameterizedInner(8))) io.data := 1.U } } } "Nested directioned anonymous Bundles" should "not need clonetype" in { elaborate { new Module { - val io = IO(new NestedAnonymousBundle).suggestName("io") + val io = IO(new NestedAnonymousBundle) val a = WireDefault(io) io.a.a := 1.U } } @@ -197,7 +184,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { val a = Output(UInt(8.W)) } } - val io = IO((new InnerClassThing).createBundle).suggestName("io") + val io = IO((new InnerClassThing).createBundle) val a = WireDefault(io) } } } @@ -208,7 +195,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { val bundleFieldType = UInt(8.W) val io = IO(Output(new Bundle { val a = bundleFieldType - })).suggestName("io") + })) io.a := 0.U } } } @@ -221,7 +208,7 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { } elaborate { new Module { - val io = IO(Output(new BadBundle(UInt(8.W), 1))).suggestName("io") + val io = IO(Output(new BadBundle(UInt(8.W), 1))) io.a := 0.U } } } @@ -265,100 +252,97 @@ class AutoClonetypeSpec extends ChiselFlatSpec with Utils { behavior of "Compiler Plugin Autoclonetype" - // New tests from the plugin - if (usingPlugin) { - it should "NOT break code that extends chisel3.util Bundles if they use the plugin" in { - class MyModule extends MultiIOModule { - val io = IO(new InheritingBundle) - io.deq <> io.enq - io.count := 0.U - io.error := true.B - } - elaborate(new MyModule) + it should "NOT break code that extends chisel3.util Bundles if they use the plugin" in { + class MyModule extends MultiIOModule { + val io = IO(new InheritingBundle) + io.deq <> io.enq + io.count := 0.U + io.error := true.B } + elaborate(new MyModule) + } - it should "support Bundles with non-val parameters" in { - class MyBundle(i: Int) extends Bundle { - val foo = UInt(i.W) - } - elaborate { new MultiIOModule { - val in = IO(Input(new MyBundle(8))) - val out = IO(Output(new MyBundle(8))) - out := in - }} + it should "support Bundles with non-val parameters" in { + class MyBundle(i: Int) extends Bundle { + val foo = UInt(i.W) } + elaborate { new MultiIOModule { + val in = IO(Input(new MyBundle(8))) + val out = IO(Output(new MyBundle(8))) + out := in + }} + } - it should "support type-parameterized Bundles" in { - class MyBundle[T <: Data](gen: T) extends Bundle { - val foo = gen - } - elaborate { new MultiIOModule { - val in = IO(Input(new MyBundle(UInt(8.W)))) - val out = IO(Output(new MyBundle(UInt(8.W)))) - out := in - }} + it should "support type-parameterized Bundles" in { + class MyBundle[T <: Data](gen: T) extends Bundle { + val foo = gen } + elaborate { new MultiIOModule { + val in = IO(Input(new MyBundle(UInt(8.W)))) + val out = IO(Output(new MyBundle(UInt(8.W)))) + out := in + }} + } - it should "support Bundles with non-val implicit parameters" in { - class MyBundle(implicit i: Int) extends Bundle { - val foo = UInt(i.W) - } - elaborate { new MultiIOModule { - implicit val x = 8 - val in = IO(Input(new MyBundle)) - val out = IO(Output(new MyBundle)) - out := in - }} + it should "support Bundles with non-val implicit parameters" in { + class MyBundle(implicit i: Int) extends Bundle { + val foo = UInt(i.W) } + elaborate { new MultiIOModule { + implicit val x = 8 + val in = IO(Input(new MyBundle)) + val out = IO(Output(new MyBundle)) + out := in + }} + } - it should "support Bundles with multiple parameter lists" in { - class MyBundle(i: Int)(j: Int, jj: Int)(k: UInt) extends Bundle { - val foo = UInt((i + j + jj + k.getWidth).W) - } - elaborate { - new MultiIOModule { - val in = IO(Input(new MyBundle(8)(8, 8)(UInt(8.W)))) - val out = IO(Output(new MyBundle(8)(8, 8)(UInt(8.W)))) - out := in - } + it should "support Bundles with multiple parameter lists" in { + class MyBundle(i: Int)(j: Int, jj: Int)(k: UInt) extends Bundle { + val foo = UInt((i + j + jj + k.getWidth).W) + } + elaborate { + new MultiIOModule { + val in = IO(Input(new MyBundle(8)(8, 8)(UInt(8.W)))) + val out = IO(Output(new MyBundle(8)(8, 8)(UInt(8.W)))) + out := in } } + } - it should "support Bundles that implement their own cloneType" in { - class MyBundle(i: Int) extends Bundle { - val foo = UInt(i.W) - } - elaborate { new MultiIOModule { - val in = IO(Input(new MyBundle(8))) - val out = IO(Output(new MyBundle(8))) - out := in - }} + it should "support Bundles that implement their own cloneType" in { + class MyBundle(i: Int) extends Bundle { + val foo = UInt(i.W) } + elaborate { new MultiIOModule { + val in = IO(Input(new MyBundle(8))) + val out = IO(Output(new MyBundle(8))) + out := in + }} + } - it should "support Bundles that capture type parameters from their parent scope" in { - class MyModule[T <: Data](gen: T) extends MultiIOModule { - class MyBundle(n: Int) extends Bundle { - val foo = Vec(n, gen) - } - val in = IO(Input(new MyBundle(4))) - val out = IO(Output(new MyBundle(4))) - out := in + it should "support Bundles that capture type parameters from their parent scope" in { + class MyModule[T <: Data](gen: T) extends MultiIOModule { + class MyBundle(n: Int) extends Bundle { + val foo = Vec(n, gen) } - elaborate(new MyModule(UInt(8.W))) + val in = IO(Input(new MyBundle(4))) + val out = IO(Output(new MyBundle(4))) + out := in } + elaborate(new MyModule(UInt(8.W))) + } - it should "work for higher-kinded types" in { - class DataGen[T <: Data](gen: T) { - def newType: T = gen.cloneType - } - class MyBundle[A <: Data, B <: DataGen[A]](gen: B) extends Bundle { - val foo = gen.newType - } - class MyModule extends MultiIOModule { - val io = IO(Output(new MyBundle[UInt, DataGen[UInt]](new DataGen(UInt(3.W))))) - io.foo := 0.U - } - elaborate(new MyModule) + it should "work for higher-kinded types" in { + class DataGen[T <: Data](gen: T) { + def newType: T = gen.cloneType + } + class MyBundle[A <: Data, B <: DataGen[A]](gen: B) extends Bundle { + val foo = gen.newType + } + class MyModule extends MultiIOModule { + val io = IO(Output(new MyBundle[UInt, DataGen[UInt]](new DataGen(UInt(3.W))))) + io.foo := 0.U } + elaborate(new MyModule) } } diff --git a/src/test/scala/chiselTests/AutoNestedCloneSpec.scala b/src/test/scala/chiselTests/AutoNestedCloneSpec.scala index 401766e2..258d0823 100644 --- a/src/test/scala/chiselTests/AutoNestedCloneSpec.scala +++ b/src/test/scala/chiselTests/AutoNestedCloneSpec.scala @@ -3,6 +3,7 @@ package chiselTests import chisel3._ import chisel3.testers.TestUtils +import chisel3.stage.ChiselStage.elaborate import org.scalatest.matchers.should.Matchers class BundleWithAnonymousInner(val w: Int) extends Bundle { @@ -11,10 +12,7 @@ class BundleWithAnonymousInner(val w: Int) extends Bundle { } } -// TODO all `.suggestNames` are due to https://github.com/chipsalliance/chisel3/issues/1802 class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { - val usingPlugin: Boolean = TestUtils.usingPlugin - val elaborate = TestUtils.elaborateNoReflectiveAutoCloneType _ behavior of "autoCloneType of inner Bundle in Chisel3" @@ -27,7 +25,7 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { } def getIO: InnerIOType = new InnerIOType } - val io = IO(new Bundle {}).suggestName("io") + val io = IO(new Bundle {}) val myWire = Wire((new Middle(w)).getIO) } new Outer(2) @@ -37,7 +35,7 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { it should "clone an anonymous inner bundle successfully" in { elaborate { class TestTop(val w: Int) extends Module { - val io = IO(new Bundle {}).suggestName("io") + val io = IO(new Bundle {}) val myWire = Wire(new Bundle{ val a = UInt(w.W) }) } new TestTop(2) @@ -50,13 +48,13 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { val io = IO(new Bundle{ val in = Input(UInt(w.W)) val out = Output(UInt(w.W)) - }).suggestName("io") + }) } class Outer(val w: Int) extends Module { val io = IO(new Bundle{ val in = Input(UInt(w.W)) val out = Output(UInt(w.W)) - }).suggestName("io") + }) val i = Module(new Inner(w)) val iw = Wire(chiselTypeOf(i.io)) iw <> io @@ -69,7 +67,7 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { it should "clone an anonymous, bound, inner bundle of another bundle successfully" in { elaborate { class TestModule(w: Int) extends Module { - val io = IO(new BundleWithAnonymousInner(w) ).suggestName("io") + val io = IO(new BundleWithAnonymousInner(w)) val w0 = WireDefault(io) val w1 = WireDefault(io.inner) } @@ -85,7 +83,7 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { } val io = IO(new Bundle { val inner = Input(bun) - }).suggestName("io") + }) val w0 = WireDefault(io) val w1 = WireDefault(io.inner) } @@ -100,42 +98,39 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers with Utils { val inner = Input(new Bundle { val x = UInt(8.W) }) - }).suggestName("io") + }) } new TestModule() } } - if (usingPlugin) { - // This works with the plugin, but is a null pointer exception when using reflective autoclonetype - it should "support an anonymous doubly-nested inner bundle" in { - elaborate { - class Outer(val w: Int) extends Module { - class Middle(val w: Int) { - def getIO: Bundle = new Bundle { - val in = Input(UInt(w.W)) - } + it should "support an anonymous doubly-nested inner bundle" in { + elaborate { + class Outer(val w: Int) extends Module { + class Middle(val w: Int) { + def getIO: Bundle = new Bundle { + val in = Input(UInt(w.W)) } - val io = IO(new Bundle {}).suggestName("io") - val myWire = Wire((new Middle(w)).getIO) } - new Outer(2) + val io = IO(new Bundle {}) + val myWire = Wire((new Middle(w)).getIO) } + new Outer(2) } + } - it should "support anonymous Inner bundles that capture type parameters from outer Bundles" in { - elaborate(new MultiIOModule { - class MyBundle[T <: Data](n: Int, gen: T) extends Bundle { - val foo = new Bundle { - val x = Input(Vec(n, gen)) - } - val bar = Output(Option(new { def mkBundle = new Bundle { val x = Vec(n, gen) }}).get.mkBundle) + it should "support anonymous Inner bundles that capture type parameters from outer Bundles" in { + elaborate(new MultiIOModule { + class MyBundle[T <: Data](n: Int, gen: T) extends Bundle { + val foo = new Bundle { + val x = Input(Vec(n, gen)) } - val io = IO(new MyBundle(4, UInt(8.W))) - val myWire = WireInit(io.foo) - val myWire2 = WireInit(io.bar) - io.bar.x := io.foo.x - }) - } + val bar = Output(Option(new { def mkBundle = new Bundle { val x = Vec(n, gen) }}).get.mkBundle) + } + val io = IO(new MyBundle(4, UInt(8.W))) + val myWire = WireInit(io.foo) + val myWire2 = WireInit(io.bar) + io.bar.x := io.foo.x + }) } } |