Merge pull request #932 from seldridge/f269

- Add Target.prettyPrint method
- Improve UninferredWidth exception message

4 files changed, 121 insertions, 33 deletions

diff --git a/src/main/scala/firrtl/annotations/Target.scala b/src/main/scala/firrtl/annotations/Target.scala
index dcf5cb02..8a9d68e8 100644
--- a/src/main/scala/firrtl/annotations/Target.scala
+++ b/src/main/scala/firrtl/annotations/Target.scala
@@ -55,6 +55,32 @@ sealed trait Target extends Named {
     }
   }
 
+  /** Pretty serialization, ideal for error messages. Cannot be deserialized.
+    * @return Human-readable serialization
+    */
+  def prettyPrint(tab: String = ""): String = {
+    val circuitString = s"""${tab}circuit ${circuitOpt.getOrElse("???")}:"""
+    val moduleString = s"""\n$tab└── module ${moduleOpt.getOrElse("???")}:"""
+    var depth = 4
+    val tokenString = tokens.map {
+      case Ref(r) => val rx = s"""\n$tab${" "*depth}└── $r"""; depth += 4; rx
+      case Instance(i) => val ix = s"""\n$tab${" "*depth}└── inst $i """; ix
+      case OfModule(o) => val ox = s"of $o:"; depth += 4; ox
+      case Field(f) => s".$f"
+      case Index(v) => s"[$v]"
+      case Clock => s"@clock"
+      case Reset => s"@reset"
+      case Init => s"@init"
+    }.mkString("")
+
+    (moduleOpt.isEmpty, tokens.isEmpty) match {
+      case (true, true) => circuitString
+      case (_, true) => circuitString + moduleString
+      case (_, _) => circuitString + moduleString + tokenString
+    }
+  }
+
+
   /** @return Converts this [[Target]] into a [[GenericTarget]] */
   def toGenericTarget: GenericTarget = GenericTarget(circuitOpt, moduleOpt, tokens.toVector)
 
diff --git a/src/main/scala/firrtl/passes/CheckWidths.scala b/src/main/scala/firrtl/passes/CheckWidths.scala
index 7827c55e..4a72b18c 100644
--- a/src/main/scala/firrtl/passes/CheckWidths.scala
+++ b/src/main/scala/firrtl/passes/CheckWidths.scala
@@ -7,42 +7,44 @@ import firrtl.ir._
 import firrtl.PrimOps._
 import firrtl.Mappers._
 import firrtl.Utils._
