Track nested scopes during debugging (#1442)

This adds new execution graph nodes for pushing and popping variable
scopes that are used only while debugging (like statement nodes) so that
the evaluator can correctly respond to local variable requests during
debugging.

Fixes #1436

Author: swernli

compiler/qsc_eval/src/lib.rs

@@ -563,27 +563,9 @@ impl State {
                     self.idx += 1;
                     self.current_span = globals.get_stmt((self.package, *stmt).into()).span;
 
-                    if let Some(bp) = breakpoints
-                        .iter()
-                        .find(|&bp| *bp == *stmt && self.package == self.source_package)
-                    {
-                        StepResult::BreakpointHit(*bp)
-                    } else {
-                        if self.current_span == Span::default() {
-                            // if there is no span, we are in generated code, so we should skip
-                            continue;
-                        }
-                        // no breakpoint, but we may stop here
-                        if step == StepAction::In {
-                            StepResult::StepIn
-                        } else if step == StepAction::Next && current_frame >= self.call_stack.len()
-                        {
-                            StepResult::Next
-                        } else if step == StepAction::Out && current_frame > self.call_stack.len() {
-                            StepResult::StepOut
-                        } else {
-                            continue;
-                        }
+                    match self.check_for_break(breakpoints, *stmt, step, current_frame) {
+                        Some(value) => value,
+                        None => continue,
                     }
                 }
                 Some(ExecGraphNode::Jump(idx)) => {
@@ -623,6 +605,16 @@ impl State {
                     env.leave_scope();
                     continue;
                 }
+                Some(ExecGraphNode::PushScope) => {
+                    self.push_scope(env);
+                    self.idx += 1;
+                    continue;
+                }
+                Some(ExecGraphNode::PopScope) => {
+                    env.leave_scope();
+                    self.idx += 1;
+                    continue;
+                }
                 None => {
                     // We have reached the end of the current graph without reaching an explicit return node,
                     // usually indicating the partial execution of a single sub-expression.
@@ -644,6 +636,38 @@ impl State {
         Ok(StepResult::Return(self.get_result()))
     }
 
+    fn check_for_break(
+        &self,
+        breakpoints: &[StmtId],
+        stmt: StmtId,
+        step: StepAction,
+        current_frame: usize,
+    ) -> Option<StepResult> {
+        Some(
+            if let Some(bp) = breakpoints
+                .iter()
+                .find(|&bp| *bp == stmt && self.package == self.source_package)
+            {
+                StepResult::BreakpointHit(*bp)
+            } else {
+                if self.current_span == Span::default() {
+                    // if there is no span, we are in generated code, so we should skip
+                    return None;
+                }
+                // no breakpoint, but we may stop here
+                if step == StepAction::In {
+                    StepResult::StepIn
+                } else if step == StepAction::Next && current_frame >= self.call_stack.len() {
+                    StepResult::Next
+                } else if step == StepAction::Out && current_frame > self.call_stack.len() {
+                    StepResult::StepOut
+                } else {
+                    return None;
+                }
+            },
+        )
+    }
+
     pub fn get_result(&mut self) -> Value {
         // Some executions don't have any statements to execute,
         // such as a fragment that has only item definitions.

compiler/qsc_fir/src/fir.rs

@@ -900,6 +900,10 @@ pub enum ExecGraphNode {
     Unit,
     /// The end of the control flow graph.
     Ret,
+    /// A push of a new scope, used when tracking variables for debugging.
+    PushScope,
+    /// A pop of the current scope, used when tracking variables for debugging.
+    PopScope,
 }
 
 /// A sequenced block of statements.

compiler/qsc_lowerer/src/lib.rs

@@ -274,6 +274,18 @@ impl Lowerer {
 
     fn lower_block(&mut self, block: &hir::Block) -> BlockId {
         let id = self.assigner.next_block();
+        // When lowering for debugging, we need to be more strict about scoping for variables
+        // otherwise variables that are not in scope will be visible in the locals view.
+        // We push a scope entry marker, `PushScope`, here and then a `PopScope` marker at the
+        // end of the block, which will cause the evaluation logic to track local variables
+        // for this block in the innermost scope matching their actual accessibility.
+        // When not in debug mode, variables may persist across block boundaries, but all access
+        // is performed via their lowered local variable ID, so they cannot be accessed outside of
+        // their scope. Associated memory is still cleaned up at callable exit rather than block
+        // exit.
+        if self.enable_debug {
+            self.exec_graph.push(ExecGraphNode::PushScope);
+        }
         let set_unit = block.stmts.is_empty()
             || !matches!(
                 block.stmts.last().expect("block should be non-empty").kind,
@@ -288,6 +300,9 @@ impl Lowerer {
         if set_unit {
             self.exec_graph.push(ExecGraphNode::Unit);
         }
+        if self.enable_debug {
+            self.exec_graph.push(ExecGraphNode::PopScope);
+        }
         self.blocks.insert(id, block);
         id
     }