diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 6eb56532a4..d6581bc255 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -46,9 +46,6 @@ fn setup_stack_and_engine_from_command(command: &str) -> (Stack, EngineState) { let mut stack = Stack::new(); - // Support running benchmarks without IR mode - stack.use_ir = std::env::var_os("NU_DISABLE_IR").is_none(); - evaluate_commands( &commands, &mut engine, diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 08f00727f0..5440f9caa2 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -306,9 +306,6 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { if let Err(err) = engine_state.merge_env(&mut stack) { report_shell_error(engine_state, &err); } - // Check whether $env.NU_DISABLE_IR is set, so that the user can change it in the REPL - // Temporary while IR eval is optional - stack.use_ir = !stack.has_env_var(engine_state, "NU_DISABLE_IR"); perf!("merge env", start_time, use_color); start_time = std::time::Instant::now(); diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index 4fee08a79c..5f1b42cb66 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/crates/nu-cmd-lang/src/core_commands/do_.rs @@ -82,9 +82,6 @@ impl Command for Do { bind_args_to(&mut callee_stack, &block.signature, rest, head)?; let eval_block_with_early_return = get_eval_block_with_early_return(engine_state); - // Applies to all block evaluation once set true - callee_stack.use_ir = !caller_stack.has_env_var(engine_state, "NU_DISABLE_IR"); - let result = eval_block_with_early_return(engine_state, &mut callee_stack, block, input); if has_env { diff --git a/crates/nu-command/src/debug/profile.rs b/crates/nu-command/src/debug/profile.rs index 41eee4c183..9f72d19d7c 100644 --- a/crates/nu-command/src/debug/profile.rs +++ b/crates/nu-command/src/debug/profile.rs @@ -30,8 +30,6 @@ impl Command for DebugProfile { "Collect pipeline element output values", Some('v'), ) - .switch("expr", "Collect expression types", Some('x')) - .switch("instructions", "Collect IR instructions", Some('i')) .switch("lines", "Collect line numbers", Some('l')) .named( "max-depth", @@ -48,37 +46,52 @@ impl Command for DebugProfile { } fn extra_description(&self) -> &str { - r#"The profiler profiles every evaluated pipeline element inside a closure, stepping into all + r#"The profiler profiles every evaluated instruction inside a closure, stepping into all commands calls and other blocks/closures. The output can be heavily customized. By default, the following columns are included: -- depth : Depth of the pipeline element. Each entered block adds one level of depth. How many +- depth : Depth of the instruction. Each entered block adds one level of depth. How many blocks deep to step into is controlled with the --max-depth option. -- id : ID of the pipeline element -- parent_id : ID of the parent element -- source : Source code of the pipeline element. If the element has multiple lines, only the - first line is used and `...` is appended to the end. Full source code can be shown - with the --expand-source flag. -- duration_ms : How long it took to run the pipeline element in milliseconds. -- (optional) span : Span of the element. Can be viewed via the `view span` command. Enabled with - the --spans flag. -- (optional) expr : The type of expression of the pipeline element. Enabled with the --expr flag. -- (optional) output : The output value of the pipeline element. Enabled with the --values flag. +- id : ID of the instruction +- parent_id : ID of the instruction that created the parent scope +- source : Source code that generated the instruction. If the source code has multiple lines, + only the first line is used and `...` is appended to the end. Full source code can + be shown with the --expand-source flag. +- pc : The index of the instruction within the block. +- instruction : The pretty printed instruction being evaluated. +- duration_ms : How long it took to run the instruction in milliseconds. +- (optional) span : Span associated with the instruction. Can be viewed via the `view span` + command. Enabled with the --spans flag. +- (optional) output : The output value of the instruction. Enabled with the --values flag. -To illustrate the depth and IDs, consider `debug profile { if true { echo 'spam' } }`. There are -three pipeline elements: +To illustrate the depth and IDs, consider `debug profile { do { if true { echo 'spam' } } }`. A unique ID is generated each time an instruction is executed, and there are two levels of depth: -depth id parent_id - 0 0 0 debug profile { do { if true { 'spam' } } } - 1 1 0 if true { 'spam' } - 2 2 1 'spam' +``` +depth id parent_id source pc instruction + 0 0 0 debug profile { do { if true { 'spam' } } } 0 + 1 1 0 { if true { 'spam' } } 0 load-literal %1, closure(2164) + 1 2 0 { if true { 'spam' } } 1 push-positional %1 + 1 3 0 { do { if true { 'spam' } } } 2 redirect-out caller + 1 4 0 { do { if true { 'spam' } } } 3 redirect-err caller + 1 5 0 do 4 call decl 7 "do", %0 + 2 6 5 true 0 load-literal %1, bool(true) + 2 7 5 if 1 not %1 + 2 8 5 if 2 branch-if %1, 5 + 2 9 5 'spam' 3 load-literal %0, string("spam") + 2 10 5 if 4 jump 6 + 2 11 5 { if true { 'spam' } } 6 return %0 + 1 12 0 { do { if true { 'spam' } } } 5 return %0 +``` Each block entered increments depth by 1 and each block left decrements it by one. This way you can -control the profiling granularity. Passing --max-depth=1 to the above would stop at -`if true { 'spam' }`. The id is used to identify each element. The parent_id tells you that 'spam' -was spawned from `if true { 'spam' }` which was spawned from the root `debug profile { ... }`. +control the profiling granularity. Passing --max-depth=1 to the above would stop inside the `do` +at `if true { 'spam' }`. The id is used to identify each element. The parent_id tells you that the +instructions inside the block are being executed because of `do` (5), which in turn was spawned from +the root `debug profile { ... }`. -Note: In some cases, the ordering of piepeline elements might not be intuitive. For example, +For a better understanding of how instructions map to source code, see the `view ir` command. + +Note: In some cases, the ordering of pipeline elements might not be intuitive. For example, `[ a bb cc ] | each { $in | str length }` involves some implicit collects and lazy evaluation confusing the id/parent_id hierarchy. The --expr flag is helpful for investigating these issues."# } @@ -94,8 +107,6 @@ confusing the id/parent_id hierarchy. The --expr flag is helpful for investigati let collect_spans = call.has_flag(engine_state, stack, "spans")?; let collect_expanded_source = call.has_flag(engine_state, stack, "expanded-source")?; let collect_values = call.has_flag(engine_state, stack, "values")?; - let collect_exprs = call.has_flag(engine_state, stack, "expr")?; - let collect_instructions = call.has_flag(engine_state, stack, "instructions")?; let collect_lines = call.has_flag(engine_state, stack, "lines")?; let max_depth = call .get_flag(engine_state, stack, "max-depth")? @@ -108,8 +119,8 @@ confusing the id/parent_id hierarchy. The --expr flag is helpful for investigati collect_source: true, collect_expanded_source, collect_values, - collect_exprs, - collect_instructions, + collect_exprs: false, + collect_instructions: true, collect_lines, }, call.span(), diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 3e5ad239fe..fc4a88f83e 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -1,20 +1,17 @@ use crate::eval_ir_block; #[allow(deprecated)] -use crate::{current_dir, get_full_help}; +use crate::get_full_help; use nu_path::{expand_path_with, AbsolutePathBuf}; use nu_protocol::{ - ast::{ - Assignment, Block, Call, Expr, Expression, ExternalArgument, PathMember, PipelineElement, - PipelineRedirection, RedirectionSource, RedirectionTarget, - }, + ast::{Assignment, Block, Call, Expr, Expression, ExternalArgument, PathMember}, debugger::DebugContext, - engine::{Closure, EngineState, Redirection, Stack, StateWorkingSet}, + engine::{Closure, EngineState, Stack}, eval_base::Eval, - BlockId, ByteStreamSource, Config, DataSource, FromValue, IntoPipelineData, OutDest, - PipelineData, PipelineMetadata, ShellError, Span, Spanned, Type, Value, VarId, ENV_VARIABLE_ID, + BlockId, Config, DataSource, IntoPipelineData, PipelineData, PipelineMetadata, ShellError, + Span, Type, Value, VarId, ENV_VARIABLE_ID, }; use nu_utils::IgnoreCaseExt; -use std::{fs::OpenOptions, path::PathBuf, sync::Arc}; +use std::sync::Arc; pub fn eval_call( engine_state: &EngineState, @@ -301,177 +298,6 @@ pub fn eval_expression_with_input( Ok(input) } -fn eval_redirection( - engine_state: &EngineState, - stack: &mut Stack, - target: &RedirectionTarget, - next_out: Option, -) -> Result { - match target { - RedirectionTarget::File { expr, append, .. } => { - #[allow(deprecated)] - let cwd = current_dir(engine_state, stack)?; - let value = eval_expression::(engine_state, stack, expr)?; - let path = Spanned::::from_value(value)?.item; - let path = expand_path_with(path, cwd, true); - - let mut options = OpenOptions::new(); - if *append { - options.append(true); - } else { - options.write(true).truncate(true); - } - Ok(Redirection::file(options.create(true).open(path)?)) - } - RedirectionTarget::Pipe { .. } => { - let dest = match next_out { - None | Some(OutDest::PipeSeparate) => OutDest::Pipe, - Some(next) => next, - }; - Ok(Redirection::Pipe(dest)) - } - } -} - -fn eval_element_redirection( - engine_state: &EngineState, - stack: &mut Stack, - element_redirection: Option<&PipelineRedirection>, - pipe_redirection: (Option, Option), -) -> Result<(Option, Option), ShellError> { - let (next_out, next_err) = pipe_redirection; - - if let Some(redirection) = element_redirection { - match redirection { - PipelineRedirection::Single { - source: RedirectionSource::Stdout, - target, - } => { - let stdout = eval_redirection::(engine_state, stack, target, next_out)?; - Ok((Some(stdout), next_err.map(Redirection::Pipe))) - } - PipelineRedirection::Single { - source: RedirectionSource::Stderr, - target, - } => { - let stderr = eval_redirection::(engine_state, stack, target, None)?; - if matches!(stderr, Redirection::Pipe(OutDest::Pipe)) { - let dest = match next_out { - None | Some(OutDest::PipeSeparate) => OutDest::Pipe, - Some(next) => next, - }; - // e>| redirection, don't override current stack `stdout` - Ok((None, Some(Redirection::Pipe(dest)))) - } else { - Ok((next_out.map(Redirection::Pipe), Some(stderr))) - } - } - PipelineRedirection::Single { - source: RedirectionSource::StdoutAndStderr, - target, - } => { - let stream = eval_redirection::(engine_state, stack, target, next_out)?; - Ok((Some(stream.clone()), Some(stream))) - } - PipelineRedirection::Separate { out, err } => { - let stdout = eval_redirection::(engine_state, stack, out, None)?; // `out` cannot be `RedirectionTarget::Pipe` - let stderr = eval_redirection::(engine_state, stack, err, next_out)?; - Ok((Some(stdout), Some(stderr))) - } - } - } else { - Ok(( - next_out.map(Redirection::Pipe), - next_err.map(Redirection::Pipe), - )) - } -} - -fn eval_element_with_input_inner( - engine_state: &EngineState, - stack: &mut Stack, - element: &PipelineElement, - input: PipelineData, -) -> Result { - let data = eval_expression_with_input::(engine_state, stack, &element.expr, input)?; - - let is_external = if let PipelineData::ByteStream(stream, ..) = &data { - matches!(stream.source(), ByteStreamSource::Child(..)) - } else { - false - }; - - if let Some(redirection) = element.redirection.as_ref() { - if !is_external { - match redirection { - &PipelineRedirection::Single { - source: RedirectionSource::Stderr, - target: RedirectionTarget::Pipe { span }, - } - | &PipelineRedirection::Separate { - err: RedirectionTarget::Pipe { span }, - .. - } => { - return Err(ShellError::GenericError { - error: "`e>|` only works on external commands".into(), - msg: "`e>|` only works on external commands".into(), - span: Some(span), - help: None, - inner: vec![], - }); - } - &PipelineRedirection::Single { - source: RedirectionSource::StdoutAndStderr, - target: RedirectionTarget::Pipe { span }, - } => { - return Err(ShellError::GenericError { - error: "`o+e>|` only works on external commands".into(), - msg: "`o+e>|` only works on external commands".into(), - span: Some(span), - help: None, - inner: vec![], - }); - } - _ => {} - } - } - } - - let data = if let Some(OutDest::File(file)) = stack.pipe_stdout() { - match &data { - PipelineData::Value(..) | PipelineData::ListStream(..) => { - data.write_to(file.as_ref())?; - PipelineData::Empty - } - PipelineData::ByteStream(..) => { - if !is_external { - data.write_to(file.as_ref())?; - PipelineData::Empty - } else { - data - } - } - PipelineData::Empty => PipelineData::Empty, - } - } else { - data - }; - - Ok(data) -} - -fn eval_element_with_input( - engine_state: &EngineState, - stack: &mut Stack, - element: &PipelineElement, - input: PipelineData, -) -> Result { - D::enter_element(engine_state, element); - let result = eval_element_with_input_inner::(engine_state, stack, element, input); - D::leave_element(engine_state, element, &result); - result -} - pub fn eval_block_with_early_return( engine_state: &EngineState, stack: &mut Stack, @@ -484,86 +310,13 @@ pub fn eval_block_with_early_return( } } -fn eval_block_inner( - engine_state: &EngineState, - stack: &mut Stack, - block: &Block, - mut input: PipelineData, -) -> Result { - // Remove once IR is the default. - if stack.use_ir { - return eval_ir_block::(engine_state, stack, block, input); - } - - let num_pipelines = block.len(); - - for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() { - let last_pipeline = pipeline_idx >= num_pipelines - 1; - - let Some((last, elements)) = pipeline.elements.split_last() else { - debug_assert!(false, "pipelines should have at least one element"); - continue; - }; - - for (i, element) in elements.iter().enumerate() { - let next = elements.get(i + 1).unwrap_or(last); - let (next_out, next_err) = next.pipe_redirection(&StateWorkingSet::new(engine_state)); - let (stdout, stderr) = eval_element_redirection::( - engine_state, - stack, - element.redirection.as_ref(), - (next_out.or(Some(OutDest::Pipe)), next_err), - )?; - let stack = &mut stack.push_redirection(stdout, stderr); - input = eval_element_with_input::(engine_state, stack, element, input)?; - } - - if last_pipeline { - let (stdout, stderr) = eval_element_redirection::( - engine_state, - stack, - last.redirection.as_ref(), - (stack.pipe_stdout().cloned(), stack.pipe_stderr().cloned()), - )?; - let stack = &mut stack.push_redirection(stdout, stderr); - input = eval_element_with_input::(engine_state, stack, last, input)?; - } else { - let (stdout, stderr) = eval_element_redirection::( - engine_state, - stack, - last.redirection.as_ref(), - (None, None), - )?; - let stack = &mut stack.push_redirection(stdout, stderr); - match eval_element_with_input::(engine_state, stack, last, input)? { - PipelineData::ByteStream(stream, ..) => { - let span = stream.span(); - if let Err(err) = stream.drain() { - stack.set_last_error(&err); - return Err(err); - } else { - stack.set_last_exit_code(0, span); - } - } - PipelineData::ListStream(stream, ..) => stream.drain()?, - PipelineData::Value(..) | PipelineData::Empty => {} - } - input = PipelineData::Empty; - } - } - - Ok(input) -} - pub fn eval_block( engine_state: &EngineState, stack: &mut Stack, block: &Block, input: PipelineData, ) -> Result { - D::enter_block(engine_state, block); - let result = eval_block_inner::(engine_state, stack, block, input); - D::leave_block(engine_state, block); + let result = eval_ir_block::(engine_state, stack, block, input); if let Err(err) = &result { stack.set_last_error(err); } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 3102e3a84e..ddd1169513 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -45,8 +45,6 @@ pub struct Stack { 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, pub parent_stack: Option>, /// Variables that have been deleted (this is used to hide values from parent stack lookups) @@ -78,7 +76,6 @@ impl Stack { active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], arguments: ArgumentStack::new(), error_handlers: ErrorHandlerStack::new(), - use_ir: true, recursion_count: 0, parent_stack: None, parent_deletions: vec![], @@ -99,7 +96,6 @@ impl Stack { active_overlays: parent.active_overlays.clone(), arguments: ArgumentStack::new(), error_handlers: ErrorHandlerStack::new(), - use_ir: parent.use_ir, recursion_count: parent.recursion_count, vars: vec![], parent_deletions: vec![], @@ -317,7 +313,6 @@ impl Stack { active_overlays: self.active_overlays.clone(), arguments: ArgumentStack::new(), error_handlers: ErrorHandlerStack::new(), - use_ir: self.use_ir, recursion_count: self.recursion_count, parent_stack: None, parent_deletions: vec![], @@ -351,7 +346,6 @@ impl Stack { active_overlays: self.active_overlays.clone(), arguments: ArgumentStack::new(), error_handlers: ErrorHandlerStack::new(), - use_ir: self.use_ir, recursion_count: self.recursion_count, parent_stack: None, parent_deletions: vec![], diff --git a/crates/nu-test-support/src/macros.rs b/crates/nu-test-support/src/macros.rs index fdfdabd0b5..f41c8cbc9d 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -248,7 +248,6 @@ pub struct NuOpts { pub locale: Option, pub envs: Option>, pub collapse_output: Option, - pub use_ir: Option, // Note: At the time this was added, passing in a file path was more convenient. However, // passing in file contents seems like a better API - consider this when adding new uses of // this field. @@ -301,15 +300,6 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O .stdout(Stdio::piped()) .stderr(Stdio::piped()); - // Explicitly set NU_DISABLE_IR - if let Some(use_ir) = opts.use_ir { - if !use_ir { - command.env("NU_DISABLE_IR", "1"); - } else { - command.env_remove("NU_DISABLE_IR"); - } - } - // Uncomment to debug the command being run: // println!("=== command\n{command:?}\n"); diff --git a/src/run.rs b/src/run.rs index 14c737c613..1cbfbc03a2 100644 --- a/src/run.rs +++ b/src/run.rs @@ -26,9 +26,6 @@ pub(crate) fn run_commands( let ask_to_create_config = nu_path::nu_config_dir().map_or(false, |p| !p.exists()); let mut stack = Stack::new(); - if stack.has_env_var(engine_state, "NU_DISABLE_IR") { - stack.use_ir = false; - } // if the --no-config-file(-n) option is NOT passed, load the plugin file, // load the default env file or custom (depending on parsed_nu_cli_args.env_file), @@ -119,10 +116,6 @@ pub(crate) fn run_file( trace!("run_file"); let mut stack = Stack::new(); - if stack.has_env_var(engine_state, "NU_DISABLE_IR") { - stack.use_ir = false; - } - // if the --no-config-file(-n) option is NOT passed, load the plugin file, // load the default env file or custom (depending on parsed_nu_cli_args.env_file), // and maybe a custom config file (depending on parsed_nu_cli_args.config_file) @@ -191,10 +184,6 @@ pub(crate) fn run_repl( let mut stack = Stack::new(); let start_time = std::time::Instant::now(); - if stack.has_env_var(engine_state, "NU_DISABLE_IR") { - stack.use_ir = false; - } - if parsed_nu_cli_args.no_config_file.is_none() { setup_config( engine_state, diff --git a/src/test_bins.rs b/src/test_bins.rs index 9752114977..7e684d5794 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -236,11 +236,6 @@ pub fn nu_repl() { engine_state.add_env_var("PWD".into(), Value::test_string(cwd.to_string_lossy())); engine_state.add_env_var("PATH".into(), Value::test_string("")); - // Disable IR in tests if set - if std::env::var_os("NU_DISABLE_IR").is_some() { - Arc::make_mut(&mut top_stack).use_ir = false; - } - let mut last_output = String::new(); load_standard_library(&mut engine_state).expect("Could not load the standard library."); diff --git a/tests/eval/mod.rs b/tests/eval/mod.rs index f8996f0a3e..191b19eb89 100644 --- a/tests/eval/mod.rs +++ b/tests/eval/mod.rs @@ -31,71 +31,43 @@ enum ExpectedOut<'a> { use self::ExpectedOut::*; fn test_eval(source: &str, expected_out: ExpectedOut) { - Playground::setup("test_eval_ast", |ast_dirs, _playground| { - Playground::setup("test_eval_ir", |ir_dirs, _playground| { - let actual_ast = nu!( - cwd: ast_dirs.test(), - use_ir: false, - source, - ); - let actual_ir = nu!( - cwd: ir_dirs.test(), - use_ir: true, - source, - ); + Playground::setup("test_eval", |dirs, _playground| { + let actual = nu!( + cwd: dirs.test(), + source, + ); - match expected_out { - Eq(eq) => { - assert_eq!(actual_ast.out, eq); - assert_eq!(actual_ir.out, eq); - assert!(actual_ast.status.success()); - assert!(actual_ir.status.success()); - } - Matches(regex) => { - let compiled_regex = Regex::new(regex).expect("regex failed to compile"); - assert!( - compiled_regex.is_match(&actual_ast.out), - "AST eval out does not match: {}\n{}", - regex, - actual_ast.out - ); - assert!( - compiled_regex.is_match(&actual_ir.out), - "IR eval out does not match: {}\n{}", - regex, - actual_ir.out, - ); - assert!(actual_ast.status.success()); - assert!(actual_ir.status.success()); - } - Error(regex) => { - let compiled_regex = Regex::new(regex).expect("regex failed to compile"); - assert!( - compiled_regex.is_match(&actual_ast.err), - "AST eval err does not match: {}", - regex - ); - assert!( - compiled_regex.is_match(&actual_ir.err), - "IR eval err does not match: {}", - regex - ); - assert!(!actual_ast.status.success()); - assert!(!actual_ir.status.success()); - } - FileEq(path, contents) => { - let ast_contents = std::fs::read_to_string(ast_dirs.test().join(path)) - .expect("failed to read AST file"); - let ir_contents = std::fs::read_to_string(ir_dirs.test().join(path)) - .expect("failed to read IR file"); - assert_eq!(ast_contents.trim(), contents); - assert_eq!(ir_contents.trim(), contents); - assert!(actual_ast.status.success()); - assert!(actual_ir.status.success()); - } + match expected_out { + Eq(eq) => { + assert_eq!(actual.out, eq); + assert!(actual.status.success()); } - assert_eq!(actual_ast.out, actual_ir.out); - }) + Matches(regex) => { + let compiled_regex = Regex::new(regex).expect("regex failed to compile"); + assert!( + compiled_regex.is_match(&actual.out), + "eval out does not match: {}\n{}", + regex, + actual.out, + ); + assert!(actual.status.success()); + } + Error(regex) => { + let compiled_regex = Regex::new(regex).expect("regex failed to compile"); + assert!( + compiled_regex.is_match(&actual.err), + "eval err does not match: {}", + regex + ); + assert!(!actual.status.success()); + } + FileEq(path, contents) => { + let read_contents = + std::fs::read_to_string(dirs.test().join(path)).expect("failed to read file"); + assert_eq!(read_contents.trim(), contents); + assert!(actual.status.success()); + } + } }); }