diff options
16 files changed, 281 insertions, 170 deletions
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala index da14cefd..ed3e1fee 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala @@ -5,7 +5,6 @@ package chisel3.core import scala.collection.immutable.ListMap import scala.collection.mutable.{ArrayBuffer, HashSet, LinkedHashMap} import scala.language.experimental.macros -import scala.util.Try import chisel3.internal._ import chisel3.internal.Builder.pushCommand @@ -558,131 +557,168 @@ class Bundle(implicit compileOptions: CompileOptions) extends Record { } override def cloneType : this.type = { - // Automatic Bundle cloning is inferred as follows: - // - User-overloaded cloneType is called instead of this (if supplied, so this method is never called) - // - Attempt to automatically fill in constructor parameters by using Scala reflection: - // - If multiple constructors, error out - // - If the constructor argument list can be matched to param accessors, call the constructor with - // the result of those accessors - // - Otherwise, error out - // - If Scala reflection crashes, fall back to Java reflection and inspect the constructor argument list: - // - If no parameters, invoke constructor directly. That was easy! - // - If the first parameter's type is equal to this Module, assume the Bundle is an inner class - // and invoke it with the current containing Module - // - Error out to the user - - def reflectError(desc: String) { - Builder.exception(s"Unable to automatically infer cloneType on $this: $desc") + // This attempts to infer constructor and arguments to clone this Bundle subtype without + // requiring the user explicitly overriding cloneType. + + def reflectError(desc: String): Nothing = { + Builder.exception(s"Unable to automatically infer cloneType on $this: $desc").asInstanceOf[Nothing] } - // Check if the bundle is an instance of an inner class by examining - // whether it has one one-argument constructor taking a type matching the enclosing class - if (this.getClass.getConstructors.size == 1) { - val aggClass = this.getClass - val constr = aggClass.getConstructors.head - val argTypes = constr.getParameterTypes - val outerClass = aggClass.getEnclosingClass - if (argTypes.size == 1 && outerClass != null) { - // attempt to clone using "$outer" - var clone: Option[this.type] = - Try[this.type](constr.newInstance(aggClass.getDeclaredField("$outer").get(this)).asInstanceOf[this.type]).toOption - if (clone.isEmpty) { - // fall back to outerModule field - clone = Try[this.type](constr.newInstance(outerModule.get).asInstanceOf[this.type]).toOption + import scala.reflect.runtime.universe._ + + // Check if this is an inner class, and if so, try to get the outer instance + val clazz = this.getClass + val outerClassInstance = Option(clazz.getEnclosingClass) match { + case Some(outerClass) => + val outerInstance = try { + clazz.getDeclaredField("$outer").get(this) // doesn't work in all cases, namely anonymous inner Bundles + } catch { + case _: NoSuchFieldException => this.outerModule match { + case Some(outerModule) => outerModule + case None => reflectError(s"Unable to determine instance of outer class $outerClass") + } } - clone.foreach(_.outerModule = this.outerModule) - if (clone.isDefined) { - return clone.get - } else { - reflectError("non-trivial inner Bundle class") + if (!outerClass.isAssignableFrom(outerInstance.getClass)) { + reflectError(s"Automatically determined outer class instance $outerInstance not assignable to outer class $outerClass") } - } + Some(outerClass, outerInstance) + case None => None } - // Try Scala reflection - import scala.reflect.runtime.universe._ + // 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 (argType isAssignableFrom outerInstance.getClass) { + Some(ctor.newInstance(outerInstance).asInstanceOf[this.type]) + } else { + None + } + case _ => None + } + clone match { + case Some(clone) => + clone.outerModule = this.outerModule + if (!clone.typeEquivalent(this)) { + reflectError(s"Automatically cloned $clone not type-equivalent to base $this." + + " Constructor argument values were not inferred, ensure constructor is deterministic.") + } + return clone.asInstanceOf[this.type] + case None => + } + } - val mirror = runtimeMirror(this.getClass.getClassLoader) - val decls = mirror.classSymbol(this.getClass).typeSignature.decls - println(s"$this decls: $decls") + // Get constructor parameters and accessible fields + val mirror = runtimeMirror(clazz.getClassLoader) + val classSymbol = try { + mirror.reflect(this).symbol + } catch { + case e: scala.reflect.internal.Symbols#CyclicReference => reflectError(s"Scala cannot reflect on $this, got exception $e." + + " 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) { - reflectError(s"found multiple constructors ($ctors). " + - "Either remove all but the default constructor, or define a custom cloneType method.") + reflectError(s"found multiple constructors ($ctors)." + + " Either remove all but the default constructor, or define a custom cloneType method.") } - - val accessors = decls.collect { case meth: MethodSymbol if meth.isParamAccessor => meth } - val ctorParamss = ctors.head.paramLists + 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 _ => reflectError(s"internal error, unexpected ctorParamss = $ctorParamss") - List() // make the type system happy } + 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.outerModule = this.outerModule + return clone + } catch { + case e @ (_: java.lang.reflect.InvocationTargetException | _: IllegalArgumentException) => + reflectError(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 ctorParamsName = ctorParams.map(_.name.toString) val accessorsName = accessors.filter(_.isStable).map(_.name.toString) - val paramsDiff = ctorParamsName.toSet -- accessorsName.toSet + val paramsDiff = ctorParamsNames.toSet -- accessorsName.toSet if (!paramsDiff.isEmpty) { - reflectError(s"constructor has parameters ($paramsDiff) that are not both immutable and accessible." + + reflectError(s"constructor has parameters $paramsDiff 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 ctorParamsVals = ctorParamsName.map { - paramName => instanceReflect.reflectMethod(accessorsMap(paramName)).apply() + val ctorParamsNameVals = ctorParamsNames.map { + paramName => paramName -> instanceReflect.reflectMethod(accessorsMap(paramName)).apply() + } + + // Oppurtunistic 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.map{ case (paramName, paramVal) => paramVal match { + case paramVal: Data => paramVal.hasBinding match { + case true => Some(paramName) + case false => None + } + case _ => None + } + }.filter(_.isDefined).map(_.get) + if (!boundDataParamNames.isEmpty) { + reflectError(s"constructor parameters ($boundDataParamNames) 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.") } + val ctorParamsVals = ctorParamsNameVals.map{ case (_, paramVal) => paramVal } + // Invoke ctor - val classReflect = mirror.reflectClass(mirror.classSymbol(this.getClass)) - classReflect.reflectConstructor(ctors.head).apply(ctorParamsVals:_*).asInstanceOf[this.type] - - - -// val constructor = this.getClass.getConstructors.head -// val args = Seq.fill(constructor.getParameterTypes.size)(null) -// constructor.newInstance(args:_*).asInstanceOf[this.type] - -// // If the user did not provide a cloneType method, try invoking one of -// // the following constructors, not all of which necessarily exist: -// // - A zero-parameter constructor -// // - A one-paramater constructor, with null as the argument -// // - A one-parameter constructor for a nested Bundle, with the enclosing -// // parent Module as the argument -// -// -// // val classMirror = runtimeMirror.reflectClass(classSymbol) -//// val constructors = classSymbol.typeSignature.members.filter(_.isConstructor).toList -//// println(constructors) -//// -// val constructor = this.getClass.getConstructors.head -// println(constructor) -// println(constructor.getParameterTypes) -// -// try { -// val args = Seq.fill(constructor.getParameterTypes.size)(null) -// constructor.newInstance(args:_*).asInstanceOf[this.type] -// } catch { -// case e: java.lang.reflect.InvocationTargetException if e.getCause.isInstanceOf[java.lang.NullPointerException] => -// try { -// constructor.newInstance(_parent.get).asInstanceOf[this.type] -// } catch { -// case _: java.lang.reflect.InvocationTargetException | _: java.lang.IllegalArgumentException => -// Builder.exception(s"Parameterized Bundle ${this.getClass} needs cloneType method. You are probably using " + -// "an anonymous Bundle object that captures external state and hence is un-cloneTypeable") -// this -// } -// case _: java.lang.reflect.InvocationTargetException | _: java.lang.IllegalArgumentException => -// Builder.exception(s"Parameterized Bundle ${this.getClass} needs cloneType method") -// this -// } + val classMirror = outerClassInstance match { + case Some((_, outerInstance)) => mirror.reflect(outerInstance).reflectClass(classSymbol) + case None => mirror.reflectClass(classSymbol) + } + val clone = classMirror.reflectConstructor(ctor).apply(ctorParamsVals:_*).asInstanceOf[this.type] + clone.outerModule = this.outerModule + + if (!clone.typeEquivalent(this)) { + reflectError(s"Automatically cloned $clone not type-equivalent to base $this." + + " 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 } /** Default "pretty-print" implementation diff --git a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala index a6cc8bac..3fdc383c 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Binding.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Binding.scala @@ -23,7 +23,7 @@ object Binding { object requireIsHardware { def apply(node: Data, msg: String = "") = { node._parent match { // Compatibility layer hack - case Some(x: BaseModule) => x._compatAutoWrapPorts + case Some(x: BaseModule) => x._autoWrapPorts case _ => } if (!node.hasBinding) { diff --git a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala index 3c99886e..aa0f8064 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/BlackBox.scala @@ -126,12 +126,12 @@ abstract class ExtModule(val params: Map[String, Param] = Map.empty[String, Para */ abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param])(implicit compileOptions: CompileOptions) extends BaseBlackBox { def io: Record - + // Allow access to bindings from the compatibility package - protected def _compatIoPortBound() = portsContains(io) - + protected def _ioPortBound() = portsContains(io) + private[core] override def generateComponent(): Component = { - _compatAutoWrapPorts() // pre-IO(...) compatibility hack + _autoWrapPorts() // pre-IO(...) compatibility hack // Restrict IO to just io, clock, and reset require(io != null, "BlackBox must have io") diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala index 0c69010b..fa9ab082 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala @@ -191,7 +191,7 @@ abstract class BaseModule extends HasId { * * TODO: remove this, perhaps by removing Bindings checks in compatibility mode. */ - def _compatAutoWrapPorts() {} + def _autoWrapPorts() {} // // BaseModule User API functions @@ -200,10 +200,29 @@ abstract class BaseModule extends HasId { Builder.annotations += annotation } - /** Chisel2 code didn't require the IO(...) wrapper and would assign a Chisel type directly to - * io, then do operations on it. This binds a Chisel type in-place (mutably) as an IO. - */ - protected def _bindIoInPlace(iodef: Data) { + /** + * This must wrap the datatype used to set the io field of any Module. + * i.e. All concrete modules must have defined io in this form: + * [lazy] val io[: io type] = IO(...[: io type]) + * + * Items in [] are optional. + * + * The granted iodef WILL NOT be cloned (to allow for more seamless use of + * anonymous Bundles in the IO) and thus CANNOT have been bound to any logic. + * This will error if any node is bound (e.g. due to logic in a Bundle + * constructor, which is considered improper). + * + * Also registers a Data as a port, also performing bindings. Cannot be called once ports are + * requested (so that all calls to ports will return the same information). + * Internal API. + * + * TODO(twigg): Specifically walk the Data definition to call out which nodes + * are problematic. + */ + protected def IO[T<:Data](iodef: T): iodef.type = { + require(!_closed, "Can't add more ports after module close") + requireIsChiselType(iodef, "io type") + // Compatibility code: Chisel2 did not require explicit direction on nodes // (unspecified treated as output, and flip on nothing was input). // This sets assigns the explicit directions required by newer semantics on @@ -229,38 +248,11 @@ abstract class BaseModule extends HasId { } } assignCompatDir(iodef, false) - + + // Bind each element of the iodef to being a Port iodef.bind(PortBinding(this)) _ports += iodef - } - - /** - * This must wrap the datatype used to set the io field of any Module. - * i.e. All concrete modules must have defined io in this form: - * [lazy] val io[: io type] = IO(...[: io type]) - * - * Items in [] are optional. - * - * The granted iodef WILL NOT be cloned (to allow for more seamless use of - * anonymous Bundles in the IO) and thus CANNOT have been bound to any logic. - * This will error if any node is bound (e.g. due to logic in a Bundle - * constructor, which is considered improper). - * - * Also registers a Data as a port, also performing bindings. Cannot be called once ports are - * requested (so that all calls to ports will return the same information). - * Internal API. - * - * TODO(twigg): Specifically walk the Data definition to call out which nodes - * are problematic. - */ - protected def IO[T<:Data](iodef: T): iodef.type = { - require(!_closed, "Can't add more ports after module close") - requireIsChiselType(iodef, "io type") - - // Clone the IO so we preserve immutability of data types - val iodefClone = iodef.cloneTypeFull - _bindIoInPlace(iodefClone) - iodefClone.asInstanceOf[iodef.type] + iodef } // diff --git a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala index ac87eda5..9c923037 100644 --- a/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala +++ b/chiselFrontend/src/main/scala/chisel3/core/UserModule.scala @@ -151,8 +151,8 @@ abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions) def io: Record // Allow access to bindings from the compatibility package - protected def _compatIoPortBound() = portsContains(io) - + protected def _ioPortBound() = portsContains(io) + protected override def nameIds(rootClass: Class[_]): HashMap[HasId, String] = { val names = super.nameIds(rootClass) @@ -165,7 +165,7 @@ abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions) } private[core] override def generateComponent(): Component = { - _compatAutoWrapPorts() // pre-IO(...) compatibility hack + _autoWrapPorts() // pre-IO(...) compatibility hack // Restrict IO to just io, clock, and reset require(io != null, "Module must have io") diff --git a/src/main/scala/chisel3/compatibility.scala b/src/main/scala/chisel3/compatibility.scala index 3c12b483..f299a287 100644 --- a/src/main/scala/chisel3/compatibility.scala +++ b/src/main/scala/chisel3/compatibility.scala @@ -250,9 +250,9 @@ package object Chisel { // scalastyle:ignore package.object.name abstract class BlackBox(params: Map[String, Param] = Map.empty[String, Param]) extends chisel3.core.BlackBox(params) { // This class auto-wraps the BlackBox with IO(...), allowing legacy code (where IO(...) wasn't // required) to build. - override def _compatAutoWrapPorts(): Unit = { // scalastyle:ignore method.name - if (!_compatIoPortBound()) { - _bindIoInPlace(io) + override def _autoWrapPorts(): Unit = { // scalastyle:ignore method.name + if (!_ioPortBound()) { + IO(io) } } } @@ -278,9 +278,9 @@ package object Chisel { // scalastyle:ignore package.object.name def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) = this(Option(_clock), Option(_reset))(moduleCompileOptions) - override def _compatAutoWrapPorts(): Unit = { // scalastyle:ignore method.name - if (!_compatIoPortBound() && io != null) { - _bindIoInPlace(io) + override def _autoWrapPorts(): Unit = { // scalastyle:ignore method.name + if (!_ioPortBound() && io != null) { + IO(io) } } } diff --git a/src/main/scala/chisel3/testers/BasicTester.scala b/src/main/scala/chisel3/testers/BasicTester.scala index 4cd03863..7bb441ba 100644 --- a/src/main/scala/chisel3/testers/BasicTester.scala +++ b/src/main/scala/chisel3/testers/BasicTester.scala @@ -11,14 +11,9 @@ import internal.firrtl._ import internal.sourceinfo.SourceInfo //import chisel3.core.ExplicitCompileOptions.NotStrict -class TesterIO extends Bundle { - // The testbench has no IOs, rather it should communicate using printf, assert, and stop. - // This is here (instead of just `new Bundle()`, since that has an implicit compileOptions - // constructor argument which is misapplied by clonetype -} - class BasicTester extends Module() { - val io = IO(new TesterIO) + // The testbench has no IOs, rather it should communicate using printf, assert, and stop. + val io = IO(new Bundle() {}) def popCount(n: Long): Int = n.toBinaryString.count(_=='1') diff --git a/src/test/scala/chiselTests/AnalogIntegrationSpec.scala b/src/test/scala/chiselTests/AnalogIntegrationSpec.scala index 952d3872..a3e6e643 100644 --- a/src/test/scala/chiselTests/AnalogIntegrationSpec.scala +++ b/src/test/scala/chiselTests/AnalogIntegrationSpec.scala @@ -19,7 +19,7 @@ class AnalogBlackBoxPort extends Bundle { // This IO can be used for a single BlackBox or to group multiple // Has multiple ports for driving and checking but only one shared bus -class AnalogBlackBoxIO(n: Int) extends Bundle { +class AnalogBlackBoxIO(val n: Int) extends Bundle { require(n > 0) val bus = Analog(32.W) val port = Vec(n, new AnalogBlackBoxPort) diff --git a/src/test/scala/chiselTests/AutoClonetypeSpec.scala b/src/test/scala/chiselTests/AutoClonetypeSpec.scala index 93031c1c..1170ed12 100644 --- a/src/test/scala/chiselTests/AutoClonetypeSpec.scala +++ b/src/test/scala/chiselTests/AutoClonetypeSpec.scala @@ -7,27 +7,108 @@ import chisel3._ import chisel3.testers.BasicTester class BundleWithIntArg(val i: Int) extends Bundle { - val out = Output(UInt(i.W)) + val out = UInt(i.W) +} + +class BundleWithImplicit()(implicit val ii: Int) extends Bundle { + val out = UInt(ii.W) +} + +class BundleWithArgAndImplicit(val i: Int)(implicit val ii: Int) extends Bundle { + val out1 = UInt(i.W) + val out2 = UInt(ii.W) +} + +class BaseBundleVal(val i: Int) extends Bundle { + val inner = UInt(i.W) +} +class SubBundle(i: Int, val i2: Int) extends BaseBundleVal(i) { + val inner2 = UInt(i2.W) +} +class SubBundleInvalid(i: Int, val i2: Int) extends BaseBundleVal(i+1) { + val inner2 = UInt(i2.W) +} + +class BaseBundleNonVal(i: Int) extends Bundle { + val inner = UInt(i.W) +} +class SubBundleVal(val i: Int, val i2: Int) extends BaseBundleNonVal(i) { + val inner2 = UInt(i2.W) } class ModuleWithInner extends Module { class InnerBundle(val i: Int) extends Bundle { - val out = Output(UInt(i.W)) + val out = UInt(i.W) } - val io = IO(new InnerBundle(14)) - io.out := 1.U + val io = IO(new Bundle{}) + + val myWire = Wire(new InnerBundle(14)) + require(myWire.i == 14) } class AutoClonetypeSpec extends ChiselFlatSpec { "Bundles with Scala args" should "not need clonetype" in { elaborate { new Module { - val io = IO(new BundleWithIntArg(8)) - io.out := 1.U + val io = IO(new Bundle{}) + + val myWire = Wire(new BundleWithIntArg(8)) + assert(myWire.i == 8) + } } + } + + "Bundles with Scala implicit args" should "not need clonetype" in { + elaborate { new Module { + val io = IO(new Bundle{}) + + implicit val implicitInt: Int = 4 + val myWire = Wire(new BundleWithImplicit()) + + assert(myWire.ii == 4) } } } + "Bundles with Scala explicit and impicit args" should "not need clonetype" in { + elaborate { new Module { + val io = IO(new Bundle{}) + + implicit val implicitInt: Int = 4 + val myWire = Wire(new BundleWithArgAndImplicit(8)) + + assert(myWire.i == 8) + assert(myWire.ii == 4) + } } + } + + "Subtyped Bundles" should "not need clonetype" in { + elaborate { new Module { + val io = IO(new Bundle{}) + + val myWire = Wire(new SubBundle(8, 4)) + + assert(myWire.i == 8) + assert(myWire.i2 == 4) + } } + elaborate { new Module { + val io = IO(new Bundle{}) + + val myWire = Wire(new SubBundleVal(8, 4)) + + assert(myWire.i == 8) + assert(myWire.i2 == 4) + } } + } + + "Subtyped Bundles that don't clone well" should "be caught" in { + a [ChiselException] should be thrownBy { + elaborate { new Module { + val io = IO(new Bundle{}) + val myWire = Wire(new SubBundleInvalid(8, 4)) + } } + } + } + "Inner bundles with Scala args" should "not need clonetype" in { elaborate { new ModuleWithInner } } diff --git a/src/test/scala/chiselTests/AutoNestedCloneSpec.scala b/src/test/scala/chiselTests/AutoNestedCloneSpec.scala index 178cb142..d3977213 100644 --- a/src/test/scala/chiselTests/AutoNestedCloneSpec.scala +++ b/src/test/scala/chiselTests/AutoNestedCloneSpec.scala @@ -9,7 +9,7 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers { behavior of "autoCloneType of inner Bundle in Chisel3" it should "clone a doubly-nested inner bundle successfully" in { - elaborate { + elaborate { class Outer(val w: Int) extends Module { class Middle(val w: Int) { class InnerIOType extends Bundle { @@ -17,16 +17,18 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers { } def getIO = new InnerIOType } - val io = IO((new Middle(w)).getIO) + val io = IO(new Bundle {}) + val myWire = Wire((new Middle(w)).getIO) } new Outer(2) } } it should "clone an anonymous inner bundle successfully" in { - elaborate { + elaborate { class TestTop(val w: Int) extends Module { - val io = IO(new Bundle{ val a = UInt(w.W) }) + val io = IO(new Bundle {}) + val myWire = Wire(new Bundle{ val a = UInt(w.W) }) } new TestTop(2) } @@ -63,10 +65,11 @@ class AutoNestedCloneSpec extends ChiselFlatSpec with Matchers { val in = Input(UInt(w.W)) } } - val io = IO((new Middle(w)).getIO) + val io = IO(new Bundle {}) + val myWire = Wire((new Middle(w)).getIO) } new Outer(2) } - }).getMessage should include("non-trivial inner Bundle class") + }).getMessage should include("Unable to determine instance") } diff --git a/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala b/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala index 457f26de..95dc87c1 100644 --- a/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala +++ b/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala @@ -12,6 +12,8 @@ object CompatibilityComponents { class ChiselBundle extends Bundle { val a = UInt(width = 32) val b = UInt(width = 32).flip + + override def cloneType = (new ChiselBundle).asInstanceOf[this.type] } class ChiselRecord extends Record { val elements = ListMap("a" -> UInt(width = 32), "b" -> UInt(width = 32).flip) @@ -46,6 +48,8 @@ object Chisel3Components { class Chisel3Bundle extends Bundle { val a = Output(UInt(32.W)) val b = Input(UInt(32.W)) + + override def cloneType = (new Chisel3Bundle).asInstanceOf[this.type] } class Chisel3Record extends Record { diff --git a/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala b/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala index 43f2b0fd..fc9346a0 100644 --- a/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala +++ b/src/test/scala/chiselTests/MissingCloneBindingExceptionSpec.scala @@ -28,7 +28,7 @@ class MissingCloneBindingExceptionSpec extends ChiselFlatSpec with Matchers { } elaborate(new TestTop) - }).getMessage should include("needs cloneType method") + }).getMessage should include("make all parameters immutable") behavior of "missing cloneType in Chisel2" ( the[ChiselException] thrownBy { @@ -53,5 +53,5 @@ class MissingCloneBindingExceptionSpec extends ChiselFlatSpec with Matchers { } elaborate(new TestTop) - }).getMessage should include("needs cloneType method") + }).getMessage should include("make all parameters immutable") } diff --git a/src/test/scala/chiselTests/NamingAnnotationTest.scala b/src/test/scala/chiselTests/NamingAnnotationTest.scala index 699dc0fb..a7b9b75c 100644 --- a/src/test/scala/chiselTests/NamingAnnotationTest.scala +++ b/src/test/scala/chiselTests/NamingAnnotationTest.scala @@ -12,7 +12,7 @@ import chisel3.testers.BasicTester import scala.collection.mutable.ListBuffer trait NamedModuleTester extends Module { - val io = IO(new Bundle()) // Named module testers don't need IO + val io = IO(new Bundle() {}) // Named module testers don't need IO val expectedNameMap = ListBuffer[(InstanceId, String)]() val expectedModuleNameMap = ListBuffer[(Module, String)]() diff --git a/src/test/scala/chiselTests/OptionBundle.scala b/src/test/scala/chiselTests/OptionBundle.scala index 2ac661ea..03b08385 100644 --- a/src/test/scala/chiselTests/OptionBundle.scala +++ b/src/test/scala/chiselTests/OptionBundle.scala @@ -6,7 +6,7 @@ import org.scalatest._ import chisel3._ import chisel3.testers.BasicTester -class OptionBundle(hasIn: Boolean) extends Bundle { +class OptionBundle(val hasIn: Boolean) extends Bundle { val in = if (hasIn) { Some(Input(Bool())) } else { @@ -15,7 +15,7 @@ class OptionBundle(hasIn: Boolean) extends Bundle { val out = Output(Bool()) } -class OptionBundleModule(hasIn: Boolean) extends Module { +class OptionBundleModule(val hasIn: Boolean) extends Module { val io = IO(new OptionBundle(hasIn)) if (hasIn) { io.out := io.in.get diff --git a/src/test/scala/chiselTests/VectorPacketIO.scala b/src/test/scala/chiselTests/VectorPacketIO.scala index 7745db57..4a8c6f78 100644 --- a/src/test/scala/chiselTests/VectorPacketIO.scala +++ b/src/test/scala/chiselTests/VectorPacketIO.scala @@ -27,7 +27,7 @@ class Packet extends Bundle { * lines also. * The problem does not occur if the Vec is taken out */ -class VectorPacketIO(n: Int) extends Bundle { +class VectorPacketIO(val n: Int) extends Bundle { val ins = Vec(n, chisel3.util.DeqIO(new Packet())) val outs = Vec(n, chisel3.util.EnqIO(new Packet())) } diff --git a/src/test/scala/examples/VendingMachineGenerator.scala b/src/test/scala/examples/VendingMachineGenerator.scala index 55a17d9f..0a9dc3e6 100644 --- a/src/test/scala/examples/VendingMachineGenerator.scala +++ b/src/test/scala/examples/VendingMachineGenerator.scala @@ -9,7 +9,7 @@ import chisel3.util._ import VendingMachineUtils._ -class VendingMachineIO(legalCoins: Seq[Coin]) extends Bundle { +class VendingMachineIO(val legalCoins: Seq[Coin]) extends Bundle { require(legalCoins.size >= 1, "The vending machine must accept at least 1 coin!") // Order of coins by value val coins: Seq[Coin] = legalCoins sortBy (_.value) |