From 7a055563a93b57434f24946811b954b626152508 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 2 Jul 2024 20:11:11 -0700 Subject: [PATCH] try/catch (wip) --- crates/nu-engine/src/compile/builder.rs | 12 ++- crates/nu-engine/src/compile/call.rs | 3 + crates/nu-engine/src/compile/keyword.rs | 94 +++++++++++++++++++ crates/nu-engine/src/eval_ir.rs | 60 +++++++++--- .../nu-protocol/src/engine/error_handler.rs | 75 +++++++++++++++ crates/nu-protocol/src/engine/mod.rs | 2 + crates/nu-protocol/src/engine/stack.rs | 22 +++-- crates/nu-protocol/src/ir/call.rs | 8 +- crates/nu-protocol/src/ir/display.rs | 16 +++- crates/nu-protocol/src/ir/mod.rs | 7 ++ tests/eval/mod.rs | 21 +++++ 11 files changed, 291 insertions(+), 29 deletions(-) create mode 100644 crates/nu-protocol/src/engine/error_handler.rs diff --git a/crates/nu-engine/src/compile/builder.rs b/crates/nu-engine/src/compile/builder.rs index 798da78239..fd1e977264 100644 --- a/crates/nu-engine/src/compile/builder.rs +++ b/crates/nu-engine/src/compile/builder.rs @@ -197,6 +197,12 @@ impl BlockBuilder { stream: _, end_index: _, } => self.mark_register(*dst)?, + Instruction::PushErrorHandler { index: _ } => (), + Instruction::PushErrorHandlerVar { + index: _, + error_var: _, + } => (), + Instruction::PopErrorHandler => (), Instruction::Return { src } => self.free_register(*src)?, } let index = self.next_instruction_index(); @@ -299,7 +305,7 @@ impl BlockBuilder { self.jump(usize::MAX, span) } - /// Modify a `branch-if`, `jump`, or `iterate` instruction's branch target `index` + /// Modify a branching instruction's branch target `index` pub(crate) fn set_branch_target( &mut self, instruction_index: usize, @@ -311,7 +317,9 @@ impl BlockBuilder { | Instruction::Jump { index } | Instruction::Iterate { end_index: index, .. - }, + } + | Instruction::PushErrorHandler { index } + | Instruction::PushErrorHandlerVar { index, .. }, ) => { *index = target_index; Ok(()) diff --git a/crates/nu-engine/src/compile/call.rs b/crates/nu-engine/src/compile/call.rs index a1588d3e00..992969d544 100644 --- a/crates/nu-engine/src/compile/call.rs +++ b/crates/nu-engine/src/compile/call.rs @@ -31,6 +31,9 @@ pub(crate) fn compile_call( "let" | "mut" => { return compile_let(working_set, builder, call, redirect_modes, io_reg); } + "try" => { + return compile_try(working_set, builder, call, redirect_modes, io_reg); + } "loop" => { return compile_loop(working_set, builder, call, redirect_modes, io_reg); } diff --git a/crates/nu-engine/src/compile/keyword.rs b/crates/nu-engine/src/compile/keyword.rs index a9ec1aecde..0099b19d2e 100644 --- a/crates/nu-engine/src/compile/keyword.rs +++ b/crates/nu-engine/src/compile/keyword.rs @@ -178,6 +178,100 @@ pub(crate) fn compile_let( Ok(()) } +/// Compile a call to `try`, setting an error handler over the evaluated block +pub(crate) fn compile_try( + working_set: &StateWorkingSet, + builder: &mut BlockBuilder, + call: &Call, + redirect_modes: RedirectModes, + io_reg: RegId, +) -> Result<(), CompileError> { + // Pseudocode: + // + // push-error-handler-var ERR, $err // or without var + // %io_reg <- <...block...> <- %io_reg + // pop-error-handler + // jump END + // ERR: %io_reg <- <...catch block...> // set to empty if none + // END: + let invalid = || CompileError::InvalidKeywordCall { + keyword: "try".into(), + span: call.head, + }; + + let block_arg = call.positional_nth(0).ok_or_else(invalid)?; + let block_id = block_arg.as_block().ok_or_else(invalid)?; + let block = working_set.get_block(block_id); + + let catch_block = match call.positional_nth(1) { + Some(kw_expr) => { + let catch_expr = kw_expr.as_keyword().ok_or_else(invalid)?; + let catch_block_id = catch_expr.as_block().ok_or_else(invalid)?; + Some(working_set.get_block(catch_block_id)) + } + None => None, + }; + let catch_var_id = catch_block + .and_then(|b| b.signature.get_positional(0)) + .and_then(|v| v.var_id); + + // Put the error handler placeholder + let error_handler_index = if let Some(catch_var_id) = catch_var_id { + builder.push( + Instruction::PushErrorHandlerVar { + index: usize::MAX, + error_var: catch_var_id, + } + .into_spanned(call.head), + )? + } else { + builder.push(Instruction::PushErrorHandler { index: usize::MAX }.into_spanned(call.head))? + }; + + // Compile the block + compile_block( + working_set, + builder, + block, + redirect_modes.clone(), + Some(io_reg), + io_reg, + )?; + + // Successful case: pop the error handler + builder.push(Instruction::PopErrorHandler.into_spanned(call.head))?; + + // Jump over the failure case + let jump_index = + builder.jump_placeholder(catch_block.and_then(|b| b.span).unwrap_or(call.head))?; + + // This is the error handler - go back and set the right branch destination + builder.set_branch_target(error_handler_index, builder.next_instruction_index())?; + + // Mark out register as likely not clean - state in error handler is not well defined + builder.mark_register(io_reg)?; + + // If we have a catch block, compile that + if let Some(catch_block) = catch_block { + compile_block( + working_set, + builder, + catch_block, + redirect_modes, + None, + io_reg, + )?; + } else { + // Otherwise just set out to empty. + builder.load_empty(io_reg)?; + } + + // This is the end - if we succeeded, should jump here + builder.set_branch_target(jump_index, builder.next_instruction_index())?; + + Ok(()) +} + /// Compile a call to `loop` (via `jump`) pub(crate) fn compile_loop( working_set: &StateWorkingSet, diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index ffd145616a..d985d320bd 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -4,7 +4,7 @@ use nu_path::expand_path_with; use nu_protocol::{ ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator}, debugger::DebugContext, - engine::{Argument, Closure, EngineState, Redirection, Stack}, + engine::{Argument, Closure, EngineState, ErrorHandler, Redirection, Stack}, ir::{Call, DataSlice, Instruction, IrBlock, Literal, RedirectMode}, DeclId, IntoPipelineData, IntoSpanned, ListStream, OutDest, PipelineData, Range, Record, RegId, ShellError, Span, Value, VarId, @@ -24,7 +24,8 @@ pub fn eval_ir_block( let block_span = block.span; - let args_base = stack.argument_stack.get_base(); + let args_base = stack.arguments.get_base(); + let error_handler_base = stack.error_handlers.get_base(); let mut registers = stack.register_buf_cache.acquire(ir_block.register_count); let result = eval_ir_block_impl::( @@ -33,6 +34,7 @@ pub fn eval_ir_block( stack, data: &ir_block.data, args_base, + error_handler_base, redirect_out: None, redirect_err: None, registers: &mut registers[..], @@ -43,6 +45,8 @@ pub fn eval_ir_block( ); stack.register_buf_cache.release(registers); + stack.error_handlers.leave_frame(error_handler_base); + stack.arguments.leave_frame(args_base); D::leave_block(engine_state, block); @@ -66,6 +70,8 @@ struct EvalContext<'a> { data: &'a Arc<[u8]>, /// Base index on the argument stack to reset to after a call args_base: usize, + /// Base index on the error handler stack to reset to after a call + error_handler_base: usize, /// State set by redirect-out redirect_out: Option, /// State set by redirect-err @@ -122,16 +128,26 @@ fn eval_ir_block_impl( "{pc:-4}: {}", instruction.display(ctx.engine_state, ctx.data) ); - match eval_instruction(ctx, instruction, span)? { - InstructionResult::Continue => { + match eval_instruction(ctx, instruction, span) { + Ok(InstructionResult::Continue) => { pc += 1; } - InstructionResult::Branch(next_pc) => { + Ok(InstructionResult::Branch(next_pc)) => { pc = next_pc; } - InstructionResult::Return(reg_id) => { + Ok(InstructionResult::Return(reg_id)) => { return Ok(ctx.take_reg(reg_id)); } + Err(err) => { + if let Some(error_handler) = ctx.stack.error_handlers.pop(ctx.error_handler_base) { + // If an error handler is set, branch there + error_handler.prepare_stack(ctx.engine_state, &mut ctx.stack, err); + pc = error_handler.handler_index; + } else { + // If not, exit the block with the error + return Err(err); + } + } } } @@ -254,19 +270,19 @@ fn eval_instruction( Instruction::PushPositional { src } => { let val = ctx.collect_reg(*src, *span)?; ctx.stack - .argument_stack + .arguments .push(Argument::Positional { span: *span, val }); Ok(Continue) } Instruction::AppendRest { src } => { let vals = ctx.collect_reg(*src, *span)?; ctx.stack - .argument_stack + .arguments .push(Argument::Spread { span: *span, vals }); Ok(Continue) } Instruction::PushFlag { name } => { - ctx.stack.argument_stack.push(Argument::Flag { + ctx.stack.arguments.push(Argument::Flag { data: ctx.data.clone(), name: *name, span: *span, @@ -275,7 +291,7 @@ fn eval_instruction( } Instruction::PushNamed { name, src } => { let val = ctx.collect_reg(*src, *span)?; - ctx.stack.argument_stack.push(Argument::Named { + ctx.stack.arguments.push(Argument::Named { data: ctx.data.clone(), name: *name, span: *span, @@ -469,6 +485,24 @@ fn eval_instruction( stream, end_index, } => eval_iterate(ctx, *dst, *stream, *end_index), + Instruction::PushErrorHandler { index } => { + ctx.stack.error_handlers.push(ErrorHandler { + handler_index: *index, + error_variable: None, + }); + Ok(Continue) + } + Instruction::PushErrorHandlerVar { index, error_var } => { + ctx.stack.error_handlers.push(ErrorHandler { + handler_index: *index, + error_variable: Some(*error_var), + }); + Ok(Continue) + } + Instruction::PopErrorHandler => { + ctx.stack.error_handlers.pop(ctx.error_handler_base); + Ok(Continue) + } Instruction::Return { src } => Ok(Return(*src)), } } @@ -651,7 +685,7 @@ fn eval_call( } = ctx; // TODO: handle block eval - let args_len = stack.argument_stack.get_len(*args_base); + let args_len = stack.arguments.get_len(*args_base); let decl = engine_state.get_decl(decl_id); // Set up redirect modes @@ -659,7 +693,7 @@ fn eval_call( // should this be precalculated? ideally we just use the call builder... let span = stack - .argument_stack + .arguments .get_args(*args_base, args_len) .into_iter() .fold(head, |span, arg| { @@ -678,7 +712,7 @@ fn eval_call( // Run the call let result = decl.run(engine_state, &mut stack, &(&call).into(), input); // Important that this runs: - stack.argument_stack.leave_frame(ctx.args_base); + stack.arguments.leave_frame(ctx.args_base); result } diff --git a/crates/nu-protocol/src/engine/error_handler.rs b/crates/nu-protocol/src/engine/error_handler.rs new file mode 100644 index 0000000000..a30186dc49 --- /dev/null +++ b/crates/nu-protocol/src/engine/error_handler.rs @@ -0,0 +1,75 @@ +use crate::{record, ShellError, Value, VarId}; + +use super::{EngineState, Stack}; + +/// Describes an error handler stored during IR evaluation. +#[derive(Debug, Clone, Copy)] +pub struct ErrorHandler { + /// Instruction index within the block that will handle the error + pub handler_index: usize, + /// Variable to put the error information into, when an error occurs + pub error_variable: Option, +} + +impl ErrorHandler { + /// Add `error_variable` to the stack with the error value. + pub fn prepare_stack(&self, engine_state: &EngineState, stack: &mut Stack, error: ShellError) { + if let Some(var_id) = self.error_variable { + let span = engine_state.get_var(var_id).declaration_span; + let value = Value::record( + record! { + "msg" => Value::string(format!("{}", error), span), + "debug" => Value::string(format!("{:?}", error), span), + "raw" => Value::error(error, span), + }, + span, + ); + stack.add_var(var_id, value); + } + } +} + +/// Keeps track of error handlers pushed during evaluation of an IR block. +#[derive(Debug, Clone)] +pub struct ErrorHandlerStack { + handlers: Vec, +} + +impl ErrorHandlerStack { + pub const fn new() -> ErrorHandlerStack { + ErrorHandlerStack { handlers: vec![] } + } + + /// Get the current base of the stack, which establishes a frame. + pub fn get_base(&self) -> usize { + self.handlers.len() + } + + /// Push a new error handler onto the stack. + pub fn push(&mut self, handler: ErrorHandler) { + self.handlers.push(handler); + } + + /// Try to pop an error handler from the stack. Won't go below `base`, to avoid retrieving a + /// handler belonging to a parent frame. + pub fn pop(&mut self, base: usize) -> Option { + if self.handlers.len() > base { + self.handlers.pop() + } else { + None + } + } + + /// Reset the stack to the state it was in at the beginning of the frame, in preparation to + /// return control to the parent frame. + pub fn leave_frame(&mut self, base: usize) { + if self.handlers.len() >= base { + self.handlers.truncate(base); + } else { + panic!( + "ErrorHandlerStack bug: tried to leave frame at {base}, but current base is {}", + self.get_base() + ) + } + } +} diff --git a/crates/nu-protocol/src/engine/mod.rs b/crates/nu-protocol/src/engine/mod.rs index 201fd53394..81a776a2bf 100644 --- a/crates/nu-protocol/src/engine/mod.rs +++ b/crates/nu-protocol/src/engine/mod.rs @@ -5,6 +5,7 @@ mod call_info; mod capture_block; mod command; mod engine_state; +mod error_handler; mod overlay; mod pattern_match; mod register_buf_cache; @@ -23,6 +24,7 @@ pub use call_info::*; pub use capture_block::*; pub use command::*; pub use engine_state::*; +pub use error_handler::*; pub use overlay::*; pub use pattern_match::*; pub use register_buf_cache::*; diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 5ea028e217..060c05fd99 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -1,7 +1,7 @@ use crate::{ engine::{ - EngineState, Redirection, StackCallArgGuard, StackCaptureGuard, StackIoGuard, StackOutDest, - DEFAULT_OVERLAY_NAME, + ArgumentStack, EngineState, ErrorHandlerStack, Redirection, RegisterBufCache, + StackCallArgGuard, StackCaptureGuard, StackIoGuard, StackOutDest, DEFAULT_OVERLAY_NAME, }, OutDest, ShellError, Span, Value, VarId, ENV_VARIABLE_ID, NU_VARIABLE_ID, }; @@ -11,8 +11,6 @@ use std::{ sync::Arc, }; -use super::{ArgumentStack, RegisterBufCache}; - /// Environment variables per overlay pub type EnvVars = HashMap>; @@ -46,7 +44,9 @@ pub struct Stack { /// Cached register buffers for IR evaluation pub register_buf_cache: RegisterBufCache, /// Argument stack for IR evaluation - pub argument_stack: ArgumentStack, + pub arguments: ArgumentStack, + /// Error handler stack for IR evaluation + pub error_handlers: ErrorHandlerStack, /// Set true to always use IR mode pub use_ir: bool, pub recursion_count: u64, @@ -77,7 +77,8 @@ impl Stack { env_hidden: HashMap::new(), active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], register_buf_cache: RegisterBufCache::new(), - argument_stack: ArgumentStack::new(), + arguments: ArgumentStack::new(), + error_handlers: ErrorHandlerStack::new(), use_ir: false, recursion_count: 0, parent_stack: None, @@ -97,7 +98,8 @@ impl Stack { env_hidden: parent.env_hidden.clone(), active_overlays: parent.active_overlays.clone(), register_buf_cache: RegisterBufCache::new(), - argument_stack: ArgumentStack::new(), + arguments: ArgumentStack::new(), + error_handlers: ErrorHandlerStack::new(), use_ir: parent.use_ir, recursion_count: parent.recursion_count, vars: vec![], @@ -269,7 +271,8 @@ impl Stack { env_hidden: self.env_hidden.clone(), active_overlays: self.active_overlays.clone(), register_buf_cache: RegisterBufCache::new(), - argument_stack: ArgumentStack::new(), + arguments: ArgumentStack::new(), + error_handlers: ErrorHandlerStack::new(), use_ir: self.use_ir, recursion_count: self.recursion_count, parent_stack: None, @@ -302,7 +305,8 @@ impl Stack { env_hidden: self.env_hidden.clone(), active_overlays: self.active_overlays.clone(), register_buf_cache: RegisterBufCache::new(), - argument_stack: ArgumentStack::new(), + arguments: ArgumentStack::new(), + error_handlers: ErrorHandlerStack::new(), use_ir: self.use_ir, recursion_count: self.recursion_count, parent_stack: None, diff --git a/crates/nu-protocol/src/ir/call.rs b/crates/nu-protocol/src/ir/call.rs index 90cc7009bb..75794f9000 100644 --- a/crates/nu-protocol/src/ir/call.rs +++ b/crates/nu-protocol/src/ir/call.rs @@ -42,7 +42,7 @@ impl Call { /// Get the arguments for this call from the arguments stack. pub fn arguments<'a>(&self, stack: &'a Stack) -> &'a [Argument] { - stack.argument_stack.get_args(self.args_base, self.args_len) + stack.arguments.get_args(self.args_base, self.args_len) } /// The span encompassing the arguments @@ -205,7 +205,7 @@ impl Call { /// Resets the [`Stack`] to its state before the call was made. pub fn leave(&self, stack: &mut Stack) { - stack.argument_stack.leave_frame(self.args_base); + stack.arguments.leave_frame(self.args_base); } } @@ -218,13 +218,13 @@ impl CallBuilder { /// Add an argument to the [`Stack`] and reference it from the [`Call`]. pub fn add_argument(&mut self, stack: &mut Stack, argument: Argument) -> &mut Self { if self.inner.args_len == 0 { - self.inner.args_base = stack.argument_stack.get_base(); + self.inner.args_base = stack.arguments.get_base(); } self.inner.args_len += 1; if let Some(span) = argument.span() { self.inner.span = self.inner.span.append(span); } - stack.argument_stack.push(argument); + stack.arguments.push(argument); self } diff --git a/crates/nu-protocol/src/ir/display.rs b/crates/nu-protocol/src/ir/display.rs index d52f2aabb9..ceed96f593 100644 --- a/crates/nu-protocol/src/ir/display.rs +++ b/crates/nu-protocol/src/ir/display.rs @@ -46,7 +46,7 @@ pub struct FmtInstruction<'a> { impl<'a> fmt::Display for FmtInstruction<'a> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - const WIDTH: usize = 20; + const WIDTH: usize = 22; match self.instruction { Instruction::LoadLiteral { dst, lit } => { @@ -170,6 +170,20 @@ impl<'a> fmt::Display for FmtInstruction<'a> { } => { write!(f, "{:WIDTH$} {dst}, {stream}, end {end_index}", "iterate") } + Instruction::PushErrorHandler { index } => { + write!(f, "{:WIDTH$} {index}", "push-error-handler") + } + Instruction::PushErrorHandlerVar { index, error_var } => { + let error_var = FmtVar::new(self.engine_state, *error_var); + write!( + f, + "{:WIDTH$} {index}, {error_var}", + "push-error-handler-var" + ) + } + Instruction::PopErrorHandler => { + write!(f, "{:WIDTH$}", "pop-error-handler") + } Instruction::Return { src } => { write!(f, "{:WIDTH$} {src}", "return") } diff --git a/crates/nu-protocol/src/ir/mod.rs b/crates/nu-protocol/src/ir/mod.rs index 049444437d..a8e420065f 100644 --- a/crates/nu-protocol/src/ir/mod.rs +++ b/crates/nu-protocol/src/ir/mod.rs @@ -149,6 +149,13 @@ pub enum Instruction { stream: RegId, end_index: usize, }, + /// Push an error handler, without capturing the error value + PushErrorHandler { index: usize }, + /// Push an error handler, capturing the error value into the `error_var` + PushErrorHandlerVar { index: usize, error_var: VarId }, + /// Pop an error handler. This is not necessary when control flow is directed to the error + /// handler due to an error. + PopErrorHandler, /// Return from the block with the value in the register Return { src: RegId }, } diff --git a/tests/eval/mod.rs b/tests/eval/mod.rs index 21d4bec390..e6f319b6b4 100644 --- a/tests/eval/mod.rs +++ b/tests/eval/mod.rs @@ -338,3 +338,24 @@ fn early_return_from_while() { fn early_return_from_for() { test_eval("do { for x in [pass fail] { return $x } }", Eq("pass")) } + +#[test] +fn try_no_catch() { + test_eval("try { error make { msg: foo } }; 'pass'", Eq("pass")) +} + +#[test] +fn try_catch_no_var() { + test_eval( + "try { error make { msg: foo } } catch { 'pass' }", + Eq("pass"), + ) +} + +#[test] +fn try_catch_var() { + test_eval( + "try { error make { msg: foo } } catch { |err| $err.msg }", + Eq("foo"), + ) +}