Skip to content

Commit

Permalink
Report source locator in when scoping error messages (#3804)
Browse files Browse the repository at this point in the history
(cherry picked from commit d21a663)

# Conflicts:
#	core/src/main/scala/chisel3/Data.scala
  • Loading branch information
jackkoenig authored and mergify[bot] committed Feb 1, 2024
1 parent 991fa8a commit 664f713
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 17 deletions.
77 changes: 77 additions & 0 deletions core/src/main/scala/chisel3/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,69 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
*/
private[chisel3] def typeEquivalent(that: Data): Boolean

<<<<<<< HEAD
private[chisel3] def requireVisible(): Unit = {
=======
/** Find and report any type mismatches
*
* @param that Data being compared to this
* @param strictTypes Does class of Bundles or Records need to match? Inverse of "structural".
* @param strictWidths do widths need to match?
* @return None if types are equivalent, Some String reporting the first mismatch if not
*/
private[chisel3] final def findFirstTypeMismatch(
that: Data,
strictTypes: Boolean,
strictWidths: Boolean
): Option[String] = {
def rec(left: Data, right: Data): Option[String] =
(left, right) match {
// Careful, EnumTypes are Element and if we don't implement this, then they are all always equal
case (e1: EnumType, e2: EnumType) =>
// TODO, should we implement a form of structural equality for enums?
if (e1.factory == e2.factory) None
else Some(s": Left ($e1) and Right ($e2) have different types.")
// Properties should be considered equal when getPropertyType is equal, not when getClass is equal.
case (p1: Property[_], p2: Property[_]) =>
if (p1.getPropertyType != p2.getPropertyType) {
Some(s": Left ($p1) and Right ($p2) have different types")
} else {
None
}
case (e1: Element, e2: Element) if e1.getClass == e2.getClass =>
if (strictWidths && e1.width != e2.width) {
Some(s": Left ($e1) and Right ($e2) have different widths.")
} else {
None
}
case (r1: Record, r2: Record) if !strictTypes || r1.getClass == r2.getClass =>
val (larger, smaller, msg) =
if (r1._elements.size >= r2._elements.size) (r1, r2, "Left") else (r2, r1, "Right")
larger._elements.collectFirst {
case (name, data) =>
val recurse = smaller._elements.get(name) match {
case None => Some(s": Dangling field on $msg")
case Some(data2) => rec(data, data2)
}
recurse.map("." + name + _)
}.flatten
case (v1: Vec[_], v2: Vec[_]) =>
if (v1.size != v2.size) {
Some(s": Left (size ${v1.size}) and Right (size ${v2.size}) have different lengths.")
} else {
val recurse = rec(v1.sample_element, v2.sample_element)
recurse.map("[_]" + _)
}
case _ => Some(s": Left ($left) and Right ($right) have different types.")
}
val leftType = if (this.hasBinding) this.cloneType else this
val rightType = if (that.hasBinding) that.cloneType else that
rec(leftType, rightType)
}

private[chisel3] def isVisible: Boolean = isVisibleFromModule && visibleFromWhen.isEmpty
private[chisel3] def isVisibleFromModule: Boolean = {
>>>>>>> d21a66358 (Report source locator in when scoping error messages (#3804))
val mod = topBindingOpt.flatMap(_.location)
topBindingOpt match {
case Some(tb: TopBinding) if (mod == Builder.currentModule) =>
Expand All @@ -604,8 +666,23 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
case _ =>
throwException(s"operand '$this' is not visible from the current module")
}
<<<<<<< HEAD
if (!MonoConnect.checkWhenVisibility(this)) {
throwException(s"operand has escaped the scope of the when in which it was constructed")
=======
}
private[chisel3] def visibleFromWhen: Option[SourceInfo] = MonoConnect.checkWhenVisibility(this)
private[chisel3] def requireVisible(): Unit = {
if (!isVisibleFromModule) {
throwException(s"operand '$this' is not visible from the current module ${Builder.currentModule.get.name}")
}
visibleFromWhen match {
case Some(sourceInfo) =>
throwException(
s"operand '$this' has escaped the scope of the when (${sourceInfo.makeMessage(x => x)}) in which it was constructed."
)
case None => ()
>>>>>>> d21a66358 (Report source locator in when scoping error messages (#3804))
}
}

Expand Down
4 changes: 3 additions & 1 deletion core/src/main/scala/chisel3/When.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ object when {
* added by preprocessing the command queue.
*/
final class WhenContext private[chisel3] (
sourceInfo: SourceInfo,
_sourceInfo: SourceInfo,
cond: Option[() => Bool],
block: => Any,
firrtlDepth: Int,
Expand All @@ -82,6 +82,8 @@ final class WhenContext private[chisel3] (

private var scopeOpen = false

private[chisel3] def sourceInfo: SourceInfo = _sourceInfo

/** Returns the local condition, inverted for an otherwise */
private[chisel3] def localCond: Bool = {
implicit val compileOptions = ExplicitCompileOptions.Strict
Expand Down
46 changes: 30 additions & 16 deletions core/src/main/scala/chisel3/internal/MonoConnect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,14 @@ private[chisel3] object MonoConnect {
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 SourceEscapedWhenScopeException(source: Data, whenInfo: SourceInfo) =
MonoConnectException(
s"Source ${formatName(source)} has escaped the scope of the when (${whenInfo.makeMessage(x => x)}) in which it was constructed."
)
def SinkEscapedWhenScopeException(sink: Data, whenInfo: SourceInfo) =
MonoConnectException(
s"Sink ${formatName(sink)} has escaped the scope of the when (${whenInfo.makeMessage(x => x)}) in which it was constructed."
)
def UnknownRelationException =
MonoConnectException("Sink or source unavailable to current module.")
// These are when recursing down aggregate types
Expand All @@ -75,12 +79,18 @@ private[chisel3] object MonoConnect {
s"Source ${formatName(source)} and sink ${formatName(sink)} of type Analog cannot participate in a mono connection (:=)"
)

def checkWhenVisibility(x: Data): Boolean = {
/** Check if the argument is visible from current when scope
*
* Returns source locator of original when declaration if not visible (for error reporting)
*
* @return None if visible, Some(location of original when declaration)
*/
def checkWhenVisibility(x: Data): Option[SourceInfo] = {
x.topBinding match {
case mp: MemoryPortBinding =>
true // TODO (albert-magyar): remove this "bridge" for odd enable logic of current CHIRRTL memories
case cd: ConditionalDeclarable => cd.visibility.map(_.active).getOrElse(true)
case _ => true
None // TODO (albert-magyar): remove this "bridge" for odd enable logic of current CHIRRTL memories
case cd: ConditionalDeclarable => cd.visibility.collect { case wc: WhenContext if !wc.active => wc.sourceInfo }
case _ => None
}
}

Expand Down Expand Up @@ -256,12 +266,14 @@ private[chisel3] object MonoConnect {
case _ => false
}

if (!checkWhenVisibility(sink)) {
throw SinkEscapedWhenScopeException(sink)
checkWhenVisibility(sink) match {
case Some(whenInfo) => throw SinkEscapedWhenScopeException(sink, whenInfo)
case None => ()
}

if (!checkWhenVisibility(source)) {
throw SourceEscapedWhenScopeException(source)
checkWhenVisibility(source) match {
case Some(whenInfo) => throw SourceEscapedWhenScopeException(source, whenInfo)
case None => ()
}

// CASE: Context is same module that both sink node and source node are in
Expand Down Expand Up @@ -411,12 +423,14 @@ private[chisel3] object MonoConnect {
val sink_direction = BindingDirection.from(sink.topBinding, sink.direction)
val source_direction = BindingDirection.from(source.topBinding, source.direction)

if (!checkWhenVisibility(sink)) {
throw SinkEscapedWhenScopeException(sink)
checkWhenVisibility(sink) match {
case Some(whenInfo) => throw SinkEscapedWhenScopeException(sink, whenInfo)
case None => ()
}

if (!checkWhenVisibility(source)) {
throw SourceEscapedWhenScopeException(source)
checkWhenVisibility(source) match {
case Some(whenInfo) => throw SourceEscapedWhenScopeException(source, whenInfo)
case None => ()
}

// CASE: Context is same module that both left node and right node are in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import circt.stage.ChiselStage
import chisel3._
import chisel3.testers.BasicTester
import chisel3.util._
import chisel3.experimental.{SourceInfo, SourceLine}

class WhenTester() extends BasicTester {
val cnt = Counter(4)
Expand Down Expand Up @@ -165,4 +166,42 @@ class WhenSpec extends ChiselFlatSpec with Utils {
}
e.getMessage should include("Cannot exit from a when() block with a \"return\"")
}

"Using a value that has escaped from a when scope in a connection" should "give a reasonable error message" in {
implicit val info: SourceInfo = SourceLine("Foo.scala", 12, 3)
val e = the[ChiselException] thrownBy {
ChiselStage.emitCHIRRTL(new Module {
override def desiredName = "Top"
val foo, bar = IO(Output(UInt(8.W)))
val a = IO(Input(Bool()))
lazy val w = Wire(UInt(8.W))
when(a) {
foo := w
}
bar := w
})
}
val msg =
"Source foo_w in Top has escaped the scope of the when (@[Foo.scala 12:3]) in which it was constructed."
e.getMessage should include(msg)
}

"Using a value that has escaped from a when scope in an operation" should "give a reasonable error message" in {
implicit val info: SourceInfo = SourceLine("Foo.scala", 12, 3)
val e = the[ChiselException] thrownBy {
ChiselStage.emitCHIRRTL(new Module {
override def desiredName = "Top"
val foo, bar = IO(Output(UInt(8.W)))
val a = IO(Input(Bool()))
lazy val w = Wire(UInt(8.W))
when(a) {
foo := w
}
bar := w + 1.U
})
}
val msg =
"operand 'Top.foo_w: Wire[UInt<8>]' has escaped the scope of the when (@[Foo.scala 12:3]) in which it was constructed."
e.getMessage should include(msg)
}
}

0 comments on commit 664f713

Please sign in to comment.