From 36faabd36d805eadc75cd65a7bbdec1a9b295065 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 9 Jul 2024 05:09:25 -0700 Subject: [PATCH] change the return keyword to return-early, use ShellError::Return :( --- crates/nu-engine/src/compile/builder.rs | 1 + crates/nu-engine/src/compile/keyword.rs | 8 +++++--- crates/nu-engine/src/eval_ir.rs | 15 +++++++++++++++ crates/nu-parser/src/known_external.rs | 4 ++-- crates/nu-protocol/src/ir/display.rs | 3 +++ crates/nu-protocol/src/ir/mod.rs | 4 ++++ 6 files changed, 30 insertions(+), 5 deletions(-) diff --git a/crates/nu-engine/src/compile/builder.rs b/crates/nu-engine/src/compile/builder.rs index ed1b1d03c1..c3d0b758c1 100644 --- a/crates/nu-engine/src/compile/builder.rs +++ b/crates/nu-engine/src/compile/builder.rs @@ -241,6 +241,7 @@ impl BlockBuilder { Instruction::OnErrorInto { index: _, dst } => allocate(&[], &[*dst]), Instruction::PopErrorHandler => Ok(()), Instruction::CheckExternalFailed { dst, src } => allocate(&[*src], &[*dst, *src]), + Instruction::ReturnEarly { src } => allocate(&[*src], &[]), Instruction::Return { src } => allocate(&[*src], &[]), }; diff --git a/crates/nu-engine/src/compile/keyword.rs b/crates/nu-engine/src/compile/keyword.rs index dc00d2f9ca..6cd4d3e7f8 100644 --- a/crates/nu-engine/src/compile/keyword.rs +++ b/crates/nu-engine/src/compile/keyword.rs @@ -844,7 +844,7 @@ pub(crate) fn compile_continue( Ok(()) } -/// Compile a call to `return` as a `return` instruction. +/// Compile a call to `return` as a `return-early` instruction. /// /// This is not strictly necessary, but it is more efficient. pub(crate) fn compile_return( @@ -857,7 +857,7 @@ pub(crate) fn compile_return( // Pseudocode: // // %io_reg <- - // return %io_reg + // return-early %io_reg if let Some(arg_expr) = call.positional_nth(0) { compile_expression( working_set, @@ -871,7 +871,9 @@ pub(crate) fn compile_return( builder.load_empty(io_reg)?; } - builder.push(Instruction::Return { src: io_reg }.into_spanned(call.head))?; + // TODO: It would be nice if this could be `return` instead, but there is a little bit of + // behaviour remaining that still depends on `ShellError::Return` + builder.push(Instruction::ReturnEarly { src: io_reg }.into_spanned(call.head))?; // io_reg is supposed to remain allocated builder.load_empty(io_reg)?; diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index d62347afd3..649b9500ee 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -210,6 +210,14 @@ fn eval_ir_block_impl( )); } } + Err( + err @ (ShellError::Return { .. } + | ShellError::Continue { .. } + | ShellError::Break { .. }), + ) => { + // These block control related errors should be passed through + return Err(err); + } Err(err) => { if let Some(error_handler) = ctx.stack.error_handlers.pop(ctx.error_handler_base) { // If an error handler is set, branch there @@ -739,6 +747,13 @@ fn eval_instruction( ctx.put_reg(*dst, Value::bool(failed, *span).into_pipeline_data()); Ok(Continue) } + Instruction::ReturnEarly { src } => { + let val = ctx.collect_reg(*src, *span)?; + Err(ShellError::Return { + span: *span, + value: Box::new(val), + }) + } Instruction::Return { src } => Ok(Return(*src)), } } diff --git a/crates/nu-parser/src/known_external.rs b/crates/nu-parser/src/known_external.rs index e1f09cf3d2..a41cf3a4e8 100644 --- a/crates/nu-parser/src/known_external.rs +++ b/crates/nu-parser/src/known_external.rs @@ -167,7 +167,7 @@ fn ir_call_to_extern_call( } => { let name_arg = engine::Argument::Positional { span: *span, - val: Value::string(known_external_option_name(&data, *name, *short), *span), + val: Value::string(known_external_option_name(data, *name, *short), *span), ast: None, }; extern_call.add_argument(stack, name_arg); @@ -182,7 +182,7 @@ fn ir_call_to_extern_call( } => { let name_arg = engine::Argument::Positional { span: *span, - val: Value::string(known_external_option_name(&data, *name, *short), *span), + val: Value::string(known_external_option_name(data, *name, *short), *span), ast: None, }; let val_arg = engine::Argument::Positional { diff --git a/crates/nu-protocol/src/ir/display.rs b/crates/nu-protocol/src/ir/display.rs index 8171cf18fb..213d2c836b 100644 --- a/crates/nu-protocol/src/ir/display.rs +++ b/crates/nu-protocol/src/ir/display.rs @@ -240,6 +240,9 @@ impl<'a> fmt::Display for FmtInstruction<'a> { Instruction::CheckExternalFailed { dst, src } => { write!(f, "{:WIDTH$} {dst}, {src}", "check-external-failed") } + Instruction::ReturnEarly { src } => { + write!(f, "{:WIDTH$} {src}", "return-early") + } 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 b27caac0ed..f0d3ecd406 100644 --- a/crates/nu-protocol/src/ir/mod.rs +++ b/crates/nu-protocol/src/ir/mod.rs @@ -248,6 +248,10 @@ pub enum Instruction { /// Check if an external command failed. Boolean value into `dst`. `src` is preserved, but it /// does require waiting for the command to exit. CheckExternalFailed { dst: RegId, src: RegId }, + /// Return early from the block, raising a `ShellError::Return` instead. + /// + /// Collecting the value is unavoidable. + ReturnEarly { src: RegId }, /// Return from the block with the value in the register Return { src: RegId }, }