Check whether signals escape their when scopes (#1518)

* Include and check when scoping as part of reg/mem/wire/node bindings

* Allow outdated 'when' behavior of CHIRRTL memory ports with enables

* Extend cross-module / when-visibility checks to all data refs

* Fixes #1512
* Cannot be checked if outside a module context
* E.g. delayed evaluation of printf / assert args

* Add basic test cases for cross-module refs / signals escaping when scopes

* Remove illegal cross-module references from existing tests

9 files changed, 96 insertions, 28 deletions

diff --git a/core/src/main/scala/chisel3/Data.scala b/core/src/main/scala/chisel3/Data.scala
index 983307a6..b7859e91 100644
--- a/core/src/main/scala/chisel3/Data.scala
+++ b/core/src/main/scala/chisel3/Data.scala
@@ -376,16 +376,16 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
   // TODO Is this okay for sample_element? It *shouldn't* be visible to users
   protected def bindingToString: String = topBindingOpt match {
     case None => ""
-    case Some(OpBinding(enclosure)) => s"(OpResult in ${enclosure.desiredName})"
-    case Some(MemoryPortBinding(enclosure)) => s"(MemPort in ${enclosure.desiredName})"
+    case Some(OpBinding(enclosure, _)) => s"(OpResult in ${enclosure.desiredName})"
+    case Some(MemoryPortBinding(enclosure, _)) => s"(MemPort in ${enclosure.desiredName})"
     case Some(PortBinding(enclosure)) if !enclosure.isClosed => s"(IO in unelaborated ${enclosure.desiredName})"
     case Some(PortBinding(enclosure)) if enclosure.isClosed =>
       DataMirror.fullModulePorts(enclosure).find(_._2 eq this) match {
         case Some((name, _)) => s"(IO $name in ${enclosure.desiredName})"
         case None => s"(IO (unknown) in ${enclosure.desiredName})"
       }
-    case Some(RegBinding(enclosure)) => s"(Reg in ${enclosure.desiredName})"
-    case Some(WireBinding(enclosure)) => s"(Wire in ${enclosure.desiredName})"
+    case Some(RegBinding(enclosure, _)) => s"(Reg in ${enclosure.desiredName})"
+    case Some(WireBinding(enclosure, _)) => s"(Wire in ${enclosure.desiredName})"
     case Some(DontCareBinding()) => s"(DontCare)"
     case Some(ElementLitBinding(litArg)) => s"(unhandled literal)"
     case Some(BundleLitBinding(litMap)) => s"(unhandled bundle literal)"
@@ -446,9 +446,23 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
     */
   private[chisel3] def typeEquivalent(that: Data): Boolean
 
+  private def requireVisible(): Unit = {
+    val mod = topBindingOpt.flatMap(_.location)
+    topBindingOpt match {
+      case Some(tb: TopBinding) if (mod == Builder.currentModule) =>
+      case Some(pb: PortBinding) if (mod.flatMap(_._parent) == Builder.currentModule) =>
+      case _ =>
+        throwException(s"operand is not visible from the current module")
+    }
+    if (!MonoConnect.checkWhenVisibility(this)) {
+      throwException(s"operand has escaped the scope of the when in which it was constructed")
+    }
+  }
+
   // Internal API: returns a ref that can be assigned to, if consistent with the binding
   private[chisel3] def lref: Node = {
     requireIsHardware(this)
+    requireVisible()
     topBindingOpt match {
       case Some(binding: ReadOnlyBinding) => throwException(s"internal error: attempted to generate LHS ref to ReadOnlyBinding $binding")
       case Some(binding: TopBinding) => Node(this)
@@ -460,6 +474,11 @@ abstract class Data extends HasId with NamedComponent with SourceInfoDoc {
   // Internal API: returns a ref, if bound. Literals should override this as needed.
   private[chisel3] def ref: Arg = {
     requireIsHardware(this)
+    if (Builder.currentModule.isDefined) {
+      // This is allowed (among other cases) for evaluating args of Printf / Assert / Printable, which are
+      // partially resolved *after* elaboration completes. If this is resolved, the check should be unconditional.
+      requireVisible()
+    }
     topBindingOpt match {
       case Some(binding: LitBinding) => throwException(s"internal error: can't handle literal binding $binding")
       case Some(binding: TopBinding) => Node(this)
@@ -595,7 +614,7 @@ trait WireFactory {
     val x = t.cloneTypeFull
 
     // Bind each element of x to being a Wire
-    x.bind(WireBinding(Builder.forcedUserModule))
+    x.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen()))
 
     pushCommand(DefWire(sourceInfo, x))
     if (!compileOptions.explicitInvalidate) {
diff --git a/core/src/main/scala/chisel3/Mem.scala b/core/src/main/scala/chisel3/Mem.scala
index 24ab4b8e..ba9dc34b 100644
--- a/core/src/main/scala/chisel3/Mem.scala
+++ b/core/src/main/scala/chisel3/Mem.scala
@@ -127,7 +127,7 @@ sealed abstract class MemBase[T <: Data](val t: T, val length: BigInt) extends H
        t.cloneTypeFull, Node(this), dir, i.ref, Builder.forcedClock.ref)
     ).id
     // Bind each element of port to being a MemoryPort
-    port.bind(MemoryPortBinding(Builder.forcedUserModule))
+    port.bind(MemoryPortBinding(Builder.forcedUserModule, Builder.currentWhen()))
     port
   }
 }
diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala
index 25037b00..194c546c 100644
--- a/core/src/main/scala/chisel3/Module.scala
+++ b/core/src/main/scala/chisel3/Module.scala
@@ -37,7 +37,7 @@ object Module extends SourceInfoDoc {
     Builder.readyForModuleConstr = true
 
     val parent = Builder.currentModule
-    val whenDepth: Int = Builder.whenDepth
+    val parentWhenStack = Builder.whenStack
 
     // Save then clear clock and reset to prevent leaking scope, must be set again in the Module
     val (saveClock, saveReset)  = (Builder.currentClock, Builder.currentReset)
@@ -49,7 +49,7 @@ object Module extends SourceInfoDoc {
     // Execute the module, this has the following side effects:
     //   - set currentModule
     //   - unset readyForModuleConstr
-    //   - reset whenDepth to 0
+    //   - reset whenStack to be empty
     //   - set currentClockAndReset
     val module: T = bc  // bc is actually evaluated here
 
@@ -62,7 +62,7 @@ object Module extends SourceInfoDoc {
                      sourceInfo.makeMessage(" See " + _))
     }
     Builder.currentModule = parent // Back to parent!
-    Builder.whenDepth = whenDepth
+    Builder.whenStack = parentWhenStack
     Builder.currentClock = saveClock   // Back to clock and reset scope
     Builder.currentReset = saveReset
 
@@ -137,7 +137,7 @@ package internal {
     private[chisel3] def cloneIORecord(proto: BaseModule)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): ClonePorts = {
       require(proto.isClosed, "Can't clone a module before module close")
       val clonePorts = new ClonePorts(proto.getModulePorts: _*)
-      clonePorts.bind(WireBinding(Builder.forcedUserModule))
+      clonePorts.bind(WireBinding(Builder.forcedUserModule, Builder.currentWhen()))
       val cloneInstance = new DefInstance(sourceInfo, proto, proto._component.get.ports) {
         override def name = clonePorts.getRef.name
       }
@@ -169,7 +169,7 @@ package experimental {
     readyForModuleConstr = false
 
     Builder.currentModule = Some(this)
-    Builder.whenDepth = 0
+    Builder.whenStack = Nil
 
     //
     // Module Construction Internals
diff --git a/core/src/main/scala/chisel3/RawModule.scala b/core/src/main/scala/chisel3/RawModule.scala
index 5b609384..a1006594 100644
--- a/core/src/main/scala/chisel3/RawModule.scala
+++ b/core/src/main/scala/chisel3/RawModule.scala
@@ -80,15 +80,15 @@ abstract class RawModule(implicit moduleCompileOptions: CompileOptions)
         case id: Data  =>
           if (id.isSynthesizable) {
             id.topBinding match {
-              case OpBinding(_) =>
+              case OpBinding(_, _) =>
                 id.forceName(Some(""), default="T", _namespace)
-              case MemoryPortBinding(_) =>
+              case MemoryPortBinding(_, _) =>
                 id.forceName(None, default="MPORT", _namespace)
               case PortBinding(_) =>
                 id.forceName(None, default="PORT", _namespace)
-              case RegBinding(_) =>
+              case RegBinding(_, _) =>
                 id.forceName(None, default="REG", _namespace)
-              case WireBinding(_) =>
+              case WireBinding(_, _) =>
                 id.forceName(Some(""), default="WIRE", _namespace)
               case _ =>  // don't name literals
             }
diff --git a/core/src/main/scala/chisel3/Reg.scala b/core/src/main/scala/chisel3/Reg.scala
index 7129c389..f57281d6 100644
--- a/core/src/main/scala/chisel3/Reg.scala
+++ b/core/src/main/scala/chisel3/Reg.scala
@@ -41,7 +41,7 @@ object Reg {
     val reg = t.cloneTypeFull
     val clock = Node(Builder.forcedClock)
 
-    reg.bind(RegBinding(Builder.forcedUserModule))
+    reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentWhen()))
     pushCommand(DefReg(sourceInfo, reg, clock))
     reg
   }
@@ -174,7 +174,7 @@ object RegInit {
     val clock = Builder.forcedClock
     val reset = Builder.forcedReset
 
-    reg.bind(RegBinding(Builder.forcedUserModule))
+    reg.bind(RegBinding(Builder.forcedUserModule, Builder.currentWhen()))
     requireIsHardware(init, "reg initializer")
     pushCommand(DefRegInit(sourceInfo, reg, clock.ref, reset.ref, init.ref))
     reg
diff --git a/core/src/main/scala/chisel3/When.scala b/core/src/main/scala/chisel3/When.scala
index b24bdb35..9e6accbc 100644
--- a/core/src/main/scala/chisel3/When.scala
+++ b/core/src/main/scala/chisel3/When.scala
@@ -45,6 +45,8 @@ object when {
   */
 final class WhenContext(sourceInfo: SourceInfo, cond: Option[() => Bool], block: => Any, firrtlDepth: Int = 0) {
 
+  private var scopeOpen = false
+
   /** This block of logic gets executed if above conditions have been
     * false and this condition is true. The lazy argument pattern
     * makes it possible to delay evaluation of cond, emitting the
@@ -65,13 +67,16 @@ final class WhenContext(sourceInfo: SourceInfo, cond: Option[() => Bool], block:
   def otherwise(block: => Any)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Unit =
     new WhenContext(sourceInfo, None, block, firrtlDepth + 1)
 
+  def active(): Boolean = scopeOpen
+
   /*
    *
    */
   if (firrtlDepth > 0) { pushCommand(AltBegin(sourceInfo)) }
   cond.foreach( c => pushCommand(WhenBegin(sourceInfo, c().ref)) )
-  Builder.whenDepth += 1
+  Builder.pushWhen(this)
   try {
+    scopeOpen = true
     block
   } catch {
     case ret: scala.runtime.NonLocalReturnControl[_] =>
@@ -79,7 +84,8 @@ final class WhenContext(sourceInfo: SourceInfo, cond: Option[() => Bool], block:
         " Perhaps you meant to use Mux or a Wire as a return value?"
       )
   }
-  Builder.whenDepth -= 1
+  scopeOpen = false
+  Builder.popWhen()
   cond.foreach( c => pushCommand(WhenEnd(sourceInfo,firrtlDepth)) )
   if (cond.isEmpty) { pushCommand(OtherwiseEnd(sourceInfo,firrtlDepth)) }
 }
diff --git a/core/src/main/scala/chisel3/internal/Binding.scala b/core/src/main/scala/chisel3/internal/Binding.scala
index 07c44f9f..95bd40e5 100644
--- a/core/src/main/scala/chisel3/internal/Binding.scala
+++ b/core/src/main/scala/chisel3/internal/Binding.scala
@@ -88,13 +88,20 @@ sealed trait ConstrainedBinding extends TopBinding {
 // A binding representing a data that cannot be (re)assigned to.
 sealed trait ReadOnlyBinding extends TopBinding
 
+// A component that can potentially be declared inside a 'when'
+sealed trait ConditionalDeclarable extends TopBinding {
+  def visibility: Option[WhenContext]
+}
+
 // TODO(twigg): Ops between unenclosed nodes can also be unenclosed
 // However, Chisel currently binds all op results to a module
-case class OpBinding(enclosure: RawModule) extends ConstrainedBinding with ReadOnlyBinding
-case class MemoryPortBinding(enclosure: RawModule) extends ConstrainedBinding
+
 case class PortBinding(enclosure: BaseModule) extends ConstrainedBinding
-case class RegBinding(enclosure: RawModule) extends ConstrainedBinding
-case class WireBinding(enclosure: RawModule) extends ConstrainedBinding
+
+case class OpBinding(enclosure: RawModule, visibility: Option[WhenContext]) extends ConstrainedBinding with ReadOnlyBinding with ConditionalDeclarable
+case class MemoryPortBinding(enclosure: RawModule, visibility: Option[WhenContext]) extends ConstrainedBinding with ConditionalDeclarable
+case class RegBinding(enclosure: RawModule, visibility: Option[WhenContext]) extends ConstrainedBinding with ConditionalDeclarable
+case class WireBinding(enclosure: RawModule, visibility: Option[WhenContext]) extends ConstrainedBinding with ConditionalDeclarable
 
 case class ChildBinding(parent: Data) extends Binding {
   def location: Option[BaseModule] = parent.topBinding.location
diff --git a/core/src/main/scala/chisel3/internal/Builder.scala b/core/src/main/scala/chisel3/internal/Builder.scala
index ceb14900..3c6d7290 100644
--- a/core/src/main/scala/chisel3/internal/Builder.scala
+++ b/core/src/main/scala/chisel3/internal/Builder.scala
@@ -332,7 +332,7 @@ private[chisel3] class DynamicContext() {
   // Set by object Module.apply before calling class Module constructor
   // Used to distinguish between no Module() wrapping, multiple wrappings, and rewrapping
   var readyForModuleConstr: Boolean = false
-  var whenDepth: Int = 0 // Depth of when nesting
+  var whenStack: List[WhenContext] = Nil
   var currentClock: Option[Clock] = None
   var currentReset: Option[Reset] = None
   val errors = new ErrorLog
@@ -461,10 +461,26 @@ private[chisel3] object Builder {
   def readyForModuleConstr_=(target: Boolean): Unit = {
     dynamicContext.readyForModuleConstr = target
   }
-  def whenDepth: Int = dynamicContext.whenDepth
-  def whenDepth_=(target: Int): Unit = {
-    dynamicContext.whenDepth = target
+
+  def whenDepth: Int = dynamicContext.whenStack.length
+
+  def pushWhen(wc: WhenContext): Unit = {
+    dynamicContext.whenStack = wc :: dynamicContext.whenStack
+  }
+
+  def popWhen(): WhenContext = {
+    val lastWhen = dynamicContext.whenStack.head
+    dynamicContext.whenStack = dynamicContext.whenStack.tail
+    lastWhen
   }
+
+  def whenStack: List[WhenContext] = dynamicContext.whenStack
+  def whenStack_=(s: List[WhenContext]): Unit = {
+    dynamicContext.whenStack = s
+  }
+
+  def currentWhen(): Option[WhenContext] = dynamicContext.whenStack.headOption
+
   def currentClock: Option[Clock] = dynamicContext.currentClock
   def currentClock_=(newClock: Option[Clock]): Unit = {
     dynamicContext.currentClock = newClock
@@ -490,7 +506,7 @@ private[chisel3] object Builder {
   }
   def pushOp[T <: Data](cmd: DefPrim[T]): T = {
     // Bind each element of the returned Data to being a Op
-    cmd.id.bind(OpBinding(forcedUserModule))
+    cmd.id.bind(OpBinding(forcedUserModule, currentWhen()))
     pushCommand(cmd).id
   }
 
diff --git a/core/src/main/scala/chisel3/internal/MonoConnect.scala b/core/src/main/scala/chisel3/internal/MonoConnect.scala
index 24c0e229..dbc4f7f2 100644
--- a/core/src/main/scala/chisel3/internal/MonoConnect.scala
+++ b/core/src/main/scala/chisel3/internal/MonoConnect.scala
@@ -40,6 +40,10 @@ private[chisel3] object MonoConnect {
     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 UnknownRelationException =
     MonoConnectException(": Sink or source unavailable to current module.")
   // These are when recursing down aggregate types
@@ -58,6 +62,14 @@ private[chisel3] object MonoConnect {
   def AnalogMonoConnectionException =
     MonoConnectException(": Analog cannot participate in a mono connection (source and sink)")
 
+  def checkWhenVisibility(x: Data): Boolean = {
+    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
+    }
+  }
+
   /** This function is what recursively tries to connect a sink and source together
   *
   * There is some cleverness in the use of internal try-catch to catch exceptions
@@ -184,6 +196,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
+    }
+
+    if (!checkWhenVisibility(source)) {
+      throw SourceEscapedWhenScopeException
+    }
+
     // CASE: Context is same module that both left node and right node are in
     if( (context_mod == sink_mod) && (context_mod == source_mod) ) {
       ((sink_direction, source_direction): @unchecked) match {