Improve runtime error debugging (#2592)

This change updates the way that runtime errors are reported from debug
execution, allowing the VS Code debugger to break in and treat those as
"exceptions" with detailed information. The tweaks avoid extra checks
during the tight loop for evaluation, and keeps behavior the same for
non-debug execution (like what Python native binaries do).

Author: swernli

source/compiler/qsc/src/interpret.rs

@@ -51,7 +51,7 @@ use qsc_data_structures::{
     target::TargetCapabilityFlags,
 };
 use qsc_eval::{
-    Env, State, VariableInfo,
+    Env, ErrorBehavior, State, VariableInfo,
     backend::{Backend, Chain as BackendChain, SparseSim},
     output::Receiver,
     val,
@@ -1051,7 +1051,12 @@ impl Debugger {
         Ok(Self {
             interpreter,
             position_encoding,
-            state: State::new(source_package_id, entry_exec_graph, None),
+            state: State::new(
+                source_package_id,
+                entry_exec_graph,
+                None,
+                ErrorBehavior::StopOnError,
+            ),
         })
     }
 
@@ -1062,7 +1067,12 @@ impl Debugger {
         Self {
             interpreter,
             position_encoding,
-            state: State::new(source_package_id, entry_exec_graph, None),
+            state: State::new(
+                source_package_id,
+                entry_exec_graph,
+                None,
+                ErrorBehavior::StopOnError,
+            ),
         }
     }
 

source/compiler/qsc_eval/src/lib.rs

@@ -290,7 +290,7 @@ pub fn eval(
     sim: &mut impl Backend<ResultType = impl Into<val::Result>>,
     receiver: &mut impl Receiver,
 ) -> Result<Value, (Error, Vec<Frame>)> {
-    let mut state = State::new(package, exec_graph, seed);
+    let mut state = State::new(package, exec_graph, seed, ErrorBehavior::FailOnError);
     let res = state.eval(globals, env, sim, receiver, &[], StepAction::Continue)?;
     let StepResult::Return(value) = res else {
         panic!("eval should always return a value");
@@ -314,7 +314,7 @@ pub fn invoke(
     callable: Value,
     args: Value,
 ) -> Result<Value, (Error, Vec<Frame>)> {
-    let mut state = State::new(package, Vec::new().into(), seed);
+    let mut state = State::new(package, Vec::new().into(), seed, ErrorBehavior::FailOnError);
     // Push the callable value into the state stack and then the args value so they are ready for evaluation.
     state.set_val_register(callable);
     state.push_val();
@@ -360,7 +360,7 @@ pub enum StepResult {
     StepIn,
     StepOut,
     Return(Value),
-    Fail,
+    Fail(String),
 }
 
 trait AsIndex {
@@ -560,6 +560,14 @@ struct Scope {
 
 type CallableCountKey = (StoreItemId, bool, bool);
 
+#[derive(Debug, Clone, Copy, Eq, PartialEq)]
+pub enum ErrorBehavior {
+    /// Fail execution if an error is encountered.
+    FailOnError,
+    /// Stop execution on the first error encountered.
+    StopOnError,
+}
+
 pub struct State {
     exec_graph_stack: Vec<ExecGraph>,
     idx: u32,
@@ -573,11 +581,18 @@ pub struct State {
     rng: RefCell<StdRng>,
     call_counts: FxHashMap<CallableCountKey, i64>,
     qubit_counter: Option<QubitCounter>,
+    error_behavior: ErrorBehavior,
+    last_error: Option<(Error, Vec<Frame>)>,
 }
 
 impl State {
     #[must_use]
-    pub fn new(package: PackageId, exec_graph: ExecGraph, classical_seed: Option<u64>) -> Self {
+    pub fn new(
+        package: PackageId,
+        exec_graph: ExecGraph,
+        classical_seed: Option<u64>,
+        error_behavior: ErrorBehavior,
+    ) -> Self {
         let rng = match classical_seed {
             Some(seed) => RefCell::new(StdRng::seed_from_u64(seed)),
             None => RefCell::new(StdRng::from_entropy()),
@@ -595,6 +610,8 @@ impl State {
             rng,
             call_counts: FxHashMap::default(),
             qubit_counter: None,
+            error_behavior,
+            last_error: None,
         }
     }
 
@@ -671,6 +688,23 @@ impl State {
         frames
     }
 
+    fn set_last_error(&mut self, error: Error, frames: Vec<Frame>) {
+        assert!(
+            self.last_error.replace((error, frames)).is_none(),
+            "last error should not be set twice"
+        );
+    }
+
+    fn get_last_error(&mut self) -> Result<(), (Error, Vec<Frame>)> {
+        // Use `is_none` to check for last error, as it avoids the unconditional
+        // `mem::replace` call that `take` would perform.
+        if self.last_error.is_none() {
+            Ok(())
+        } else {
+            Err(self.last_error.take().expect("last error should be set"))
+        }
+    }
+
     /// # Errors
     /// Returns the first error encountered during execution.
     /// # Panics
@@ -698,9 +732,20 @@ impl State {
                 }
                 Some(ExecGraphNode::Expr(expr)) => {
                     self.idx += 1;
-                    self.eval_expr(env, sim, globals, out, *expr)
-                        .map_err(|e| (e, self.get_stack_frames()))?;
-                    continue;
+                    match self.eval_expr(env, sim, globals, out, *expr) {
+                        Ok(()) => continue,
+                        Err(e) => {
+                            if self.error_behavior == ErrorBehavior::StopOnError {
+                                let error_str = e.to_string();
+                                self.set_last_error(e, self.get_stack_frames());
+                                // Clear the execution graph stack to indicate that execution has failed.
+                                // This will prevent further execution steps.
+                                self.exec_graph_stack.clear();
+                                return Ok(StepResult::Fail(error_str));
+                            }
+                            return Err((e, self.get_stack_frames()));
+                        }
+                    }
                 }
                 Some(ExecGraphNode::Stmt(stmt)) => {
                     self.idx += 1;
@@ -711,10 +756,6 @@ impl State {
                         None => continue,
                     }
                 }
-                Some(ExecGraphNode::Fail) => {
-                    self.idx += 1;
-                    return Ok(StepResult::Fail);
-                }
                 Some(ExecGraphNode::Jump(idx)) => {
                     self.idx = *idx;
                     continue;
@@ -785,6 +826,11 @@ impl State {
             return Ok(res);
         }
 
+        // If we made it out of the execution loop, we either reached the end of the graph,
+        // a return expression, or hit a runtime error. Check here for the error case
+        // and return it if it exists.
+        self.get_last_error()?;
+
         Ok(StepResult::Return(self.get_result()))
     }
 
@@ -1175,6 +1221,7 @@ impl State {
         out: &mut impl Receiver,
     ) -> Result<(), Error> {
         self.push_frame(Vec::new().into(), callee_id, functor);
+        self.current_span = callee_span.span;
         self.increment_call_count(callee_id, functor);
         let name = &callee.name.name;
         let val = match name.as_ref() {

source/compiler/qsc_eval/src/tests.rs

@@ -2,7 +2,7 @@
 // Licensed under the MIT License.
 
 use crate::{
-    Env, Error, State, StepAction, StepResult, Value,
+    Env, Error, ErrorBehavior, State, StepAction, StepResult, Value,
     backend::{Backend, SparseSim},
     debug::Frame,
     exec_graph_section,
@@ -30,7 +30,7 @@ pub(super) fn eval_graph(
     env: &mut Env,
     out: &mut impl Receiver,
 ) -> Result<Value, (Error, Vec<Frame>)> {
-    let mut state = State::new(package, graph, None);
+    let mut state = State::new(package, graph, None, ErrorBehavior::FailOnError);
     let StepResult::Return(value) =
         state.eval(globals, env, sim, out, &[], StepAction::Continue)?
     else {

source/compiler/qsc_fir/src/fir.rs

@@ -929,8 +929,6 @@ pub enum ExecGraphNode {
     PushScope,
     /// A pop of the current scope, used when tracking variables for debugging.
     PopScope,
-    /// A failure node, inserted just before a `fail` expression to halt execution for debugging.
-    Fail,
 }
 
 /// A sequenced block of statements.

source/compiler/qsc_lowerer/src/lib.rs

@@ -560,11 +560,7 @@ impl Lowerer {
             hir::ExprKind::Fail(message) => {
                 // Ensure the right-hand side expression is lowered first so that it
                 // is executed before the fail node, if any.
-                let fail = fir::ExprKind::Fail(self.lower_expr(message));
-                if self.enable_debug {
-                    self.exec_graph.push(ExecGraphNode::Fail);
-                }
-                fail
+                fir::ExprKind::Fail(self.lower_expr(message))
             }
             hir::ExprKind::Field(container, field) => {
                 let container = self.lower_expr(container);

source/compiler/qsc_partial_eval/src/lib.rs

@@ -17,7 +17,7 @@ use management::{QuantumIntrinsicsChecker, ResourceManager};
 use miette::Diagnostic;
 use qsc_data_structures::{functors::FunctorApp, span::Span, target::TargetCapabilityFlags};
 use qsc_eval::{
-    self, Error as EvalError, PackageSpan, State, StepAction, StepResult, Variable,
+    self, Error as EvalError, ErrorBehavior, PackageSpan, State, StepAction, StepResult, Variable,
     are_ctls_unique, exec_graph_section,
     intrinsic::qubit_relabel,
     output::GenericReceiver,
@@ -1084,7 +1084,12 @@ impl<'a> PartialEvaluator<'a> {
         let scope_exec_graph = self.get_current_scope_exec_graph().clone();
         let scope = self.eval_context.get_current_scope_mut();
         let exec_graph = exec_graph_section(&scope_exec_graph, expr.exec_graph_range.clone());
-        let mut state = State::new(current_package_id, exec_graph, None);
+        let mut state = State::new(
+            current_package_id,
+            exec_graph,
+            None,
+            ErrorBehavior::FailOnError,
+        );
         let classical_result = state.eval(
             self.package_store,
             &mut scope.env,

source/compiler/qsc_passes/src/replace_qubit_allocation.rs

@@ -515,7 +515,7 @@ fn create_general_dealloc_stmt(
         span: Span::default(),
         kind: StmtKind::Semi(Expr {
             id: assigner.next_node(),
-            span: Span::default(),
+            span: ident.span,
             ty: Ty::UNIT,
             kind: ExprKind::Call(Box::new(call_expr), Box::new(ident.gen_local_ref(assigner))),
         }),