IR: allow subexpression with redirection. (#15617)

# Description
Try to fixes https://github.com/nushell/nushell/issues/15326 in another
way.

The main point of this change is to avoid duplicate `write` and `close`
a redirected file. So during compile, if compiler know current element
is a sub-expression(defined by private `is_subexpression` function), it
will no longer invoke `finish_redirection`.

In this way, we can avoid duplicate `finish_redirection`.

# User-Facing Changes
`(^echo aa) o> /tmp/aaa` will no longer raise an error.

Here is the IR after the pr:
```
# 3 registers, 12 instructions, 11 bytes of data
# 1 file used for redirection
   0: load-literal           %1, string("aaa")
   1: open-file              file(0), %1, append = false
   2: load-literal           %1, glob-pattern("echo", no_expand = false)
   3: load-literal           %2, glob-pattern("true", no_expand = false)
   4: push-positional        %1
   5: push-positional        %2
   6: redirect-out           file(0)
   7: redirect-err           caller
   8: call                   decl 135 "run-external", %0
   9: write-file             file(0), %0
  10: close-file             file(0)
  11: return                 %0
```

# Tests + Formatting
Added 3 tests.

# After Submitting
Maybe need to update doc
https://github.com/nushell/nushell.github.io/pull/1876

---------

Co-authored-by: Stefan Holderbach <sholderbach@users.noreply.github.com>
This commit is contained in:
Wind 2025-04-24 19:47:04 +08:00 committed by GitHub
parent 78903724f5
commit 7add38fe32
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 32 additions and 3 deletions

View File

@ -473,3 +473,18 @@ fn pipe_redirection_in_let_and_mut(
);
assert_eq!(actual.out, output);
}
#[rstest::rstest]
#[case("o>", "bar")]
#[case("e>", "")]
#[case("o+e>", "bar\nbaz")] // in subexpression, the stderr is go to the terminal
fn subexpression_redirection(#[case] redir: &str, #[case] stdout_file_body: &str) {
Playground::setup("file redirection with subexpression", |dirs, _| {
let actual = nu!(
cwd: dirs.test(),
format!("$env.BAR = 'bar'; $env.BAZ = 'baz'; (nu --testbin echo_env_mixed out-err BAR BAZ) {redir} result.txt")
);
assert!(actual.status.success());
assert!(file_contents(dirs.test().join("result.txt")).contains(stdout_file_body));
})
}

View File

@ -1,5 +1,5 @@
use nu_protocol::{
ast::{Block, Pipeline, PipelineRedirection, RedirectionSource, RedirectionTarget},
ast::{Block, Expr, Pipeline, PipelineRedirection, RedirectionSource, RedirectionTarget},
engine::StateWorkingSet,
ir::{Instruction, IrBlock, RedirectMode},
CompileError, IntoSpanned, RegId, Span,
@ -194,11 +194,25 @@ fn compile_pipeline(
out_reg,
)?;
// Clean up the redirection
finish_redirection(builder, redirect_modes, out_reg)?;
// only clean up the redirection if current element is not
// a subexpression. The subexpression itself already clean it.
if !is_subexpression(&element.expr.expr) {
// Clean up the redirection
finish_redirection(builder, redirect_modes, out_reg)?;
}
// The next pipeline element takes input from this output
in_reg = Some(out_reg);
}
Ok(())
}
fn is_subexpression(expr: &Expr) -> bool {
match expr {
Expr::FullCellPath(inner) => {
matches!(&inner.head.expr, &Expr::Subexpression(..))
}
Expr::Subexpression(..) => true,
_ => false,
}
}