Fix debugger scope handling during early returns (#1528)

Changes introduced in #1442 to enable debugger tracking of variable
scopes caused a bug if a program used callables with early returns,
namely those returns would not pop any added scopes and later execution
would read/write the wrong variable states. This fixes the issue by
introducing a debug-only return node that triggers a frame leave
handling in the environment. This needs to be debug only because it
should not be used during partial evaluation.

compiler/qsc/src/interpret/tests.rs

@@ -1580,6 +1580,99 @@ mod given_interpreter {
             assert_eq!(2, bps.len());
         }
 
+        #[test]
+        fn debugger_simple_execution_succeeds() {
+            let source = indoc! { r#"
+            namespace Test {
+                function Hello() : Unit {
+                    Message("hello there...");
+                }
+
+                @EntryPoint()
+                operation Main() : Unit {
+                    Hello()
+                }
+            }"#};
+
+            let sources = SourceMap::new([("test".into(), source.into())], None);
+            let mut debugger = Debugger::new(
+                sources,
+                TargetCapabilityFlags::all(),
+                Encoding::Utf8,
+                LanguageFeatures::default(),
+            )
+            .expect("debugger should be created");
+            let (result, output) = entry(&mut debugger.interpreter);
+            is_unit_with_output_eval_entry(&result, &output, "hello there...");
+        }
+
+        #[test]
+        fn debugger_execution_with_call_to_library_succeeds() {
+            let source = indoc! { r#"
+            namespace Test {
+                open Microsoft.Quantum.Math;
+                @EntryPoint()
+                operation Main() : Int {
+                    Binom(31, 7)
+                }
+            }"#};
+
+            let sources = SourceMap::new([("test".into(), source.into())], None);
+            let mut debugger = Debugger::new(
+                sources,
+                TargetCapabilityFlags::all(),
+                Encoding::Utf8,
+                LanguageFeatures::default(),
+            )
+            .expect("debugger should be created");
+            let (result, output) = entry(&mut debugger.interpreter);
+            is_only_value(&result, &output, &Value::Int(2_629_575));
+        }
+
+        #[test]
+        fn debugger_execution_with_early_return_succeeds() {
+            let source = indoc! { r#"
+            namespace Test {
+                open Microsoft.Quantum.Arrays;
+
+                operation Max20(i : Int) : Int {
+                    if (i > 20) {
+                        return 20;
+                    }
+                    return i;
+                }
+
+                @EntryPoint()
+                operation Main() : Int[] {
+                    ForEach(Max20, [10, 20, 30, 40, 50])
+                }
+            }"#};
+
+            let sources = SourceMap::new([("test".into(), source.into())], None);
+            let mut debugger = Debugger::new(
+                sources,
+                TargetCapabilityFlags::all(),
+                Encoding::Utf8,
+                LanguageFeatures::default(),
+            )
+            .expect("debugger should be created");
+            let (result, output) = entry(&mut debugger.interpreter);
+            is_only_value(
+                &result,
+                &output,
+                &Value::Array(
+                    vec![
+                        Value::Int(10),
+                        Value::Int(20),
+                        Value::Int(20),
+                        Value::Int(20),
+                        Value::Int(20),
+                    ]
+                    .into(),
+                ),
+            );
+        }
+
         #[test]
         fn multiple_namespaces_are_loaded_from_sources_into_eval_context() {
             let source = indoc! { r#"

compiler/qsc_eval/src/lib.rs

@@ -359,6 +359,19 @@ impl Env {
         }
     }
 
+    pub fn leave_current_frame(&mut self) {
+        let current_frame_id = self
+            .0
+            .last()
+            .expect("should be at least one scope")
+            .frame_id;
+        if current_frame_id == 0 {
+            // Do not remove the global scope.
+            return;
+        }
+        self.0.retain(|scope| scope.frame_id != current_frame_id);
+    }
+
     pub fn bind_variable_in_top_frame(&mut self, local_var_id: LocalVarId, var: Variable) {
         let Some(scope) = self.0.last_mut() else {
             panic!("no frames in scope");
@@ -611,6 +624,11 @@ impl State {
                     env.leave_scope();
                     continue;
                 }
+                Some(ExecGraphNode::RetFrame) => {
+                    self.leave_frame();
+                    env.leave_current_frame();
+                    continue;
+                }
                 Some(ExecGraphNode::PushScope) => {
                     self.push_scope(env);
                     self.idx += 1;

compiler/qsc_fir/src/fir.rs

@@ -884,8 +884,6 @@ pub enum ExecGraphNode {
     Bind(PatId),
     /// An expression to execute.
     Expr(ExprId),
-    /// A statement to track for debugging.
-    Stmt(StmtId),
     /// An unconditional jump with to given location.
     Jump(u32),
     /// A conditional jump with to given location, where the jump is only taken if the condition is
@@ -900,6 +898,11 @@ pub enum ExecGraphNode {
     Unit,
     /// The end of the control flow graph.
     Ret,
+    /// The end of the control flow graph plus a pop of the current debug frame. Used instead of `Ret`
+    /// when debugging.
+    RetFrame,
+    /// A statement to track for debugging.
+    Stmt(StmtId),
     /// 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.

compiler/qsc_lowerer/src/lib.rs

@@ -32,6 +32,7 @@ pub struct Lowerer {
     assigner: Assigner,
     exec_graph: Vec<ExecGraphNode>,
     enable_debug: bool,
+    ret_node: ExecGraphNode,
 }
 
 impl Default for Lowerer {
@@ -53,19 +54,25 @@ impl Lowerer {
             assigner: Assigner::new(),
             exec_graph: Vec::new(),
             enable_debug: false,
+            ret_node: ExecGraphNode::Ret,
         }
     }
 
     #[must_use]
     pub fn with_debug(mut self, dbg: bool) -> Self {
         self.enable_debug = dbg;
+        if dbg {
+            self.ret_node = ExecGraphNode::RetFrame;
+        } else {
+            self.ret_node = ExecGraphNode::Ret;
+        }
         self
     }
 
     pub fn take_exec_graph(&mut self) -> Vec<ExecGraphNode> {
         self.exec_graph
             .drain(..)
-            .chain(once(ExecGraphNode::Ret))
+            .chain(once(self.ret_node))
             .collect()
     }
 
@@ -248,7 +255,7 @@ impl Lowerer {
             exec_graph: self
                 .exec_graph
                 .drain(..)
-                .chain(once(ExecGraphNode::Ret))
+                .chain(once(self.ret_node))
                 .collect(),
         }
     }
@@ -546,7 +553,7 @@ impl Lowerer {
             }
             hir::ExprKind::Return(expr) => {
                 let expr = self.lower_expr(expr);
-                self.exec_graph.push(ExecGraphNode::Ret);
+                self.exec_graph.push(self.ret_node);
                 fir::ExprKind::Return(expr)
             }
             hir::ExprKind::Tuple(items) => fir::ExprKind::Tuple(