diff --git a/crates/nu-engine/src/compile/mod.rs b/crates/nu-engine/src/compile/mod.rs index 1f66bf3131..02f6bcc0d6 100644 --- a/crates/nu-engine/src/compile/mod.rs +++ b/crates/nu-engine/src/compile/mod.rs @@ -4,7 +4,7 @@ use nu_protocol::{ PipelineRedirection, RedirectionSource, RedirectionTarget, }, engine::StateWorkingSet, - ir::{Instruction, IrBlock, Literal, RedirectMode}, + ir::{CallArg, Instruction, IrBlock, Literal, RedirectMode}, IntoSpanned, OutDest, RegId, ShellError, Span, Spanned, ENV_VARIABLE_ID, }; @@ -390,13 +390,7 @@ fn compile_call( // We could technically compile anything that isn't another call safely without worrying about // the argument state, but we'd have to check all of that first and it just isn't really worth // it. - enum CompiledArg { - Positional(RegId, Span), - Named(Box, Option, Span), - Spread(RegId, Span), - } - - let mut compiled_args = vec![]; + let mut call_args = vec![]; for arg in &call.arguments { let arg_reg = arg @@ -418,41 +412,28 @@ fn compile_call( .transpose()?; match arg { - Argument::Positional(_) => compiled_args.push(CompiledArg::Positional( + Argument::Positional(_) => call_args.push(CallArg::Positional( arg_reg.expect("expr() None in non-Named"), - arg.span(), - )), - Argument::Named((name, _, _)) => compiled_args.push(CompiledArg::Named( - name.item.as_str().into(), - arg_reg, - arg.span(), )), + Argument::Named((name, _, _)) => call_args.push({ + if let Some(arg_reg) = arg_reg { + CallArg::Named(name.item.as_str().into(), arg_reg) + } else { + CallArg::Flag(name.item.as_str().into()) + } + }), Argument::Unknown(_) => return Err(CompileError::Garbage), - Argument::Spread(_) => compiled_args.push(CompiledArg::Spread( - arg_reg.expect("expr() None in non-Named"), - arg.span(), - )), - } - } - - // Now that the args are all compiled, set up the call state (argument stack and redirections) - for arg in compiled_args { - match arg { - CompiledArg::Positional(reg, span) => { - builder.push(Instruction::PushPositional { src: reg }.into_spanned(span))? - } - CompiledArg::Named(name, Some(reg), span) => { - builder.push(Instruction::PushNamed { name, src: reg }.into_spanned(span))? - } - CompiledArg::Named(name, None, span) => { - builder.push(Instruction::PushFlag { name }.into_spanned(span))? - } - CompiledArg::Spread(reg, span) => { - builder.push(Instruction::AppendRest { src: reg }.into_spanned(span))? + Argument::Spread(_) => { + call_args.push(CallArg::Spread(arg_reg.expect("expr() None in non-Named"))) } } } + let args_start = builder.call_args.len(); + let args_len = call_args.len(); + builder.call_args.extend(call_args); + + // If redirections are needed for the call, set those up with extra instructions if let Some(mode) = redirect_modes.out { builder.push(mode.map(|mode| Instruction::RedirectOut { mode }))?; } @@ -466,6 +447,8 @@ fn compile_call( Instruction::Call { decl_id: call.decl_id, src_dst: io_reg, + args_start: if args_len > 0 { args_start } else { 0 }, + args_len, } .into_spanned(call.head), )?; @@ -629,6 +612,7 @@ impl CompileError { struct BlockBuilder { instructions: Vec, spans: Vec, + call_args: Vec, register_allocation_state: Vec, } @@ -638,6 +622,7 @@ impl BlockBuilder { BlockBuilder { instructions: vec![], spans: vec![], + call_args: vec![], register_allocation_state: vec![true], } } @@ -696,6 +681,11 @@ impl BlockBuilder { /// Insert an instruction into the block, automatically marking any registers populated by /// the instruction, and freeing any registers consumed by the instruction. fn push(&mut self, instruction: Spanned) -> Result<(), CompileError> { + // We have to mark any registers that may have been free before the instruction, and free + // any registers that are consumed by the instructions and will be free after. + // + // In the case of an src_dst type register, where it's assumed initialized before and is + // also replaced with a new value after the instruction, nothing needs to be done. match &instruction.item { Instruction::LoadLiteral { dst, lit: _ } => self.mark_register(*dst)?, Instruction::Move { dst, src } => { @@ -710,10 +700,6 @@ impl BlockBuilder { Instruction::LoadEnv { dst, key: _ } => self.mark_register(*dst)?, Instruction::LoadEnvOpt { dst, key: _ } => self.mark_register(*dst)?, Instruction::StoreEnv { key: _, src } => self.free_register(*src)?, - Instruction::PushPositional { src } => self.free_register(*src)?, - Instruction::AppendRest { src } => self.free_register(*src)?, - Instruction::PushFlag { name: _ } => (), - Instruction::PushNamed { name: _, src } => self.free_register(*src)?, Instruction::RedirectOut { mode } | Instruction::RedirectErr { mode } => match mode { RedirectMode::File { path, .. } => self.free_register(*path)?, _ => (), @@ -721,7 +707,22 @@ impl BlockBuilder { Instruction::Call { decl_id: _, src_dst: _, - } => (), + args_start, + args_len, + } => { + if *args_len > 0 { + // Free registers used by call arguments. This instruction can use a variable + // number of arguments, described by the sliced values in the `call_args` array. + for index in 0..*args_len { + match &self.call_args[*args_start + index] { + CallArg::Positional(reg) => self.free_register(*reg)?, + CallArg::Spread(reg) => self.free_register(*reg)?, + CallArg::Flag(_) => (), + CallArg::Named(_, reg) => self.free_register(*reg)?, + } + } + } + } Instruction::BinaryOp { lhs_dst: _, op: _, @@ -781,6 +782,7 @@ impl BlockBuilder { IrBlock { instructions: self.instructions, spans: self.spans, + call_args: self.call_args, register_count: self.register_allocation_state.len(), } } diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index de50df10eb..9eb9e7d8ef 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -85,7 +85,10 @@ fn eval_ir_block_impl( while pc < ir_block.instructions.len() { let instruction = &ir_block.instructions[pc]; let span = &ir_block.spans[pc]; - log::trace!("{pc:-4}: {}", instruction.display(ctx.engine_state)); + log::trace!( + "{pc:-4}: {}", + instruction.display(ctx.engine_state, &ir_block.call_args) + ); match do_instruction(ctx, instruction, span)? { InstructionResult::Continue => { pc += 1; @@ -159,13 +162,14 @@ fn do_instruction( Instruction::LoadEnv { dst, key } => todo!(), Instruction::LoadEnvOpt { dst, key } => todo!(), Instruction::StoreEnv { key, src } => todo!(), - Instruction::PushPositional { src } => todo!(), - Instruction::AppendRest { src } => todo!(), - Instruction::PushFlag { name } => todo!(), - Instruction::PushNamed { name, src } => todo!(), Instruction::RedirectOut { mode } => todo!(), Instruction::RedirectErr { mode } => todo!(), - Instruction::Call { decl_id, src_dst } => todo!(), + Instruction::Call { + decl_id, + src_dst, + args_start, + args_len, + } => todo!(), Instruction::BinaryOp { lhs_dst, op, rhs } => binary_op(ctx, *lhs_dst, op, *rhs, *span), Instruction::FollowCellPath { src_dst, path } => todo!(), Instruction::CloneCellPath { dst, src, path } => todo!(), diff --git a/crates/nu-protocol/src/ir/display.rs b/crates/nu-protocol/src/ir/display.rs index a630206fe6..b0fe9d5ead 100644 --- a/crates/nu-protocol/src/ir/display.rs +++ b/crates/nu-protocol/src/ir/display.rs @@ -1,8 +1,8 @@ -use std::fmt; +use std::fmt::{self, Write}; use crate::{engine::EngineState, DeclId, VarId}; -use super::{Instruction, IrBlock, RedirectMode}; +use super::{CallArg, Instruction, IrBlock, Literal, RedirectMode}; pub struct FmtIrBlock<'a> { pub(super) engine_state: &'a EngineState, @@ -27,6 +27,7 @@ impl<'a> fmt::Display for FmtIrBlock<'a> { index, FmtInstruction { engine_state: self.engine_state, + call_args: &self.ir_block.call_args, instruction } )?; @@ -37,6 +38,7 @@ impl<'a> fmt::Display for FmtIrBlock<'a> { pub struct FmtInstruction<'a> { pub(super) engine_state: &'a EngineState, + pub(super) call_args: &'a [CallArg], pub(super) instruction: &'a Instruction, } @@ -46,7 +48,7 @@ impl<'a> fmt::Display for FmtInstruction<'a> { match self.instruction { Instruction::LoadLiteral { dst, lit } => { - write!(f, "{:WIDTH$} {dst}, {lit:?}", "load-literal") + write!(f, "{:WIDTH$} {dst}, {lit}", "load-literal") } Instruction::Move { dst, src } => { write!(f, "{:WIDTH$} {dst}, {src}", "move") @@ -77,27 +79,25 @@ impl<'a> fmt::Display for FmtInstruction<'a> { Instruction::StoreEnv { key, src } => { write!(f, "{:WIDTH$} {key:?}, {src}", "store-env") } - Instruction::PushPositional { src } => { - write!(f, "{:WIDTH$} {src}", "push-positional") - } - Instruction::AppendRest { src } => { - write!(f, "{:WIDTH$} {src}", "append-rest") - } - Instruction::PushFlag { name } => { - write!(f, "{:WIDTH$} {name:?}", "push-flag") - } - Instruction::PushNamed { name, src } => { - write!(f, "{:WIDTH$} {name:?}, {src}", "push-named") - } Instruction::RedirectOut { mode } => { write!(f, "{:WIDTH$} {mode}", "redirect-out") } Instruction::RedirectErr { mode } => { write!(f, "{:WIDTH$} {mode}", "redirect-err") } - Instruction::Call { decl_id, src_dst } => { + Instruction::Call { + decl_id, + src_dst, + args_start, + args_len, + } => { let decl = FmtDecl::new(self.engine_state, *decl_id); - write!(f, "{:WIDTH$} {decl}, {src_dst}", "call") + let args = FmtCallArgs { + call_args: self.call_args, + args_start: *args_start, + args_len: *args_len, + }; + write!(f, "{:WIDTH$} {decl}, {src_dst}, {args}", "call") } Instruction::BinaryOp { lhs_dst, op, rhs } => { write!(f, "{:WIDTH$} {lhs_dst}, {op:?}, {rhs}", "binary-op") @@ -146,6 +146,34 @@ impl fmt::Display for FmtDecl<'_> { } } +struct FmtCallArgs<'a> { + call_args: &'a [CallArg], + args_start: usize, + args_len: usize, +} + +impl fmt::Display for FmtCallArgs<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_char('[')?; + for index in 0..self.args_len { + if index != 0 { + f.write_char(' ')?; + } + if let Some(arg) = self.call_args.get(self.args_start + index) { + match arg { + CallArg::Positional(reg) => write!(f, "{reg}")?, + CallArg::Spread(reg) => write!(f, "...{reg}")?, + CallArg::Flag(name) => write!(f, "--{name}")?, + CallArg::Named(name, reg) => write!(f, "--{name} {reg}")?, + } + } else { + f.write_str("")?; + } + } + f.write_char(']') + } +} + struct FmtVar<'a>(DeclId, Option<&'a str>); impl<'a> FmtVar<'a> { @@ -181,3 +209,12 @@ impl std::fmt::Display for RedirectMode { } } } + +impl std::fmt::Display for Literal { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Literal::CellPath(cell_path) => write!(f, "CellPath({})", cell_path), + _ => write!(f, "{:?}", self), + } + } +} diff --git a/crates/nu-protocol/src/ir/mod.rs b/crates/nu-protocol/src/ir/mod.rs index a89d54ea2c..76b095b35c 100644 --- a/crates/nu-protocol/src/ir/mod.rs +++ b/crates/nu-protocol/src/ir/mod.rs @@ -11,8 +11,19 @@ pub use display::{FmtInstruction, FmtIrBlock}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct IrBlock { + /// Instructions to execute in sequence in the block. + /// + /// Execution starts at index zero. A [`Return`](Instruction::Return) instruction must be + /// present. pub instructions: Vec, + /// Spans for each instruction. Indexes are matched 1:1 with the instructions, so this can be + /// zipped if desired. pub spans: Vec, + /// Array that describes arguments for [`Call`](Instruction::Call) instructions, sliced into by + /// the `args_start` and `args_len` fields. + pub call_args: Vec, + /// The number of registers to allocate at runtime. This number is statically determined during + /// compilation, and can't be adjusted dynamically. pub register_count: usize, } @@ -50,21 +61,21 @@ pub enum Instruction { LoadEnvOpt { dst: RegId, key: Box }, /// Store the value of an environment variable from the `src` register StoreEnv { key: Box, src: RegId }, - /// Add a positional arg to the next call - PushPositional { src: RegId }, - /// Add a list of args to the next call (spread/rest) - AppendRest { src: RegId }, - /// Add a named arg with no value to the next call. - PushFlag { name: Box }, - /// Add a named arg with a value to the next call. - PushNamed { name: Box, src: RegId }, /// Set the redirection for stdout for the next call (only) RedirectOut { mode: RedirectMode }, /// Set the redirection for stderr for the next call (only) RedirectErr { mode: RedirectMode }, /// Make a call. The input is taken from `src_dst`, and the output is placed in `src_dst`, - /// overwriting it. The argument stack is used implicitly and cleared when the call ends. - Call { decl_id: DeclId, src_dst: RegId }, + /// overwriting it. + /// + /// Call arguments are specified by the `args_start` and `args_len` fields, which point at a + /// range of values within the `arguments` array, and both may be zero for a zero-arg call. + Call { + decl_id: DeclId, + src_dst: RegId, + args_start: usize, + args_len: usize, + }, /// Do a binary operation on `lhs_dst` (left) and `rhs` (right) and write the result to /// `lhs_dst`. BinaryOp { @@ -97,9 +108,14 @@ pub enum Instruction { impl Instruction { /// Returns a value that can be formatted with [`Display`](std::fmt::Display) to show a detailed /// listing of the instruction. - pub fn display<'a>(&'a self, engine_state: &'a EngineState) -> FmtInstruction<'a> { + pub fn display<'a>( + &'a self, + engine_state: &'a EngineState, + call_args: &'a [CallArg], + ) -> FmtInstruction<'a> { FmtInstruction { engine_state, + call_args, instruction: self, } } @@ -129,6 +145,17 @@ pub enum Literal { Nothing, } +/// Describes an argument made to a call. This enum is contained within the `arguments` array of an +/// [`IrBlock`], which is referenced into by [`Instruction::Call`]. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub enum CallArg { + Positional(RegId), + Spread(RegId), + /// Like `Named`, but with no value. Smaller than using an `Option`. + Flag(Box), + Named(Box, RegId), +} + /// A redirection mode for the next call. See [`OutDest`](crate::OutDest). /// /// This is generated by: