Auto Clone Bundles in Companion Objects (#788)

chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala

@@ -572,12 +572,25 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
 
     val clazz = this.getClass
 
-    def reflectError(desc: String): Nothing = {
+    def autoClonetypeError(desc: String): Nothing = {
       throw new AutoClonetypeException(s"Unable to automatically infer cloneType on $clazz: $desc")
     }
 
+    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)
+    }
+
     // Check if this is an inner class, and if so, try to get the outer instance
-    val outerClassInstance = Option(clazz.getEnclosingClass).map { outerClass =>
+    val outerClassInstance = enclosingClassOption.map { outerClass =>
       def canAssignOuterClass(x: Object) = outerClass.isAssignableFrom(x.getClass)
 
       val outerInstance = _outerInst match {
@@ -599,9 +612,9 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
                 case outer :: Nil =>
                   _outerInst = Some(outer)  // record the guess for future use
                   outer
-                case Nil => reflectError(s"Unable to determine instance of outer class $outerClass," +
+                case Nil => autoClonetypeError(s"Unable to determine instance of outer class $outerClass," +
                     s" no candidates assignable to outer class types; examined $allOuterCandidates")
-                case candidates => reflectError(s"Unable to determine instance of outer class $outerClass," +
+                case candidates => autoClonetypeError(s"Unable to determine instance of outer class $outerClass," +
                     s" multiple possible candidates $candidates assignable to outer class type")
               }
           }
@@ -628,12 +641,13 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
             None
           }
         case _ => None
+
       }
       clone match {
         case Some(clone) =>
           clone._outerInst = this._outerInst
           if (!clone.typeEquivalent(this)) {
-            reflectError(s"automatically cloned $clone not type-equivalent to base." +
+            autoClonetypeError(s"automatically cloned $clone not type-equivalent to base." +
             " Constructor argument values were not inferred, ensure constructor is deterministic.")
           }
           return clone.asInstanceOf[this.type]
@@ -642,19 +656,14 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     }
 
     // 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"got exception $e attempting Scala reflection." +
-          " 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 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) {
-      reflectError(s"found multiple constructors ($ctors)." +
+      autoClonetypeError(s"found multiple constructors ($ctors)." +
           " Either remove all but the default constructor, or define a custom cloneType method.")
     }
     val ctor = ctors.head
@@ -663,7 +672,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
       case Nil => List()
       case ctorParams :: Nil => ctorParams
       case ctorParams :: ctorImplicits :: Nil => ctorParams ++ ctorImplicits
-      case _ => reflectError(s"internal error, unexpected ctorParamss = $ctorParamss")
+      case _ => autoClonetypeError(s"internal error, unexpected ctorParamss = $ctorParamss")
     }
     val ctorParamsNames = ctorParams.map(_.name.toString)
 
@@ -680,7 +689,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
         return clone
       } catch {
         case e @ (_: java.lang.reflect.InvocationTargetException | _: IllegalArgumentException) =>
-          reflectError(s"unexpected failure at constructor invocation, got $e.")
+          autoClonetypeError(s"unexpected failure at constructor invocation, got $e.")
       }
     }
 
@@ -699,7 +708,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     val accessorsName = accessors.filter(_.isStable).map(_.name.toString)
     val paramsDiff = ctorParamsNames.toSet -- accessorsName.toSet
     if (!paramsDiff.isEmpty) {
-      reflectError(s"constructor has parameters (${paramsDiff.toList.sorted.mkString(", ")}) that are not both immutable and accessible." +
+      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.")
     }
 
@@ -717,7 +726,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
       case (paramName, paramVal: Data) if paramVal.hasBinding => paramName
     }
     if (boundDataParamNames.nonEmpty) {
-      reflectError(s"constructor parameters (${boundDataParamNames.sorted.mkString(", ")}) have values that are hardware types, which is likely to cause subtle errors." +
+      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.")
     }
 
@@ -730,13 +739,13 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
     // Invoke ctor
     val classMirror = outerClassInstance match {
       case Some((_, outerInstance)) => mirror.reflect(outerInstance).reflectClass(classSymbol)
-      case None => mirror.reflectClass(classSymbol)
+      case _ => mirror.reflectClass(classSymbol)
     }
     val clone = classMirror.reflectConstructor(ctor).apply(ctorParamsVals:_*).asInstanceOf[this.type]
     clone._outerInst = this._outerInst
 
     if (!clone.typeEquivalent(this)) {
-      reflectError(s"Automatically cloned $clone not type-equivalent to base $this." +
+      autoClonetypeError(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.")
     }

src/test/scala/chiselTests/AutoClonetypeSpec.scala

@@ -47,6 +47,15 @@ class ModuleWithInner extends Module {
   require(myWire.i == 14)
 }
 
+object CompanionObjectWithBundle {
+  class ParameterizedInner(val i: Int) extends Bundle {
+    val data = UInt(i.W)
+  }
+  class Inner extends Bundle {
+    val data = UInt(8.W)
+  }
+}
+
 // A Bundle with an argument that is also a field.
 // Not necessarily good style (and not necessarily recommended), but allowed to preserve compatibility.
 class BundleWithArgumentField(val x: Data, val y: Data) extends Bundle
@@ -123,4 +132,18 @@ class AutoClonetypeSpec extends ChiselFlatSpec {
       io.y := 1.U
     } }
   }
+
+  "Bundles inside companion objects" should "not need clonetype" in {
+    elaborate { new Module {
+      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)))
+      io.data := 1.U
+    } }
+  }
 }