Skip to content

Commit

Permalink
Error rather than throw on scope violation issues.
Browse files Browse the repository at this point in the history
Errors are aggregated by default and all reported at the end whereas
exceptions throw immediately. This allows users to fix multiple scope
violations at the same time.
  • Loading branch information
jackkoenig committed Jan 2, 2025
1 parent e461069 commit 27aa9d9
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 88 deletions.
5 changes: 5 additions & 0 deletions core/src/main/scala/chisel3/BitsImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -142,24 +142,29 @@ private[chisel3] trait BitsImpl extends Element { self: Bits =>
_applyImpl(castToInt(x, "High index"), castToInt(y, "Low index"))

private[chisel3] def unop[T <: Data](sourceInfo: SourceInfo, dest: T, op: PrimOp): T = {
implicit val info: SourceInfo = sourceInfo
requireIsHardware(this, "bits operated on")
pushOp(DefPrim(sourceInfo, dest, op, this.ref))
}
private[chisel3] def binop[T <: Data](sourceInfo: SourceInfo, dest: T, op: PrimOp, other: BigInt): T = {
implicit val info: SourceInfo = sourceInfo
requireIsHardware(this, "bits operated on")
pushOp(DefPrim(sourceInfo, dest, op, this.ref, ILit(other)))
}
private[chisel3] def binop[T <: Data](sourceInfo: SourceInfo, dest: T, op: PrimOp, other: Bits): T = {
implicit val info: SourceInfo = sourceInfo
requireIsHardware(this, "bits operated on")
requireIsHardware(other, "bits operated on")
pushOp(DefPrim(sourceInfo, dest, op, this.ref, other.ref))
}
private[chisel3] def compop(sourceInfo: SourceInfo, op: PrimOp, other: Bits): Bool = {
implicit val info: SourceInfo = sourceInfo
requireIsHardware(this, "bits operated on")
requireIsHardware(other, "bits operated on")
pushOp(DefPrim(sourceInfo, Bool(), op, this.ref, other.ref))
}
private[chisel3] def redop(sourceInfo: SourceInfo, op: PrimOp): Bool = {
implicit val info: SourceInfo = sourceInfo
requireIsHardware(this, "bits operated on")
pushOp(DefPrim(sourceInfo, Bool(), op, this.ref))
}
Expand Down
1 change: 1 addition & 0 deletions core/src/main/scala/chisel3/ChiselEnumImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ private[chisel3] abstract class EnumTypeImpl(private[chisel3] val factory: Chise
override def cloneType: this.type = factory().asInstanceOf[this.type]

private[chisel3] def compop(sourceInfo: SourceInfo, op: PrimOp, other: EnumType): Bool = {
implicit val info: SourceInfo = sourceInfo
requireIsHardware(this, "bits operated on")
requireIsHardware(other, "bits operated on")

Expand Down
11 changes: 4 additions & 7 deletions core/src/main/scala/chisel3/DataImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -697,16 +697,13 @@ private[chisel3] trait DataImpl extends HasId with NamedComponent { self: Data =
}
}
private[chisel3] def visibleFromBlock: Option[SourceInfo] = MonoConnect.checkBlockVisibility(this)
private[chisel3] def requireVisible(): Unit = {
private[chisel3] def requireVisible()(implicit info: SourceInfo): Unit = {
if (!isVisibleFromModule) {
throwException(s"operand '$this' is not visible from the current module ${Builder.currentModule.get.name}")
}
visibleFromBlock match {
case Some(sourceInfo) =>
throwException(
s"operand '$this' has escaped the scope of the block (${sourceInfo.makeMessage()}) in which it was constructed."
)
case None => ()
case Some(blockInfo) => MonoConnect.escapedScopeError(this, blockInfo)
case None => ()
}
}

Expand All @@ -727,7 +724,7 @@ private[chisel3] trait DataImpl extends HasId with NamedComponent { self: Data =
}

// Internal API: returns a ref, if bound
private[chisel3] def ref: Arg = {
private[chisel3] def ref(implicit info: SourceInfo): Arg = {
def materializeWire(makeConst: Boolean = false): Arg = {
if (!Builder.currentModule.isDefined) throwException(s"internal error: cannot materialize ref for $this")
implicit val sourceInfo = UnlocatableSourceInfo
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/scala/chisel3/MemImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,14 @@ private[chisel3] trait MemBaseImpl[T <: Data] extends HasId with NamedComponent
dir: MemPortDirection,
clock: Clock
): T = {
implicit val info: SourceInfo = sourceInfo
if (Builder.currentModule != _parent) {
throwException(
s"Cannot create a memory port in a different module (${Builder.currentModule.get.name}) than where the memory is (${_parent.get.name})."
)
}
requireIsHardware(idx, "memory port index")
val i = Vec.truncateIndex(idx, length)(sourceInfo)
val i = Vec.truncateIndex(idx, length)

val port = pushCommand(
DefMemPort(sourceInfo, t.cloneTypeFull, Node(this), dir, i.ref, clock.ref)
Expand Down
18 changes: 10 additions & 8 deletions core/src/main/scala/chisel3/Printable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

package chisel3

import chisel3.experimental.SourceInfo
import chisel3.internal.firrtl.ir.Component

import scala.collection.mutable
Expand Down Expand Up @@ -49,7 +50,7 @@ sealed abstract class Printable {
* @note This must be called after elaboration when Chisel nodes actually
* have names
*/
def unpack(ctx: Component): (String, Iterable[String])
def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String])

/** Unpack into a Seq of captured Bits arguments
*/
Expand Down Expand Up @@ -139,7 +140,7 @@ object Printable {
StringContext(bufEscapeBackSlash.toSeq: _*).cf(data: _*)
}

private[chisel3] def checkScope(message: Printable): Unit = {
private[chisel3] def checkScope(message: Printable)(implicit info: SourceInfo): Unit = {
def getData(x: Printable): Seq[Data] = {
x match {
case y: FirrtlFormat => Seq(y.bits)
Expand All @@ -155,7 +156,7 @@ object Printable {

case class Printables(pables: Iterable[Printable]) extends Printable {
require(pables.hasDefiniteSize, "Infinite-sized iterables are not supported!")
final def unpack(ctx: Component): (String, Iterable[String]) = {
final def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String]) = {
val (fmts, args) = pables.map(_.unpack(ctx)).unzip
(fmts.mkString, args.flatten)
}
Expand All @@ -165,7 +166,7 @@ case class Printables(pables: Iterable[Printable]) extends Printable {

/** Wrapper for printing Scala Strings */
case class PString(str: String) extends Printable {
final def unpack(ctx: Component): (String, Iterable[String]) =
final def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String]) =
(str.replaceAll("%", "%%"), List.empty)

final def unpackArgs: Seq[Bits] = List.empty
Expand All @@ -174,7 +175,7 @@ case class PString(str: String) extends Printable {
/** Superclass for Firrtl format specifiers for Bits */
sealed abstract class FirrtlFormat(private[chisel3] val specifier: Char) extends Printable {
def bits: Bits
def unpack(ctx: Component): (String, Iterable[String]) = {
def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String]) = {
(s"%$specifier", List(bits.ref.fullName(ctx)))
}

Expand Down Expand Up @@ -218,18 +219,19 @@ 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(ctx: Component): (String, Iterable[String]) = (data.ref.name, List.empty)
final def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String]) = (data.ref.name, List.empty)
final def unpackArgs: Seq[Bits] = List.empty
}

/** Put full name within parent namespace (eg. bundleName.field) */
case class FullName(data: Data) extends Printable {
final def unpack(ctx: Component): (String, Iterable[String]) = (data.ref.fullName(ctx), List.empty)
final def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String]) =
(data.ref.fullName(ctx), List.empty)
final def unpackArgs: Seq[Bits] = List.empty
}

/** Represents escaped percents */
case object Percent extends Printable {
final def unpack(ctx: Component): (String, Iterable[String]) = ("%%", List.empty)
final def unpack(ctx: Component)(implicit info: SourceInfo): (String, Iterable[String]) = ("%%", List.empty)
final def unpackArgs: Seq[Bits] = List.empty
}
2 changes: 1 addition & 1 deletion core/src/main/scala/chisel3/When.scala
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ final class WhenContext private[chisel3] (
// Create the `When` operation and run the `block` thunk inside the
// `ifRegion`. Any commands that this thunk creates will be put inside this
// block.
private val whenCommand = pushCommand(new When(sourceInfo, cond().ref))
private val whenCommand = pushCommand(new When(sourceInfo, cond().ref(sourceInfo)))
Builder.pushWhen(this)
scope = Some(Scope.If)
try {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private[chisel3] trait HasId extends chisel3.InstanceId {
private[chisel3] def setRef(parent: Node, index: Int): Unit = setRef(LitIndex(parent, index))
private[chisel3] def setRef(parent: Node, index: UInt): Unit = index.litOption match {
case Some(lit) if lit.isValidInt => setRef(LitIndex(parent, lit.intValue))
case _ => setRef(Index(parent, index.ref))
case _ => setRef(Index(parent, index.ref(UnlocatableSourceInfo)))
}
private[chisel3] def getRef: Arg = _ref.get
private[chisel3] def getOptionRef: Option[Arg] = _ref
Expand Down
36 changes: 16 additions & 20 deletions core/src/main/scala/chisel3/internal/MonoConnect.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,10 @@ private[chisel3] object MonoConnect {
MonoConnectException(
s"""${formatName(sink)} cannot be written from module ${source.parentNameOpt.getOrElse("(unknown)")}."""
)
def SourceEscapedBlockScopeException(source: Data, blockInfo: SourceInfo) =
MonoConnectException(
s"Source ${formatName(source)} has escaped the scope of the block (${blockInfo.makeMessage()}) in which it was constructed."
)
def SinkEscapedBlockScopeException(sink: Data, blockInfo: SourceInfo) =
MonoConnectException(
s"Sink ${formatName(sink)} has escaped the scope of the block (${blockInfo.makeMessage()}) in which it was constructed."
)
def escapedScopeError(data: Data, blockInfo: SourceInfo)(implicit lineInfo: SourceInfo): Unit = {
val msg = s"'$data' has escaped the scope of the block (${blockInfo.makeMessage()}) in which it was constructed."
Builder.error(msg)
}
def UnknownRelationException =
MonoConnectException("Sink or source unavailable to current module.")
// These are when recursing down aggregate types
Expand Down Expand Up @@ -177,7 +173,7 @@ private[chisel3] object MonoConnect {
context_mod
)
) {
pushCommand(Connect(sourceInfo, sinkReified.get.lref(sourceInfo), sourceReified.get.ref))
pushCommand(Connect(sourceInfo, sinkReified.get.lref(sourceInfo), sourceReified.get.ref(sourceInfo)))
} else {
for (idx <- 0 until sink_v.length) {
try {
Expand Down Expand Up @@ -212,7 +208,7 @@ private[chisel3] object MonoConnect {
context_mod
)
) {
pushCommand(Connect(sourceInfo, sinkReified.get.lref(sourceInfo), sourceReified.get.ref))
pushCommand(Connect(sourceInfo, sinkReified.get.lref(sourceInfo), sourceReified.get.ref(sourceInfo)))
} else {
// For each field, descend with right
for ((field, sink_sub) <- sink_r._elements) {
Expand Down Expand Up @@ -288,12 +284,12 @@ private[chisel3] object MonoConnect {
}

checkBlockVisibility(sink) match {
case Some(blockInfo) => throw SinkEscapedBlockScopeException(sink, blockInfo)
case Some(blockInfo) => escapedScopeError(sink, blockInfo)
case None => ()
}

checkBlockVisibility(source) match {
case Some(blockInfo) => throw SourceEscapedBlockScopeException(source, blockInfo)
case Some(blockInfo) => escapedScopeError(source, blockInfo)
case None => ()
}

Expand Down Expand Up @@ -440,6 +436,7 @@ private[chisel3] object MonoConnect {
sourceProp: Property[_],
context: BaseModule
): Unit = {
implicit val info: SourceInfo = sourceInfo
// Reify sink and source if they're views.
val (sink, writable) = reify(sinkProp)
val (source, _) = reify(sourceProp)
Expand All @@ -449,11 +446,11 @@ private[chisel3] object MonoConnect {
context match {
case rm: RawModule =>
writable.reportIfReadOnlyUnit {
rm.addCommand(PropAssign(sourceInfo, sink.lref(sourceInfo), source.ref))
rm.addCommand(PropAssign(sourceInfo, sink.lref, source.ref))
}(sourceInfo)
case cls: Class =>
writable.reportIfReadOnlyUnit {
cls.addCommand(PropAssign(sourceInfo, sink.lref(sourceInfo), source.ref))
cls.addCommand(PropAssign(sourceInfo, sink.lref, source.ref))
}(sourceInfo)
case _ => throwException("Internal Error! Property connection can only occur within RawModule or Class.")
}
Expand All @@ -465,7 +462,7 @@ private[chisel3] object MonoConnect {
sourceProbe: Data,
context: BaseModule
): Unit = {

implicit val info: SourceInfo = sourceInfo
val (sink, writable) = reifyIdentityView(sinkProbe).getOrElse(
throwException(
s"If a DataView contains a Probe, it must resolve to one Data. $sinkProbe does not meet this criteria."
Expand All @@ -480,7 +477,7 @@ private[chisel3] object MonoConnect {
context match {
case rm: RawModule =>
writable.reportIfReadOnlyUnit {
rm.addCommand(ProbeDefine(sourceInfo, sink.lref(sourceInfo), source.ref))
rm.addCommand(ProbeDefine(sourceInfo, sink.lref, source.ref))
}(sourceInfo) // Nothing to push if an error.
case _ => throwException("Internal Error! Probe connection can only occur within RawModule.")
}
Expand Down Expand Up @@ -519,8 +516,7 @@ private[chisel3] object checkConnect {
// Import helpers and exception types.
import MonoConnect.{
checkBlockVisibility,
SinkEscapedBlockScopeException,
SourceEscapedBlockScopeException,
escapedScopeError,
UnknownRelationException,
UnreadableSourceException,
UnwritableSinkException
Expand Down Expand Up @@ -598,12 +594,12 @@ private[chisel3] object checkConnect {
else throw UnknownRelationException

checkBlockVisibility(sink) match {
case Some(blockInfo) => throw SinkEscapedBlockScopeException(sink, blockInfo)
case Some(blockInfo) => escapedScopeError(sink, blockInfo)(sourceInfo)
case None => ()
}

checkBlockVisibility(source) match {
case Some(blockInfo) => throw SourceEscapedBlockScopeException(source, blockInfo)
case Some(blockInfo) => escapedScopeError(source, blockInfo)(sourceInfo)
case None => ()
}
}
Expand Down
29 changes: 16 additions & 13 deletions core/src/main/scala/chisel3/internal/firrtl/Converter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,19 @@ import scala.collection.immutable.{Queue, VectorBuilder, VectorMap}

private[chisel3] object Converter {
// TODO modeled on unpack method on Printable, refactor?
def unpack(pable: Printable, ctx: Component): (String, Seq[Arg]) = pable match {
case Printables(pables) =>
val (fmts, args) = pables.map(p => unpack(p, ctx)).unzip
(fmts.mkString, args.flatten.toSeq)
case PString(str) => (str.replaceAll("%", "%%"), List.empty)
case format: FirrtlFormat =>
("%" + format.specifier, List(format.bits.ref))
case Name(data) => (data.ref.name, List.empty)
case FullName(data) => (data.ref.fullName(ctx), List.empty)
case Percent => ("%%", List.empty)
def unpack(pable: Printable, ctx: Component, sourceInfo: SourceInfo): (String, Seq[Arg]) = {
implicit val info: SourceInfo = sourceInfo
pable match {
case Printables(pables) =>
val (fmts, args) = pables.map(p => unpack(p, ctx, sourceInfo)).unzip
(fmts.mkString, args.flatten.toSeq)
case PString(str) => (str.replaceAll("%", "%%"), List.empty)
case format: FirrtlFormat =>
("%" + format.specifier, List(format.bits.ref))
case Name(data) => (data.ref.name, List.empty)
case FullName(data) => (data.ref.fullName(ctx), List.empty)
case Percent => ("%%", List.empty)
}
}

private def reportInternalError(msg: String): Nothing = {
Expand Down Expand Up @@ -191,7 +194,7 @@ private[chisel3] object Converter {
case e @ Stop(_, info, clock, ret) =>
fir.Stop(convert(info), ret, convert(clock, ctx, info), firrtl.Utils.one, e.name)
case e @ Printf(_, info, clock, pable) =>
val (fmt, args) = unpack(pable, ctx)
val (fmt, args) = unpack(pable, ctx, info)
fir.Print(
convert(info),
fir.StringLit(fmt),
Expand Down Expand Up @@ -222,7 +225,7 @@ private[chisel3] object Converter {
convert(probe, ctx, sourceInfo)
)
case e @ Verification(_, op, info, clk, pred, pable) =>
val (fmt, args) = unpack(pable, ctx)
val (fmt, args) = unpack(pable, ctx, info)
val firOp = op match {
case Formal.Assert => fir.Formal.Assert
case Formal.Assume => fir.Formal.Assume
Expand Down Expand Up @@ -388,7 +391,7 @@ private[chisel3] object Converter {
case StringParam(value) => fir.StringParam(name, fir.StringLit(value))
case PrintableParam(value, id) => {
val ctx = id._component.get
val (fmt, _) = unpack(value, ctx)
val (fmt, _) = unpack(value, ctx, UnlocatableSourceInfo)
fir.StringParam(name, fir.StringLit(fmt))
}
case RawParam(value) => fir.RawStringParam(name, value)
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/scala/chisel3/properties/Class.scala
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ case class ClassType private[chisel3] (name: String) { self =>
override def convert(value: Underlying, ctx: Component, info: SourceInfo): fir.Expression =
Converter.convert(value, ctx, info)
type Underlying = Arg
override def convertUnderlying(value: Property[ClassType] with self.Type) = value.ref
override def convertUnderlying(value: Property[ClassType] with self.Type, info: SourceInfo) = value.ref(info)
}
}
def copy(name: String = this.name) = new ClassType(name)
Expand All @@ -147,7 +147,7 @@ object AnyClassType {
override def convert(value: Underlying, ctx: Component, info: SourceInfo): fir.Expression =
Converter.convert(value, ctx, info)
type Underlying = Arg
override def convertUnderlying(value: Property[ClassType] with AnyClassType) = value.ref
override def convertUnderlying(value: Property[ClassType] with AnyClassType, info: SourceInfo) = value.ref(info)
}
}

Expand Down
Loading

0 comments on commit 27aa9d9

Please sign in to comment.