Refactor and fix field reflection (#342)

No more need for e.g.

    new Bundle {
      def foo(dummy: Int): Data
    }

as now you can write

    new Bundle {
      def foo: Data
    }

This also removes code duplication with Module.

h/t @sdtwigg 

3 files changed, 25 insertions, 40 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
index 5e88560c..677fa43e 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
@@ -353,24 +353,10 @@ class Bundle extends Aggregate {
   /** Returns a field's contained user-defined Bundle element if it appears to
     * be one, otherwise returns None.
     */
-  private def getBundleField(m: java.lang.reflect.Method): Option[Data] = {
-    if (isBundleField(m) &&
-        (classOf[Data].isAssignableFrom(m.getReturnType) ||
-         classOf[Option[_]].isAssignableFrom(m.getReturnType))) {
-      m.invoke(this) match {
-        case d: Data =>
-          Some(d)
-        case o: Option[_] =>
-          o.getOrElse(None) match {
-            case d: Data =>
-              Some(d)
-            case _ => None
-          }
-        case _ => None
-      }
-    } else {
-      None
-    }
+  private def getBundleField(m: java.lang.reflect.Method): Option[Data] = m.invoke(this) match {
+    case d: Data => Some(d)
+    case Some(d: Data) => Some(d)
+    case _ => None
   }
 
   /** Returns a list of elements in this Bundle.
@@ -378,15 +364,14 @@ class Bundle extends Aggregate {
   private[core] lazy val namedElts = {
     val nameMap = LinkedHashMap[String, Data]()
     val seen = HashSet[Data]()
-    for (m <- getClass.getMethods.sortWith(_.getName < _.getName)) {
-      getBundleField(m) match {
-        case Some(d) =>
-          if (nameMap contains m.getName) {
-            require(nameMap(m.getName) eq d)
-          } else if (!seen(d)) {
-            nameMap(m.getName) = d; seen += d
-          }
-        case None =>
+    for (m <- getPublicFields(classOf[Bundle])) {
+      getBundleField(m) foreach { d =>
+        if (nameMap contains m.getName) {
+          require(nameMap(m.getName) eq d)
+        } else if (!seen(d)) {
+          nameMap(m.getName) = d
+          seen += d
+        }
       }
     }
     ArrayBuffer(nameMap.toSeq:_*) sortWith {case ((an, a), (bn, b)) => (a._id > b._id) || ((a eq b) && (an > bn))}
@@ -399,8 +384,6 @@ class Bundle extends Aggregate {
     s"{${namedElts.reverse.map(e => eltPort(e._2)).mkString(", ")}}"
   }
   private[chisel3] lazy val flatten = namedElts.flatMap(_._2.flatten)
-  private[core] def addElt(name: String, elt: Data): Unit =
-    namedElts += name -> elt
   private[chisel3] override def _onModuleClose: Unit = // scalastyle:ignore method.name
     for ((name, elt) <- namedElts) { elt.setRef(this, _namespace.name(name)) }
     
diff --git a/chiselFrontend/src/main/scala/chisel3/core/Module.scala b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
index 55522b4a..c700dc1b 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Module.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Module.scala
@@ -162,15 +162,6 @@ extends HasId {
       port.setRef(ModuleIO(this, _namespace.name(name)))
     }
 
-    // Suggest names to nodes using runtime reflection
-    def getValNames(c: Class[_]): Set[String] = {
-      if (c == classOf[Module]) Set()
-      else getValNames(c.getSuperclass) ++ c.getDeclaredFields.map(_.getName)
-    }
-    val valNames = getValNames(this.getClass)
-    def isPublicVal(m: java.lang.reflect.Method) =
-      m.getParameterTypes.isEmpty && valNames.contains(m.getName)
-
     /** Recursively suggests names to supported "container" classes
       * Arbitrary nestings of supported classes are allowed so long as the
       * innermost element is of type HasId
@@ -189,8 +180,7 @@ extends HasId {
           }
         case _ => // Do nothing
       }
-    val methods = getClass.getMethods.sortWith(_.getName > _.getName)
-    for (m <- methods if isPublicVal(m)) {
+    for (m <- getPublicFields(classOf[Module])) {
       nameRecursively(m.getName, m.invoke(this))
     }
 
diff --git a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
index 12cc840e..b4b0e028 100644
--- a/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
+++ b/chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
@@ -127,6 +127,18 @@ private[chisel3] trait HasId extends InstanceId {
     case Some(p) => p.modName
     case None => throwException(s"$instanceName doesn't have a parent")
   }
+
+  private[chisel3] def getPublicFields(rootClass: Class[_]): Seq[java.lang.reflect.Method] = {
+    // Suggest names to nodes using runtime reflection
+    def getValNames(c: Class[_]): Set[String] = {
+      if (c == rootClass) Set()
+      else getValNames(c.getSuperclass) ++ c.getDeclaredFields.map(_.getName)
+    }
+    val valNames = getValNames(this.getClass)
+    def isPublicVal(m: java.lang.reflect.Method) =
+      m.getParameterTypes.isEmpty && valNames.contains(m.getName)
+    this.getClass.getMethods.sortWith(_.getName < _.getName).filter(isPublicVal(_))
+  }
 }
 
 private[chisel3] class DynamicContext() {