Better MonoConnect error messages (#2248)

Co-authored-by: Megan Wachs <megan@sifive.com>

4 files changed, 74 insertions, 37 deletions

diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
index 89908401..d67a3af6 100644
--- a/core/src/main/scala/chisel3/Data.scala
+++ b/core/src/main/scala/chisel3/Data.scala
@@ -494,7 +494,7 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
       } catch {
         case MonoConnectException(message) =>
           throwException(
-            s"Connection between sink ($this) and source ($that) failed @$message"
+            s"Connection between sink ($this) and source ($that) failed @: $message"
           )
       }
     } else {
diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala
index 5cbab329..6173fc91 100644
--- a/core/src/main/scala/chisel3/internal/MonoConnect.scala
+++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala
@@ -36,33 +36,35 @@ import chisel3.internal.sourceinfo.SourceInfo
 */
 
 private[chisel3] object MonoConnect {
+  def formatName(data: Data) = s"""${data.earlyName} in ${data.parentNameOpt.getOrElse("(unknown)")}"""
+
   // These are all the possible exceptions that can be thrown.
   // These are from element-level connection
-  def UnreadableSourceException =
-    MonoConnectException(": Source is unreadable from current module.")
-  def UnwritableSinkException =
-    MonoConnectException(": Sink is unwriteable by current module.")
-  def SourceEscapedWhenScopeException =
-    MonoConnectException(": Source has escaped the scope of the when in which it was constructed.")
-  def SinkEscapedWhenScopeException =
-    MonoConnectException(": Sink has escaped the scope of the when in which it was constructed.")
+  def UnreadableSourceException(sink: Data, source: Data) =
+    MonoConnectException(s"""${formatName(source)} cannot be read from module ${sink.parentNameOpt.getOrElse("(unknown)")}.""")
+  def UnwritableSinkException(sink: Data, source: Data) =
+    MonoConnectException(s"""${formatName(sink)} cannot be written from module ${source.parentNameOpt.getOrElse("(unknown)")}.""")
+  def SourceEscapedWhenScopeException(source: Data) =
+    MonoConnectException(s"Source ${formatName(source)} has escaped the scope of the when in which it was constructed.")
+  def SinkEscapedWhenScopeException(sink: Data) =
+    MonoConnectException(s"Sink ${formatName(sink)} has escaped the scope of the when in which it was constructed.")
   def UnknownRelationException =
-    MonoConnectException(": Sink or source unavailable to current module.")
+    MonoConnectException("Sink or source unavailable to current module.")
   // These are when recursing down aggregate types
   def MismatchedVecException =
-    MonoConnectException(": Sink and Source are different length Vecs.")
+    MonoConnectException("Sink and Source are different length Vecs.")
   def MissingFieldException(field: String) =
-    MonoConnectException(s": Source Record missing field ($field).")
-  def MismatchedException(sink: String, source: String) =
-    MonoConnectException(s": Sink ($sink) and Source ($source) have different types.")
+    MonoConnectException(s"Source Record missing field ($field).")
+  def MismatchedException(sink: Data, source: Data) =
+    MonoConnectException(s"Sink (${sink.cloneType.toString}) and Source (${source.cloneType.toString}) have different types.")
   def DontCareCantBeSink =
-    MonoConnectException(": DontCare cannot be a connection sink (LHS)")
-  def AnalogCantBeMonoSink =
-    MonoConnectException(": Analog cannot participate in a mono connection (sink - LHS)")
-  def AnalogCantBeMonoSource =
-    MonoConnectException(": Analog cannot participate in a mono connection (source - RHS)")
-  def AnalogMonoConnectionException =
-    MonoConnectException(": Analog cannot participate in a mono connection (source and sink)")
+    MonoConnectException("DontCare cannot be a connection sink")
+  def AnalogCantBeMonoSink(sink: Data) =
+    MonoConnectException(s"Sink ${formatName(sink)} of type Analog cannot participate in a mono connection (:=)")
+  def AnalogCantBeMonoSource(source: Data) =
+    MonoConnectException(s"Source ${formatName(source)} of type Analog cannot participate in a mono connection (:=)")
+  def AnalogMonoConnectionException(source: Data, sink: Data) =
+    MonoConnectException(s"Source ${formatName(source)} and sink ${formatName(sink)} of type Analog cannot participate in a mono connection (:=)")
 
   def checkWhenVisibility(x: Data): Boolean = {
     x.topBinding match {
@@ -169,13 +171,13 @@ private[chisel3] object MonoConnect {
       // DontCare as a sink is illegal.
       case (DontCare, _) => throw DontCareCantBeSink
       // Analog is illegal in mono connections.
-      case (_: Analog, _:Analog) => throw AnalogMonoConnectionException
+      case (_: Analog, _:Analog) => throw AnalogMonoConnectionException(source, sink)
       // Analog is illegal in mono connections.
-      case (_: Analog, _) => throw AnalogCantBeMonoSink
+      case (_: Analog, _) => throw AnalogCantBeMonoSink(sink)
       // Analog is illegal in mono connections.
-      case (_, _: Analog) => throw AnalogCantBeMonoSource
+      case (_, _: Analog) => throw AnalogCantBeMonoSource(source)
       // Sink and source are different subtypes of data so fail
-      case (sink, source) => throw MismatchedException(sink.toString, source.toString)
+      case (sink, source) => throw MismatchedException(sink, source)
     }
 
   // This function (finally) issues the connection operation
@@ -196,7 +198,7 @@ private[chisel3] object MonoConnect {
     val source = reify(_source)
     // If source has no location, assume in context module
     // This can occur if is a literal, unbound will error previously
-    val sink_mod: BaseModule   = sink.topBinding.location.getOrElse(throw UnwritableSinkException)
+    val sink_mod: BaseModule   = sink.topBinding.location.getOrElse(throw UnwritableSinkException(sink, source))
     val source_mod: BaseModule = source.topBinding.location.getOrElse(context_mod)
 
     val sink_parent = Builder.retrieveParent(sink_mod, context_mod).getOrElse(None)
@@ -206,11 +208,11 @@ private[chisel3] object MonoConnect {
     val source_direction = BindingDirection.from(source.topBinding, source.direction)
 
     if (!checkWhenVisibility(sink)) {
-      throw SinkEscapedWhenScopeException
+      throw SinkEscapedWhenScopeException(sink)
     }
 
     if (!checkWhenVisibility(source)) {
-      throw SourceEscapedWhenScopeException
+      throw SourceEscapedWhenScopeException(source)
     }
 
     // CASE: Context is same module that both left node and right node are in
@@ -220,7 +222,7 @@ private[chisel3] object MonoConnect {
         //    CURRENT MOD   CURRENT MOD
         case (Output,       _) => issueConnect(sink, source)
         case (Internal,     _) => issueConnect(sink, source)
-        case (Input,        _) => throw UnwritableSinkException
+        case (Input,        _) => throw UnwritableSinkException(sink, source)
       }
     }
 
@@ -238,11 +240,11 @@ private[chisel3] object MonoConnect {
           if (!(connectCompileOptions.dontAssumeDirectionality)) {
             issueConnect(sink, source)
           } else {
-            throw UnreadableSourceException
+            throw UnreadableSourceException(sink, source)
           }
         }
         case (Input,        Output) if (!(connectCompileOptions.dontTryConnectionsSwapped)) => issueConnect(source, sink)
-        case (Input,        _)    => throw UnwritableSinkException
+        case (Input,        _)    => throw UnwritableSinkException(sink, source)
       }
     }
 
@@ -253,8 +255,8 @@ private[chisel3] object MonoConnect {
         //    SINK          SOURCE
         //    CHILD MOD     CURRENT MOD
         case (Input,        _) => issueConnect(sink, source)
-        case (Output,       _) => throw UnwritableSinkException
-        case (Internal,     _) => throw UnwritableSinkException
+        case (Output,       _) => throw UnwritableSinkException(sink, source)
+        case (Internal,     _) => throw UnwritableSinkException(sink, source)
       }
     }
 
@@ -268,15 +270,15 @@ private[chisel3] object MonoConnect {
         //    CHILD MOD     CHILD MOD
         case (Input,        Input)  => issueConnect(sink, source)
         case (Input,        Output) => issueConnect(sink, source)
-        case (Output,       _)      => throw UnwritableSinkException
+        case (Output,       _)      => throw UnwritableSinkException(sink, source)
         case (_,            Internal) => {
           if (!(connectCompileOptions.dontAssumeDirectionality)) {
             issueConnect(sink, source)
           } else {
-            throw UnreadableSourceException
+            throw UnreadableSourceException(sink, source)
           }
         }
-        case (Internal,     _)      => throw UnwritableSinkException
+        case (Internal,     _)      => throw UnwritableSinkException(sink, source)
       }
     }
 
diff --git a/src/test/scala/chiselTests/ConnectSpec.scala b/src/test/scala/chiselTests/ConnectSpec.scala
index 367864e6..f9ef5946 100644
--- a/src/test/scala/chiselTests/ConnectSpec.scala
+++ b/src/test/scala/chiselTests/ConnectSpec.scala
@@ -2,6 +2,8 @@
 
 package chiselTests
 
+import org.scalatest._
+
 import chisel3._
 import chisel3.experimental.{Analog, FixedPoint}
 import chisel3.stage.ChiselStage
@@ -126,4 +128,37 @@ class ConnectSpec extends ChiselPropSpec with Utils {
   property("Pipe internal connections should succeed") {
     ChiselStage.elaborate( new PipeInternalWires)
   }
+
+  property("Connect error messages should have meaningful information") {
+    class InnerExample extends Module {
+      val myReg = RegInit(0.U(8.W))
+    }
+
+    class OuterAssignExample extends Module {
+      val inner = Module(new InnerExample())
+      inner.myReg := false.B // ERROR
+    }
+
+    val assignError = the [ChiselException] thrownBy {ChiselStage.elaborate { new OuterAssignExample}}
+    val expectedAssignError = """.*@: myReg in InnerExample cannot be written from module OuterAssignExample."""
+    assignError.getMessage should fullyMatch regex expectedAssignError
+
+    class OuterReadExample extends Module {
+      val myReg = RegInit(0.U(8.W))
+      val inner = Module(new InnerExample())
+      myReg := inner.myReg // ERROR
+    }
+
+    val readError = the [ChiselException] thrownBy {ChiselStage.elaborate { new OuterReadExample }}
+    val expectedReadError = """.*@: myReg in InnerExample cannot be read from module OuterReadExample."""
+    readError.getMessage should fullyMatch regex expectedReadError
+
+    val typeMismatchError = the [ChiselException] thrownBy {ChiselStage.elaborate { new RawModule {
+      val myUInt = Wire(UInt(4.W))
+      val mySInt = Wire(SInt(4.W))
+      myUInt := mySInt
+    }}}
+    val expectedTypeMismatchError = """.*@: Sink \(UInt<4>\) and Source \(SInt<4>\) have different types."""
+    typeMismatchError.getMessage should fullyMatch regex expectedTypeMismatchError
+  }
 }
diff --git a/src/test/scala/chiselTests/InvalidateAPISpec.scala b/src/test/scala/chiselTests/InvalidateAPISpec.scala
index b7db33cc..52ad02b4 100644
--- a/src/test/scala/chiselTests/InvalidateAPISpec.scala
+++ b/src/test/scala/chiselTests/InvalidateAPISpec.scala
@@ -105,7 +105,7 @@ class InvalidateAPISpec extends ChiselPropSpec with Matchers with BackendCompila
         ChiselStage.elaborate(new ModuleWithDontCareSink)
       }
     }
-    exception.getMessage should include("DontCare cannot be a connection sink (LHS)")
+    exception.getMessage should include("DontCare cannot be a connection sink")
   }
 
   property("a DontCare cannot be a connection sink (LHS) for <>") {