Fix IR for try (#13811)

# Description
Fixes a bug in the IR for `try` to match that of the regular evaluator
(continuing from #13515):
```nushell
# without IR:
try { ^false } catch { 'caught' } # == 'caught'

# with IR:
try { ^false } catch { 'caught' } # error, non-zero exit code
```

In this PR, both now evaluate to `caught`. For the implementation, I had
to add another instruction, and feel free to suggest better
alternatives. In the future, it might be possible to get rid of this
extra instruction.

# User-Facing Changes
Bug fix, `try { ^false } catch { 'caught' }` now works in IR.
This commit is contained in:
Ian Manske 2024-09-09 19:44:04 -07:00 committed by GitHub
parent 6600b3edfb
commit 493850b1bf
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 36 additions and 5 deletions

View file

@ -96,12 +96,12 @@ fn can_catch_infinite_recursion() {
#[test]
fn exit_code_available_in_catch_env() {
let actual = nu!("try { nu -c 'exit 42'; null } catch { $env.LAST_EXIT_CODE }");
let actual = nu!("try { nu -c 'exit 42' } catch { $env.LAST_EXIT_CODE }");
assert_eq!(actual.out, "42");
}
#[test]
fn exit_code_available_in_catch() {
let actual = nu!("try { nu -c 'exit 42'; null } catch { |e| $e.exit_code }");
let actual = nu!("try { nu -c 'exit 42' } catch { |e| $e.exit_code }");
assert_eq!(actual.out, "42");
}

View file

@ -202,6 +202,7 @@ impl BlockBuilder {
Instruction::Span { src_dst } => allocate(&[*src_dst], &[*src_dst]),
Instruction::Drop { src } => allocate(&[*src], &[]),
Instruction::Drain { src } => allocate(&[*src], &[]),
Instruction::WriteToOutDests { src } => allocate(&[*src], &[]),
Instruction::LoadVariable { dst, var_id: _ } => allocate(&[], &[*dst]),
Instruction::StoreVariable { var_id: _, src } => allocate(&[*src], &[]),
Instruction::DropVariable { var_id: _ } => Ok(()),

View file

@ -362,6 +362,7 @@ pub(crate) fn compile_try(
//
// on-error-into ERR, %io_reg // or without
// %io_reg <- <...block...> <- %io_reg
// write-to-out-dests %io_reg
// pop-error-handler
// jump END
// ERR: clone %err_reg, %io_reg
@ -374,6 +375,7 @@ pub(crate) fn compile_try(
// %closure_reg <- <catch_expr>
// on-error-into ERR, %io_reg
// %io_reg <- <...block...> <- %io_reg
// write-to-out-dests %io_reg
// pop-error-handler
// jump END
// ERR: clone %err_reg, %io_reg
@ -461,7 +463,17 @@ pub(crate) fn compile_try(
io_reg,
)?;
// Successful case: pop the error handler
// Successful case:
// - write to the current output destinations
// - pop the error handler
if let Some(mode) = redirect_modes.out {
builder.push(mode.map(|mode| Instruction::RedirectOut { mode }))?;
}
if let Some(mode) = redirect_modes.err {
builder.push(mode.map(|mode| Instruction::RedirectErr { mode }))?;
}
builder.push(Instruction::WriteToOutDests { src: io_reg }.into_spanned(call.head))?;
builder.push(Instruction::PopErrorHandler.into_spanned(call.head))?;
// Jump over the failure case

View file

@ -317,6 +317,17 @@ fn eval_instruction<D: DebugContext>(
let data = ctx.take_reg(*src);
drain(ctx, data)
}
Instruction::WriteToOutDests { src } => {
let data = ctx.take_reg(*src);
let res = {
let stack = &mut ctx
.stack
.push_redirection(ctx.redirect_out.clone(), ctx.redirect_err.clone());
data.write_to_out_dests(ctx.engine_state, stack)?
};
ctx.put_reg(*src, res);
Ok(Continue)
}
Instruction::LoadVariable { dst, var_id } => {
let value = get_var(ctx, *var_id, *span)?;
ctx.put_reg(*dst, value.into_pipeline_data());

View file

@ -94,6 +94,9 @@ impl<'a> fmt::Display for FmtInstruction<'a> {
Instruction::Drain { src } => {
write!(f, "{:WIDTH$} {src}", "drain")
}
Instruction::WriteToOutDests { src } => {
write!(f, "{:WIDTH$} {src}", "write-to-out-dests")
}
Instruction::LoadVariable { dst, var_id } => {
let var = FmtVar::new(self.engine_state, *var_id);
write!(f, "{:WIDTH$} {dst}, {var}", "load-variable")

View file

@ -123,6 +123,9 @@ pub enum Instruction {
/// code, and invokes any available error handler with Empty, or if not available, returns an
/// exit-code-only stream, leaving the block.
Drain { src: RegId },
/// Write to the output destinations in the stack
// TODO: see if it's possible to remove this
WriteToOutDests { src: RegId },
/// Load the value of a variable into the `dst` register
LoadVariable { dst: RegId, var_id: VarId },
/// Store the value of a variable from the `src` register
@ -287,6 +290,7 @@ impl Instruction {
Instruction::Span { src_dst } => Some(src_dst),
Instruction::Drop { .. } => None,
Instruction::Drain { .. } => None,
Instruction::WriteToOutDests { .. } => None,
Instruction::LoadVariable { dst, .. } => Some(dst),
Instruction::StoreVariable { .. } => None,
Instruction::DropVariable { .. } => None,

View file

@ -65,7 +65,7 @@ fn fancy_default_errors() {
r#"force_error "My error""#,
]);
let actual = nu!(format!("try {{ {code}; null }}"));
let actual = nu!(format!("try {{ {code} }}"));
assert_eq!(
actual.err,
@ -89,7 +89,7 @@ fn narratable_errors() {
r#"force_error "my error""#,
]);
let actual = nu!(format!("try {{ {code}; null }}"));
let actual = nu!(format!("try {{ {code} }}"));
assert_eq!(
actual.err,