Fix bug in Printable FullName of submodule port

Printable was using HasId.instanceName to get full names of Chisel nodes.
instanceName uses the parent module of the HasId to get the Component to use in
calling fullName on the underlying Ref. Unfortunately this means that any
reference to a port of a instance will leave off the instance name. Fixing this
required the following:

- Add Component argument to Printable.unpack so that we can call Arg.fullName
  directly in the Printable
- Pass the currently emitting module as the Component to Printable.unpack in
  the Emitter
- Remove ability to create FullName Printables from Modules since the Module
  name is not known until after the printf is already emitted

This commit also updates the PrintableSpec test to check that FullName and
Decimal printing work on ports of instances

3 files changed, 42 insertions, 15 deletions

diff --git a/chiselFrontend/src/main/scala/chisel3/core/Printable.scala b/chiselFrontend/src/main/scala/chisel3/core/Printable.scala
index 13d5d74e..f6e63936 100644
--- a/chiselFrontend/src/main/scala/chisel3/core/Printable.scala
+++ b/chiselFrontend/src/main/scala/chisel3/core/Printable.scala
@@ -15,6 +15,9 @@ import java.util.{
 /** Superclass of things that can be printed in the resulting circuit
   *
   * Usually created using the custom string interpolator p"..."
+  * TODO Add support for names of Modules
+  *   Currently impossible because unpack is called before the name is selected
+  *   Could be implemented by adding a new format specifier to Firrtl (eg. %m)
   * TODO Should we provide more functions like map and mkPrintable?
   */
 sealed abstract class Printable {
@@ -22,7 +25,7 @@ sealed abstract class Printable {
     * @note This must be called after elaboration when Chisel nodes actually
     *   have names
     */
-  def unpack: (String, Iterable[String])
+  def unpack(ctx: Component): (String, Iterable[String])
   /** Allow for appending Printables like Strings */
   final def +(that: Printable) = Printables(List(this, that))
   /** Allow for appending Strings to Printables */
@@ -87,22 +90,21 @@ object Printable {
 
 case class Printables(pables: Iterable[Printable]) extends Printable {
   require(pables.hasDefiniteSize, "Infinite-sized iterables are not supported!")
-  final def unpack: (String, Iterable[String]) = {
-    val (fmts, args) = pables.map(_.unpack).unzip
+  final def unpack(ctx: Component): (String, Iterable[String]) = {
+    val (fmts, args) = pables.map(_ unpack ctx).unzip
     (fmts.mkString, args.flatten)
   }
 }
 /** Wrapper for printing Scala Strings */
 case class PString(str: String) extends Printable {
-  final def unpack: (String, Iterable[String]) =
+  final def unpack(ctx: Component): (String, Iterable[String]) =
     (str replaceAll ("%", "%%"), List.empty)
 }
 /** Superclass for Firrtl format specifiers for Bits */
 sealed abstract class FirrtlFormat(specifier: Char) extends Printable {
   def bits: Bits
-  def unpack: (String, Iterable[String]) = {
-    val id = if (bits.isLit) bits.ref.name else bits.instanceName
-    (s"%$specifier", List(id))
+  def unpack(ctx: Component): (String, Iterable[String]) = {
+    (s"%$specifier", List(bits.ref.fullName(ctx)))
   }
 }
 object FirrtlFormat {
@@ -138,13 +140,13 @@ case class Binary(bits: Bits) extends FirrtlFormat('b')
 case class Character(bits: Bits) extends FirrtlFormat('c')
 /** Put innermost name (eg. field of bundle) */
 case class Name(data: Data) extends Printable {
-  final def unpack: (String, Iterable[String]) = (data.ref.name, List.empty)
+  final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.name, List.empty)
 }
 /** Put full name within parent namespace (eg. bundleName.field) */
-case class FullName(hasId: HasId) extends Printable {
-  final def unpack: (String, Iterable[String]) = (hasId.instanceName, List.empty)
+case class FullName(data: Data) extends Printable {
+  final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.fullName(ctx), List.empty)
 }
 /** Represents escaped percents */
 case object Percent extends Printable {
-  final def unpack: (String, Iterable[String]) = ("%%", List.empty)
+  final def unpack(ctx: Component): (String, Iterable[String]) = ("%%", List.empty)
 }
diff --git a/src/main/scala/chisel3/internal/firrtl/Emitter.scala b/src/main/scala/chisel3/internal/firrtl/Emitter.scala
index 8ace27f9..8849077d 100644
--- a/src/main/scala/chisel3/internal/firrtl/Emitter.scala
+++ b/src/main/scala/chisel3/internal/firrtl/Emitter.scala
@@ -26,7 +26,7 @@ private class Emitter(circuit: Circuit) {
       case e: BulkConnect => s"${e.loc1.fullName(ctx)} <- ${e.loc2.fullName(ctx)}"
       case e: Stop => s"stop(${e.clk.fullName(ctx)}, UInt<1>(1), ${e.ret})"
       case e: Printf =>
-        val (fmt, args) = e.pable.unpack
+        val (fmt, args) = e.pable.unpack(ctx)
         val printfArgs = Seq(e.clk.fullName(ctx), "UInt<1>(1)",
           "\"" + printf.format(fmt) + "\"") ++ args
         printfArgs mkString ("printf(", ", ", ")")
diff --git a/src/test/scala/chiselTests/PrintableSpec.scala b/src/test/scala/chiselTests/PrintableSpec.scala
index a2c8c62a..12564a40 100644
--- a/src/test/scala/chiselTests/PrintableSpec.scala
+++ b/src/test/scala/chiselTests/PrintableSpec.scala
@@ -97,7 +97,14 @@ class PrintableSpec extends FlatSpec with Matchers {
       case e => fail()
     }
   }
-  it should "support names of circuit elements and the current module" in {
+  it should "support names of circuit elements including submodule IO" in {
+    // Submodule IO is a subtle issue because the Chisel element has a different
+    // parent module
+    class MySubModule extends Module {
+      val io = new Bundle {
+        val fizz = UInt(width = 32)
+      }
+    }
     class MyBundle extends Bundle {
       val foo = UInt(width = 32)
       override def cloneType = (new MyBundle).asInstanceOf[this.type]
@@ -105,15 +112,33 @@ class PrintableSpec extends FlatSpec with Matchers {
     class MyModule extends BasicTester {
       override def desiredName = "MyModule"
       val myWire = Wire(new MyBundle)
+      val myInst = Module(new MySubModule)
       printf(p"${Name(myWire.foo)}")
       printf(p"${FullName(myWire.foo)}")
-      printf(p"${FullName(this)}")
+      printf(p"${FullName(myInst.io.fizz)}")
     }
     val firrtl = Driver.emit(() => new MyModule)
+    println(firrtl)
     getPrintfs(firrtl) match {
       case Seq(Printf("foo", Seq()),
                Printf("myWire.foo", Seq()),
-               Printf("MyModule", Seq())) =>
+               Printf("myInst.io.fizz", Seq())) =>
+      case e => fail()
+    }
+  }
+  it should "handle printing ports of submodules" in {
+    class MySubModule extends Module {
+      val io = new Bundle {
+        val fizz = UInt(width = 32)
+      }
+    }
+    class MyModule extends BasicTester {
+      val myInst = Module(new MySubModule)
+      printf(p"${myInst.io.fizz}")
+    }
+    val firrtl = Driver.emit(() => new MyModule)
+    getPrintfs(firrtl) match {
+      case Seq(Printf("%d", Seq("myInst.io.fizz"))) =>
       case e => fail()
     }
   }