From 9ca0fb772ddc9ab362efa9dd15d98f12c7130b8f Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Sun, 15 Sep 2024 14:54:38 -0700 Subject: [PATCH] Make IR the default evaluator (#13718) # Description Makes IR the default evaluator, in preparation to remove the non-IR evaluator in a future release. # User-Facing Changes * Remove `NU_USE_IR` option * Add `NU_DISABLE_IR` option * IR is enabled unless `NU_DISABLE_IR` is set # After Submitting - [ ] release notes --- benches/benchmarks.rs | 4 +- crates/nu-cli/src/repl.rs | 4 +- .../nu-cmd-lang/src/core_commands/describe.rs | 2 +- crates/nu-cmd-lang/src/core_commands/do_.rs | 2 +- crates/nu-cmd-lang/src/core_commands/try_.rs | 14 ++--- crates/nu-cmd-lang/src/example_support.rs | 53 +++++++++++-------- crates/nu-command/src/formats/to/csv.rs | 3 +- crates/nu-command/src/formats/to/json.rs | 3 +- crates/nu-command/src/formats/to/md.rs | 3 +- crates/nu-command/src/formats/to/msgpack.rs | 3 +- crates/nu-command/src/formats/to/nuon.rs | 3 +- crates/nu-command/src/formats/to/text.rs | 3 +- crates/nu-command/src/formats/to/tsv.rs | 3 +- crates/nu-command/src/formats/to/xml.rs | 3 +- crates/nu-command/src/formats/to/yaml.rs | 3 +- crates/nu-protocol/src/engine/stack.rs | 2 +- crates/nu-test-support/src/macros.rs | 8 +-- src/run.rs | 12 ++--- src/test_bins.rs | 6 +-- 19 files changed, 77 insertions(+), 57 deletions(-) diff --git a/benches/benchmarks.rs b/benches/benchmarks.rs index 4efb666507..6eb56532a4 100644 --- a/benches/benchmarks.rs +++ b/benches/benchmarks.rs @@ -46,8 +46,8 @@ fn setup_stack_and_engine_from_command(command: &str) -> (Stack, EngineState) { let mut stack = Stack::new(); - // Support running benchmarks with IR mode - stack.use_ir = std::env::var_os("NU_USE_IR").is_some(); + // Support running benchmarks without IR mode + stack.use_ir = std::env::var_os("NU_DISABLE_IR").is_none(); evaluate_commands( &commands, diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 0e734571c9..d89af0bb9e 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -288,9 +288,9 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { if let Err(err) = engine_state.merge_env(&mut stack, cwd) { report_shell_error(engine_state, &err); } - // Check whether $env.NU_USE_IR is set, so that the user can change it in the REPL + // 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_USE_IR"); + 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/describe.rs b/crates/nu-cmd-lang/src/core_commands/describe.rs index 5df62e6793..ec98dc896f 100644 --- a/crates/nu-cmd-lang/src/core_commands/describe.rs +++ b/crates/nu-cmd-lang/src/core_commands/describe.rs @@ -70,7 +70,7 @@ impl Command for Describe { Example { description: "Describe the type of a record in a detailed way", example: - "{shell:'true', uwu:true, features: {bugs:false, multiplatform:true, speed: 10}, fib: [1 1 2 3 5 8], on_save: {|x| print $'Saving ($x)'}, first_commit: 2019-05-10, my_duration: (4min + 20sec)} | describe -d", + "{shell:'true', uwu:true, features: {bugs:false, multiplatform:true, speed: 10}, fib: [1 1 2 3 5 8], on_save: {|x| $'Saving ($x)'}, first_commit: 2019-05-10, my_duration: (4min + 20sec)} | describe -d", result: Some(Value::test_record(record!( "type" => Value::test_string("record"), "columns" => Value::test_record(record!( diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index 6c7c36e7d1..b05de81d57 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/crates/nu-cmd-lang/src/core_commands/do_.rs @@ -83,7 +83,7 @@ impl Command for Do { 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_USE_IR"); + 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); diff --git a/crates/nu-cmd-lang/src/core_commands/try_.rs b/crates/nu-cmd-lang/src/core_commands/try_.rs index 25d6fe59a3..9e5196c650 100644 --- a/crates/nu-cmd-lang/src/core_commands/try_.rs +++ b/crates/nu-cmd-lang/src/core_commands/try_.rs @@ -77,18 +77,18 @@ impl Command for Try { fn examples(&self) -> Vec { vec![ Example { - description: "Try to run a missing command", - example: "try { asdfasdf }", + description: "Try to run a division by zero", + example: "try { 1 / 0 }", result: None, }, Example { - description: "Try to run a missing command", - example: "try { asdfasdf } catch { 'missing' }", - result: Some(Value::test_string("missing")), + description: "Try to run a division by zero and return a string instead", + example: "try { 1 / 0 } catch { 'divided by zero' }", + result: Some(Value::test_string("divided by zero")), }, Example { - description: "Try to run a missing command and report the message", - example: "try { asdfasdf } catch { |err| $err.msg }", + description: "Try to run a division by zero and report the message", + example: "try { 1 / 0 } catch { |err| $err.msg }", result: None, }, ] diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index 9ae3808480..8df5a6c1e8 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -1,10 +1,7 @@ use itertools::Itertools; -use nu_engine::command_prelude::*; +use nu_engine::{command_prelude::*, compile}; use nu_protocol::{ - ast::Block, - debugger::WithoutDebug, - engine::{StateDelta, StateWorkingSet}, - report_shell_error, Range, + ast::Block, debugger::WithoutDebug, engine::StateWorkingSet, report_shell_error, Range, }; use std::{ sync::Arc, @@ -77,16 +74,25 @@ pub fn eval_pipeline_without_terminal_expression( cwd: &std::path::Path, engine_state: &mut Box, ) -> Option { - let (mut block, delta) = parse(src, engine_state); + let (mut block, mut working_set) = parse(src, engine_state); if block.pipelines.len() == 1 { let n_expressions = block.pipelines[0].elements.len(); - Arc::make_mut(&mut block).pipelines[0] - .elements - .truncate(&n_expressions - 1); + // Modify the block to remove the last element and recompile it + { + let mut_block = Arc::make_mut(&mut block); + mut_block.pipelines[0].elements.truncate(n_expressions - 1); + mut_block.ir_block = Some(compile(&working_set, mut_block).expect( + "failed to compile block modified by eval_pipeline_without_terminal_expression", + )); + } + working_set.add_block(block.clone()); + engine_state + .merge_delta(working_set.render()) + .expect("failed to merge delta"); if !block.pipelines[0].elements.is_empty() { let empty_input = PipelineData::empty(); - Some(eval_block(block, empty_input, cwd, engine_state, delta)) + Some(eval_block(block, empty_input, cwd, engine_state)) } else { Some(Value::nothing(Span::test_data())) } @@ -96,28 +102,30 @@ pub fn eval_pipeline_without_terminal_expression( } } -pub fn parse(contents: &str, engine_state: &EngineState) -> (Arc, StateDelta) { +pub fn parse<'engine>( + contents: &str, + engine_state: &'engine EngineState, +) -> (Arc, StateWorkingSet<'engine>) { let mut working_set = StateWorkingSet::new(engine_state); let output = nu_parser::parse(&mut working_set, None, contents.as_bytes(), false); if let Some(err) = working_set.parse_errors.first() { - panic!("test parse error in `{contents}`: {err:?}") + panic!("test parse error in `{contents}`: {err:?}"); } - (output, working_set.render()) + if let Some(err) = working_set.compile_errors.first() { + panic!("test compile error in `{contents}`: {err:?}"); + } + + (output, working_set) } pub fn eval_block( block: Arc, input: PipelineData, cwd: &std::path::Path, - engine_state: &mut Box, - delta: StateDelta, + engine_state: &EngineState, ) -> Value { - engine_state - .merge_delta(delta) - .expect("Error merging delta"); - let mut stack = Stack::new().capture(); stack.add_env_var("PWD".to_string(), Value::test_string(cwd.to_string_lossy())); @@ -191,8 +199,11 @@ fn eval( cwd: &std::path::Path, engine_state: &mut Box, ) -> Value { - let (block, delta) = parse(contents, engine_state); - eval_block(block, input, cwd, engine_state, delta) + let (block, working_set) = parse(contents, engine_state); + engine_state + .merge_delta(working_set.render()) + .expect("failed to merge delta"); + eval_block(block, input, cwd, engine_state) } pub struct DebuggableValue<'a>(pub &'a Value); diff --git a/crates/nu-command/src/formats/to/csv.rs b/crates/nu-command/src/formats/to/csv.rs index 4e971a3abb..73eacf49a7 100644 --- a/crates/nu-command/src/formats/to/csv.rs +++ b/crates/nu-command/src/formats/to/csv.rs @@ -137,7 +137,7 @@ mod test { use nu_cmd_lang::eval_pipeline_without_terminal_expression; - use crate::Metadata; + use crate::{Get, Metadata}; use super::*; @@ -157,6 +157,7 @@ mod test { working_set.add_decl(Box::new(ToCsv {})); working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(Get {})); working_set.render() }; diff --git a/crates/nu-command/src/formats/to/json.rs b/crates/nu-command/src/formats/to/json.rs index dc2b89c0c9..9e68961092 100644 --- a/crates/nu-command/src/formats/to/json.rs +++ b/crates/nu-command/src/formats/to/json.rs @@ -161,7 +161,7 @@ fn json_list(input: &[Value]) -> Result, ShellError> { mod test { use nu_cmd_lang::eval_pipeline_without_terminal_expression; - use crate::Metadata; + use crate::{Get, Metadata}; use super::*; @@ -182,6 +182,7 @@ mod test { working_set.add_decl(Box::new(ToJson {})); working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(Get {})); working_set.render() }; diff --git a/crates/nu-command/src/formats/to/md.rs b/crates/nu-command/src/formats/to/md.rs index 2cf6b5215e..434b65a8d9 100644 --- a/crates/nu-command/src/formats/to/md.rs +++ b/crates/nu-command/src/formats/to/md.rs @@ -335,7 +335,7 @@ fn get_padded_string(text: String, desired_length: usize, padding_character: cha #[cfg(test)] mod tests { - use crate::Metadata; + use crate::{Get, Metadata}; use super::*; use nu_cmd_lang::eval_pipeline_without_terminal_expression; @@ -474,6 +474,7 @@ mod tests { working_set.add_decl(Box::new(ToMd {})); working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(Get {})); working_set.render() }; diff --git a/crates/nu-command/src/formats/to/msgpack.rs b/crates/nu-command/src/formats/to/msgpack.rs index b4f7c7d79e..537aa9efdb 100644 --- a/crates/nu-command/src/formats/to/msgpack.rs +++ b/crates/nu-command/src/formats/to/msgpack.rs @@ -275,7 +275,7 @@ where mod test { use nu_cmd_lang::eval_pipeline_without_terminal_expression; - use crate::Metadata; + use crate::{Get, Metadata}; use super::*; @@ -296,6 +296,7 @@ mod test { working_set.add_decl(Box::new(ToMsgpack {})); working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(Get {})); working_set.render() }; diff --git a/crates/nu-command/src/formats/to/nuon.rs b/crates/nu-command/src/formats/to/nuon.rs index db29934a4d..8f4fc80e7f 100644 --- a/crates/nu-command/src/formats/to/nuon.rs +++ b/crates/nu-command/src/formats/to/nuon.rs @@ -107,7 +107,7 @@ mod test { use super::*; use nu_cmd_lang::eval_pipeline_without_terminal_expression; - use crate::Metadata; + use crate::{Get, Metadata}; #[test] fn test_examples() { @@ -126,6 +126,7 @@ mod test { working_set.add_decl(Box::new(ToNuon {})); working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(Get {})); working_set.render() }; diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 626596ea76..21b543d0eb 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -144,7 +144,7 @@ fn update_metadata(metadata: Option) -> Option Result mod test { use nu_cmd_lang::eval_pipeline_without_terminal_expression; - use crate::Metadata; + use crate::{Get, Metadata}; use super::*; @@ -146,6 +146,7 @@ mod test { working_set.add_decl(Box::new(ToYaml {})); working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(Get {})); working_set.render() }; diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 5819f1134b..854b338c1d 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -78,7 +78,7 @@ impl Stack { active_overlays: vec![DEFAULT_OVERLAY_NAME.to_string()], arguments: ArgumentStack::new(), error_handlers: ErrorHandlerStack::new(), - use_ir: false, + use_ir: true, recursion_count: 0, 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 3104c04e7d..2547b1f4c8 100644 --- a/crates/nu-test-support/src/macros.rs +++ b/crates/nu-test-support/src/macros.rs @@ -290,12 +290,12 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef, with_std: bool) -> O .stdout(Stdio::piped()) .stderr(Stdio::piped()); - // Explicitly set NU_USE_IR + // Explicitly set NU_DISABLE_IR if let Some(use_ir) = opts.use_ir { - if use_ir { - command.env("NU_USE_IR", "1"); + if !use_ir { + command.env("NU_DISABLE_IR", "1"); } else { - command.env_remove("NU_USE_IR"); + command.env_remove("NU_DISABLE_IR"); } } diff --git a/src/run.rs b/src/run.rs index 89e70b8f0f..068a581b72 100644 --- a/src/run.rs +++ b/src/run.rs @@ -26,8 +26,8 @@ pub(crate) fn run_commands( let mut stack = Stack::new(); let start_time = std::time::Instant::now(); - if stack.has_env_var(engine_state, "NU_USE_IR") { - stack.use_ir = true; + 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, @@ -115,8 +115,8 @@ pub(crate) fn run_file( trace!("run_file"); let mut stack = Stack::new(); - if stack.has_env_var(engine_state, "NU_USE_IR") { - stack.use_ir = true; + 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, @@ -184,8 +184,8 @@ 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_USE_IR") { - stack.use_ir = true; + if stack.has_env_var(engine_state, "NU_DISABLE_IR") { + stack.use_ir = false; } if parsed_nu_cli_args.no_config_file.is_none() { diff --git a/src/test_bins.rs b/src/test_bins.rs index c2ec287138..d5ae92ec60 100644 --- a/src/test_bins.rs +++ b/src/test_bins.rs @@ -244,9 +244,9 @@ 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("")); - // Enable IR in tests if set - if std::env::var_os("NU_USE_IR").is_some() { - Arc::make_mut(&mut top_stack).use_ir = true; + // 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();