+import firrtl.annotations.{Target, TargetToken, CircuitTarget, ModuleTarget}
 
 object CheckWidths extends Pass {
   /** The maximum allowed width for any circuit element */
   val MaxWidth = 1000000
   val DshlMaxWidth = ceilLog2(MaxWidth + 1)
-  class UninferredWidth (info: Info, mname: String) extends PassException(
-    s"$info : [module $mname]  Uninferred width.")
+  class UninferredWidth (info: Info, target: String) extends PassException(
+    s"""|$info : Uninferred width for target below. (Did you forget to assign to it?)
+        |$target""".stripMargin)
   class WidthTooSmall(info: Info, mname: String, b: BigInt) extends PassException(
-    s"$info : [module $mname]  Width too small for constant $b.")
+    s"$info : [target $mname]  Width too small for constant $b.")
   class WidthTooBig(info: Info, mname: String, b: BigInt) extends PassException(
-    s"$info : [module $mname]  Width $b greater than max allowed width of $MaxWidth bits")
+    s"$info : [target $mname]  Width $b greater than max allowed width of $MaxWidth bits")
   class DshlTooBig(info: Info, mname: String) extends PassException(
-    s"$info : [module $mname]  Width of dshl shift amount cannot be larger than $DshlMaxWidth bits.")
+    s"$info : [target $mname]  Width of dshl shift amount cannot be larger than $DshlMaxWidth bits.")
   class NegWidthException(info:Info, mname: String) extends PassException(
-    s"$info: [module $mname] Width cannot be negative or zero.")
+    s"$info: [target $mname] Width cannot be negative or zero.")
   class BitsWidthException(info: Info, mname: String, hi: BigInt, width: BigInt, exp: String) extends PassException(
-    s"$info: [module $mname] High bit $hi in bits operator is larger than input width $width in $exp.")
+    s"$info: [target $mname] High bit $hi in bits operator is larger than input width $width in $exp.")
   class HeadWidthException(info: Info, mname: String, n: BigInt, width: BigInt) extends PassException(
-    s"$info: [module $mname] Parameter $n in head operator is larger than input width $width.")
+    s"$info: [target $mname] Parameter $n in head operator is larger than input width $width.")
   class TailWidthException(info: Info, mname: String, n: BigInt, width: BigInt) extends PassException(
-    s"$info: [module $mname] Parameter $n in tail operator is larger than input width $width.")
+    s"$info: [target $mname] Parameter $n in tail operator is larger than input width $width.")
   class AttachWidthsNotEqual(info: Info, mname: String, eName: String, source: String) extends PassException(
-    s"$info: [module $mname] Attach source $source and expression $eName must have identical widths.")
+    s"$info: [target $mname] Attach source $source and expression $eName must have identical widths.")
 
   def run(c: Circuit): Circuit = {
     val errors = new Errors()
 
-    def check_width_w(info: Info, mname: String)(w: Width): Width = {
+    def check_width_w(info: Info, target: Target)(w: Width): Width = {
       w match {
         case IntWidth(width) if width >= MaxWidth =>
-          errors.append(new WidthTooBig(info, mname, width))
+          errors.append(new WidthTooBig(info, target.serialize, width))
         case w: IntWidth if w.width >= 0 =>
         case _: IntWidth =>
-          errors append new NegWidthException(info, mname)
+          errors append new NegWidthException(info, target.serialize)
         case _ =>
-          errors append new UninferredWidth(info, mname)
+          errors append new UninferredWidth(info, target.prettyPrint("    "))
       }
       w
     }
@@ -53,62 +55,71 @@ object CheckWidths extends Pass {
       case _ => throwInternalError(s"hasWidth - $tpe")
     }
 
-    def check_width_t(info: Info, mname: String)(t: Type): Type =
-      t map check_width_t(info, mname) map check_width_w(info, mname)
+    def check_width_t(info: Info, target: Target)(t: Type): Type = {
+      val tx = t match {
+        case tt: BundleType => BundleType(tt.fields.map(check_width_f(info, target)))
+        case tt => tt map check_width_t(info, target)
+      }
+      tx map check_width_w(info, target)
+    }
+
+    def check_width_f(info: Info, target: Target)(f: Field): Field = f
+      .copy(tpe = check_width_t(info, target.modify(tokens = target.tokens :+ TargetToken.Field(f.name)))(f.tpe))
 
-    def check_width_e(info: Info, mname: String)(e: Expression): Expression = {
+    def check_width_e(info: Info, target: Target)(e: Expression): Expression = {
       e match {
         case e: UIntLiteral => e.width match {
           case w: IntWidth if math.max(1, e.value.bitLength) > w.width =>
-            errors append new WidthTooSmall(info, mname, e.value)
+            errors append new WidthTooSmall(info, target.serialize, e.value)
           case _ =>
         }
         case e: SIntLiteral => e.width match {
           case w: IntWidth if e.value.bitLength + 1 > w.width =>
-            errors append new WidthTooSmall(info, mname, e.value)
+            errors append new WidthTooSmall(info, target.serialize, e.value)
           case _ =>
         }
         case DoPrim(Bits, Seq(a), Seq(hi, lo), _) if (hasWidth(a.tpe) && bitWidth(a.tpe) <= hi) =>
-          errors append new BitsWidthException(info, mname, hi, bitWidth(a.tpe), e.serialize)
+          errors append new BitsWidthException(info, target.serialize, hi, bitWidth(a.tpe), e.serialize)
         case DoPrim(Head, Seq(a), Seq(n), _) if (hasWidth(a.tpe) && bitWidth(a.tpe) < n) =>
-          errors append new HeadWidthException(info, mname, n, bitWidth(a.tpe))
+          errors append new HeadWidthException(info, target.serialize, n, bitWidth(a.tpe))
         case DoPrim(Tail, Seq(a), Seq(n), _) if (hasWidth(a.tpe) && bitWidth(a.tpe) <= n) =>
-          errors append new TailWidthException(info, mname, n, bitWidth(a.tpe))
+          errors append new TailWidthException(info, target.serialize, n, bitWidth(a.tpe))
         case DoPrim(Dshl, Seq(a, b), _, _) if (hasWidth(a.tpe) && bitWidth(b.tpe) >= DshlMaxWidth) =>
-          errors append new DshlTooBig(info, mname)
+          errors append new DshlTooBig(info, target.serialize)
         case _ =>
       }
       //e map check_width_t(info, mname) map check_width_e(info, mname)
-      e map check_width_e(info, mname)
+      e map check_width_e(info, target)
     }
 
 
-    def check_width_s(minfo: Info, mname: String)(s: Statement): Statement = {
+    def check_width_s(minfo: Info, target: ModuleTarget)(s: Statement): Statement = {
       val info = get_info(s) match { case NoInfo => minfo case x => x }
-      s map check_width_e(info, mname) map check_width_s(info, mname) map check_width_t(info, mname) match {
+      val subRef = s match { case sx: HasName => target.ref(sx.name) case _ => target }
+      s map check_width_e(info, target) map check_width_s(info, target) map check_width_t(info, subRef) match {
         case Attach(infox, exprs) =>
           exprs.tail.foreach ( e =>
             if (bitWidth(e.tpe) != bitWidth(exprs.head.tpe))
-              errors.append(new AttachWidthsNotEqual(infox, mname, e.serialize, exprs.head.serialize))
+              errors.append(new AttachWidthsNotEqual(infox, target.serialize, e.serialize, exprs.head.serialize))
           )
           s
         case sx: DefRegister =>
           sx.reset.tpe match {
             case UIntType(IntWidth(w)) if w == 1 =>
-            case _ => errors.append(new CheckTypes.IllegalResetType(info, mname, sx.name))
+            case _ => errors.append(new CheckTypes.IllegalResetType(info, target.serialize, sx.name))
           }
           s
         case _ => s
       }
     }
 
-    def check_width_p(minfo: Info, mname: String)(p: Port): Port = p.copy(tpe =  check_width_t(p.info, mname)(p.tpe))
+    def check_width_p(minfo: Info, target: ModuleTarget)(p: Port): Port = p.copy(tpe =  check_width_t(p.info, target)(p.tpe))
 
-    def check_width_m(m: DefModule) {
-      m map check_width_p(m.info, m.name) map check_width_s(m.info, m.name)
+    def check_width_m(circuit: CircuitTarget)(m: DefModule) {
+      m map check_width_p(m.info, circuit.module(m.name)) map check_width_s(m.info, circuit.module(m.name))
     }
 
-    c.modules foreach check_width_m
+    c.modules foreach check_width_m(CircuitTarget(c.main))
     errors.trigger()
     c
   }
diff --git a/src/test/scala/firrtlTests/WidthSpec.scala b/src/test/scala/firrtlTests/WidthSpec.scala
index ab8cb7ac..058cc1fa 100644
--- a/src/test/scala/firrtlTests/WidthSpec.scala
+++ b/src/test/scala/firrtlTests/WidthSpec.scala
@@ -156,4 +156,29 @@ class WidthSpec extends FirrtlFlatSpec {
       executeTest(input, check, passes)
     }
   }
+
+  behavior of "CheckWidths.UniferredWidth"
+
+  it should "provide a good error message with a full target if a user forgets an assign" in {
+    val passes = Seq(
+      ToWorkingIR,
+      ResolveKinds,
+      InferTypes,
+      CheckTypes,
+      ResolveGenders,
+      InferWidths,
+      CheckWidths)
+    val input =
+      """|circuit Foo :
+         |  module Foo :
+         |    input clock : Clock
+         |    inst bar of Bar
+         |  module Bar :
+         |    wire a: { b : UInt<1>, c : { d : UInt<1>, e : UInt } }
+         |""".stripMargin
+    val msg = intercept[CheckWidths.UninferredWidth] { executeTest(input, Nil, passes) }
+      .getMessage should include ("""|    circuit Foo:
+                                     |    └── module Bar:
+                                     |        └── a.c.e""".stripMargin)
+  }
 }
diff --git a/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala b/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala
index 4ae4e036..da154b6a 100644
--- a/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala
+++ b/src/test/scala/firrtlTests/annotationTests/TargetSpec.scala
@@ -55,5 +55,31 @@ class TargetSpec extends FirrtlPropSpec {
       assert(Target.deserialize(t.serialize) == t, s"$t does not properly serialize/deserialize")
     }
   }
+  property("Pretty Printer should work") {
+    val circuit = CircuitTarget("A")
+    val top = circuit.module("B")
+    val targets = Seq(
+      (circuit, "circuit A:"),
+      (top,
+       """|circuit A:
+          |└── module B:""".stripMargin),
+      (top.instOf("c", "C"),
+       """|circuit A:
+          |└── module B:
+          |    └── inst c of C:""".stripMargin),
+      (top.ref("r"),
+       """|circuit A:
+          |└── module B:
+          |    └── r""".stripMargin),
+      (top.ref("r").index(1).field("hi").clock,
+       """|circuit A:
+          |└── module B:
+          |    └── r[1].hi@clock""".stripMargin),
+      (GenericTarget(None, None, Vector(Ref("r"))),
+       """|circuit ???:
+          |└── module ???:
+          |    └── r""".stripMargin)
+    )
+    targets.foreach { case (t, str) => assert(t.prettyPrint() == str, s"$t didn't properly prettyPrint") }
+  }
 }
-