Add some error context to Converter .getRefs (#1878)

1 files changed, 51 insertions, 36 deletions

diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
index aefbf8ab..ff0fa770 100644
--- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
+++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala
@@ -3,9 +3,9 @@
 package chisel3.internal.firrtl
 import chisel3._
 import chisel3.experimental._
-import chisel3.internal.sourceinfo.{NoSourceInfo, SourceLine, SourceInfo}
+import chisel3.internal.sourceinfo.{NoSourceInfo, SourceInfo, SourceLine, UnlocatableSourceInfo}
 import firrtl.{ir => fir}
-import chisel3.internal.{castToInt, throwException}
+import chisel3.internal.{HasId, castToInt, throwException}
 
 import scala.annotation.tailrec
 import scala.collection.immutable.Queue
@@ -24,6 +24,16 @@ private[chisel3] object Converter {
     case Percent => ("%%", List.empty)
   }
 
+  def getRef(id: HasId, sourceInfo: SourceInfo): Arg =
+    id.getOptionRef.getOrElse {
+      val module = id._parent.map(m => s" '$id' was defined in module '$m'.").getOrElse("")
+      val loc = sourceInfo.makeMessage(" " + _)
+      val link = "https://github.com/chipsalliance/chisel3/issues/new"
+      val msg = s"Internal error! Could not get ref for '$id'$loc!$module " +
+                s"This is a bug in Chisel, please file an issue at '$link'."
+      throwException(msg)
+    }
+
   def convert(info: SourceInfo): fir.Info = info match {
     case _: NoSourceInfo => fir.NoInfo
     case SourceLine(fn, line, col) => fir.FileInfo(fir.StringLit(s"$fn $line:$col"))
@@ -41,37 +51,37 @@ private[chisel3] object Converter {
   // TODO
   //   * Memoize?
   //   * Move into the Chisel IR?
-  def convert(arg: Arg, ctx: Component): fir.Expression = arg match {
+  def convert(arg: Arg, ctx: Component, info: SourceInfo): fir.Expression = arg match {
     case Node(id) =>
-      convert(id.getRef, ctx)
+      convert(getRef(id, info), ctx, info)
     case Ref(name) =>
       fir.Reference(name, fir.UnknownType)
     case Slot(imm, name) =>
-      fir.SubField(convert(imm, ctx), name, fir.UnknownType)
+      fir.SubField(convert(imm, ctx, info), name, fir.UnknownType)
     case Index(imm, ILit(idx)) =>
-      fir.SubIndex(convert(imm, ctx), castToInt(idx, "Index"), fir.UnknownType)
+      fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType)
     case Index(imm, value) =>
-      fir.SubAccess(convert(imm, ctx), convert(value, ctx), fir.UnknownType)
+      fir.SubAccess(convert(imm, ctx, info), convert(value, ctx, info), fir.UnknownType)
     case ModuleIO(mod, name) =>
       if (mod eq ctx.id) fir.Reference(name, fir.UnknownType)
-      else fir.SubField(fir.Reference(mod.getRef.name, fir.UnknownType), name, fir.UnknownType)
+      else fir.SubField(fir.Reference(getRef(mod, info).name, fir.UnknownType), name, fir.UnknownType)
     case u @ ULit(n, UnknownWidth()) =>
       fir.UIntLiteral(n, fir.IntWidth(u.minWidth))
     case ULit(n, w) =>
       fir.UIntLiteral(n, convert(w))
     case slit @ SLit(n, w) => fir.SIntLiteral(n, convert(w))
       val unsigned = if (n < 0) (BigInt(1) << slit.width.get) + n else n
-      val uint = convert(ULit(unsigned, slit.width), ctx)
+      val uint = convert(ULit(unsigned, slit.width), ctx, info)
       fir.DoPrim(firrtl.PrimOps.AsSInt, Seq(uint), Seq.empty, fir.UnknownType)
     // TODO Simplify
     case fplit @ FPLit(n, w, bp) =>
       val unsigned = if (n < 0) (BigInt(1) << fplit.width.get) + n else n
-      val uint = convert(ULit(unsigned, fplit.width), ctx)
+      val uint = convert(ULit(unsigned, fplit.width), ctx, info)
       val lit = bp.asInstanceOf[KnownBinaryPoint].value
       fir.DoPrim(firrtl.PrimOps.AsFixedPoint, Seq(uint), Seq(lit), fir.UnknownType)
     case intervalLit @ IntervalLit(n, w, bp) =>
       val unsigned = if (n < 0) (BigInt(1) << intervalLit.width.get) + n else n
-      val uint = convert(ULit(unsigned, intervalLit.width), ctx)
+      val uint = convert(ULit(unsigned, intervalLit.width), ctx, info)
       val lit = bp.asInstanceOf[KnownBinaryPoint].value
       fir.DoPrim(firrtl.PrimOps.AsInterval, Seq(uint), Seq(n, n, lit), fir.UnknownType)
     case lit: ILit =>
@@ -84,7 +94,7 @@ private[chisel3] object Converter {
       val consts = e.args.collect { case ILit(i) => i }
       val args = e.args.flatMap {
         case _: ILit => None
-        case other => Some(convert(other, ctx))
+        case other => Some(convert(other, ctx, e.sourceInfo))
       }
       val expr = e.op.name match {
         case "mux" =>
@@ -95,44 +105,45 @@ private[chisel3] object Converter {
       }
       Some(fir.DefNode(convert(e.sourceInfo), e.name, expr))
     case e @ DefWire(info, id) =>
-      Some(fir.DefWire(convert(info), e.name, extractType(id)))
+      Some(fir.DefWire(convert(info), e.name, extractType(id, info)))
     case e @ DefReg(info, id, clock) =>
-      Some(fir.DefRegister(convert(info), e.name, extractType(id), convert(clock, ctx),
-                           firrtl.Utils.zero, convert(id.getRef, ctx)))
+      Some(fir.DefRegister(convert(info), e.name, extractType(id, info), convert(clock, ctx, info),
+                           firrtl.Utils.zero, convert(getRef(id, info), ctx, info)))
     case e @ DefRegInit(info, id, clock, reset, init) =>
-      Some(fir.DefRegister(convert(info), e.name, extractType(id), convert(clock, ctx),
-                           convert(reset, ctx), convert(init, ctx)))
+      Some(fir.DefRegister(convert(info), e.name, extractType(id, info), convert(clock, ctx, info),
+                           convert(reset, ctx, info), convert(init, ctx, info)))
     case e @ DefMemory(info, id, t, size) =>
-      Some(firrtl.CDefMemory(convert(info), e.name, extractType(t), size, false))
+      Some(firrtl.CDefMemory(convert(info), e.name, extractType(t, info), size, false))
     case e @ DefSeqMemory(info, id, t, size, ruw) =>
-      Some(firrtl.CDefMemory(convert(info), e.name, extractType(t), size, true, ruw))
+      Some(firrtl.CDefMemory(convert(info), e.name, extractType(t, info), size, true, ruw))
     case e: DefMemPort[_] =>
+      val info = e.sourceInfo
       Some(firrtl.CDefMPort(convert(e.sourceInfo), e.name, fir.UnknownType,
-             e.source.fullName(ctx), Seq(convert(e.index, ctx), convert(e.clock, ctx)), convert(e.dir)))
+             e.source.fullName(ctx), Seq(convert(e.index, ctx, info), convert(e.clock, ctx, info)), convert(e.dir)))
     case Connect(info, loc, exp) =>
-      Some(fir.Connect(convert(info), convert(loc, ctx), convert(exp, ctx)))
+      Some(fir.Connect(convert(info), convert(loc, ctx, info), convert(exp, ctx, info)))
     case BulkConnect(info, loc, exp) =>
-      Some(fir.PartialConnect(convert(info), convert(loc, ctx), convert(exp, ctx)))
+      Some(fir.PartialConnect(convert(info), convert(loc, ctx, info), convert(exp, ctx, info)))
     case Attach(info, locs) =>
-      Some(fir.Attach(convert(info), locs.map(l => convert(l, ctx))))
+      Some(fir.Attach(convert(info), locs.map(l => convert(l, ctx, info))))
     case DefInvalid(info, arg) =>
-      Some(fir.IsInvalid(convert(info), convert(arg, ctx)))
+      Some(fir.IsInvalid(convert(info), convert(arg, ctx, info)))
     case e @ DefInstance(info, id, _) =>
       Some(fir.DefInstance(convert(info), e.name, id.name))
     case Stop(info, clock, ret) =>
-      Some(fir.Stop(convert(info), ret, convert(clock, ctx), firrtl.Utils.one))
+      Some(fir.Stop(convert(info), ret, convert(clock, ctx, info), firrtl.Utils.one))
     case Printf(info, clock, pable) =>
       val (fmt, args) = unpack(pable, ctx)
       Some(fir.Print(convert(info), fir.StringLit(fmt),
-                     args.map(a => convert(a, ctx)), convert(clock, ctx), firrtl.Utils.one))
+                     args.map(a => convert(a, ctx, info)), convert(clock, ctx, info), firrtl.Utils.one))
     case Verification(op, info, clk, pred, msg) =>
       val firOp = op match {
         case Formal.Assert => fir.Formal.Assert
         case Formal.Assume => fir.Formal.Assume
         case Formal.Cover => fir.Formal.Cover
       }
-      Some(fir.Verification(firOp, convert(info), convert(clk, ctx),
-        convert(pred, ctx), firrtl.Utils.one, fir.StringLit(msg)))
+      Some(fir.Verification(firOp, convert(info), convert(clk, ctx, info),
+        convert(pred, ctx, info), firrtl.Utils.one, fir.StringLit(msg)))
     case _ => None
   }
 
@@ -173,7 +184,7 @@ private[chisel3] object Converter {
         // Please see WhenFrame for more details
         case None => cmds.head match {
           case WhenBegin(info, pred) =>
-            val when = fir.Conditionally(convert(info), convert(pred, ctx), fir.EmptyStmt, fir.EmptyStmt)
+            val when = fir.Conditionally(convert(info), convert(pred, ctx, info), fir.EmptyStmt, fir.EmptyStmt)
             val frame = WhenFrame(when, acc, false)
             rec(Queue.empty, frame +: scope)(cmds.tail)
           case WhenEnd(info, depth, _) =>
@@ -221,7 +232,9 @@ private[chisel3] object Converter {
     case d => d.specifiedDirection
   }
 
-  def extractType(data: Data, clearDir: Boolean = false): fir.Type = data match {
+  def extractType(data: Data, info: SourceInfo): fir.Type = extractType(data, false, info)
+
+  def extractType(data: Data, clearDir: Boolean, info: SourceInfo): fir.Type = data match {
     case _: Clock => fir.ClockType
     case _: AsyncReset => fir.AsyncResetType
     case _: ResetType => fir.ResetType
@@ -231,16 +244,16 @@ private[chisel3] object Converter {
     case d: FixedPoint => fir.FixedType(convert(d.width), convert(d.binaryPoint))
     case d: Interval => fir.IntervalType(d.range.lowerBound, d.range.upperBound, d.range.firrtlBinaryPoint)
     case d: Analog => fir.AnalogType(convert(d.width))
-    case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir), d.length)
+    case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir, info), d.length)
     case d: Record =>
       val childClearDir = clearDir ||
         d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output
       def eltField(elt: Data): fir.Field = (childClearDir, firrtlUserDirOf(elt)) match {
-        case (true, _) => fir.Field(elt.getRef.name, fir.Default, extractType(elt, true))
+        case (true, _) => fir.Field(getRef(elt, info).name, fir.Default, extractType(elt, true, info))
         case (false, SpecifiedDirection.Unspecified | SpecifiedDirection.Output) =>
-          fir.Field(elt.getRef.name, fir.Default, extractType(elt, false))
+          fir.Field(getRef(elt, info).name, fir.Default, extractType(elt, false, info))
         case (false, SpecifiedDirection.Flip | SpecifiedDirection.Input) =>
-          fir.Field(elt.getRef.name, fir.Flip, extractType(elt, false))
+          fir.Field(getRef(elt, info).name, fir.Flip, extractType(elt, false, info))
       }
       fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) })
     }
@@ -251,6 +264,7 @@ private[chisel3] object Converter {
     case StringParam(value) => fir.StringParam(name, fir.StringLit(value))
     case RawParam(value) => fir.RawStringParam(name, value)
   }
+
   def convert(port: Port, topDir: SpecifiedDirection = SpecifiedDirection.Unspecified): fir.Port = {
     val resolvedDir = SpecifiedDirection.fromParent(topDir, port.dir)
     val dir = resolvedDir match {
@@ -261,8 +275,9 @@ private[chisel3] object Converter {
       case SpecifiedDirection.Input | SpecifiedDirection.Output => true
       case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => false
     }
-    val tpe = extractType(port.id, clearDir)
-    fir.Port(fir.NoInfo, port.id.getRef.name, dir, tpe)
+    val info = UnlocatableSourceInfo // Unfortunately there is no source locator for ports ATM
+    val tpe = extractType(port.id, clearDir, info)
+    fir.Port(fir.NoInfo, getRef(port.id, info).name, dir, tpe)
   }
 
   def convert(component: Component): fir.DefModule = component match {