From eee62ff58095c08c37b87dc7ce040d07552ee012 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Sun, 9 Jun 2024 02:09:22 -0700 Subject: [PATCH] support subexpressions, cell paths --- Cargo.lock | 1 + crates/nu-engine/Cargo.toml | 1 + crates/nu-engine/src/compile/mod.rs | 267 +++++++++++++++++++++------- crates/nu-protocol/src/ir/mod.rs | 36 ++-- 4 files changed, 224 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c95f0d1435..983f954df5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3024,6 +3024,7 @@ dependencies = [ name = "nu-engine" version = "0.94.3" dependencies = [ + "log", "nu-glob", "nu-path", "nu-protocol", diff --git a/crates/nu-engine/Cargo.toml b/crates/nu-engine/Cargo.toml index fae2b3eff4..a02dad6415 100644 --- a/crates/nu-engine/Cargo.toml +++ b/crates/nu-engine/Cargo.toml @@ -15,6 +15,7 @@ nu-protocol = { path = "../nu-protocol", features = ["plugin"], version = "0.94. nu-path = { path = "../nu-path", version = "0.94.3" } nu-glob = { path = "../nu-glob", version = "0.94.3" } nu-utils = { path = "../nu-utils", version = "0.94.3" } +log = { workspace = true } [features] plugin = [] diff --git a/crates/nu-engine/src/compile/mod.rs b/crates/nu-engine/src/compile/mod.rs index e638189170..d845727588 100644 --- a/crates/nu-engine/src/compile/mod.rs +++ b/crates/nu-engine/src/compile/mod.rs @@ -1,6 +1,6 @@ use nu_protocol::{ ast::{ - Argument, Block, Call, Expr, Expression, Operator, Pipeline, PipelineRedirection, + Argument, Block, Call, CellPath, Expr, Expression, Operator, Pipeline, PipelineRedirection, RedirectionSource, RedirectionTarget, }, engine::EngineState, @@ -15,40 +15,85 @@ const BLOCK_INPUT: RegId = RegId(0); pub fn compile(engine_state: &EngineState, block: &Block) -> Result { let mut builder = BlockBuilder::new(); - compile_block(engine_state, &mut builder, block, BLOCK_INPUT) + compile_block( + engine_state, + &mut builder, + block, + RedirectModes::default(), + Some(BLOCK_INPUT), + BLOCK_INPUT, + ) + .map_err(|err| err.to_shell_error(block.span))?; + + // A complete block has to end with a `return` + builder + .push( + Instruction::Return { src: BLOCK_INPUT } + .into_spanned(block.span.unwrap_or(Span::unknown())), + ) .map_err(|err| err.to_shell_error(block.span))?; Ok(builder.finish()) } +#[derive(Default)] +struct RedirectModes { + out: Option>, + err: Option>, +} + +impl RedirectModes { + fn capture_out(span: Span) -> Self { + RedirectModes { + out: Some(RedirectMode::Capture.into_spanned(span)), + err: None, + } + } +} + fn compile_block( engine_state: &EngineState, builder: &mut BlockBuilder, block: &Block, - input: RegId, + redirect_modes: RedirectModes, + in_reg: Option, + out_reg: RegId, ) -> Result<(), CompileError> { let span = block.span.unwrap_or(Span::unknown()); - let io_reg = input; + let mut redirect_modes = Some(redirect_modes); if !block.pipelines.is_empty() { let last_index = block.pipelines.len() - 1; for (index, pipeline) in block.pipelines.iter().enumerate() { - compile_pipeline(engine_state, builder, pipeline, span, io_reg)?; + compile_pipeline( + engine_state, + builder, + pipeline, + span, + // the redirect mode only applies to the last pipeline. + if index == last_index { + redirect_modes + .take() + .expect("should only take redirect_modes once") + } else { + RedirectModes::default() + }, + // input is only passed to the first pipeline. + if index == 0 { in_reg } else { None }, + out_reg, + )?; if index != last_index { - // Explicitly drain the I/O reg after each non-final pipeline, and replace - // with Nothing, because that's how the semicolon functions. - builder.push(Instruction::Drain { src: io_reg }.into_spanned(span))?; - builder.push( - Instruction::LoadLiteral { - dst: io_reg, - lit: Literal::Nothing, - } - .into_spanned(span), - )?; + // Explicitly drain the out reg after each non-final pipeline, because that's how + // the semicolon functions. + builder.push(Instruction::Drain { src: out_reg }.into_spanned(span))?; } } + Ok(()) + } else if in_reg.is_none() { + builder.load_nothing(out_reg) + } else { + Ok(()) } - builder.push(Instruction::Return { src: io_reg }.into_spanned(span)) } fn compile_pipeline( @@ -56,50 +101,77 @@ fn compile_pipeline( builder: &mut BlockBuilder, pipeline: &Pipeline, fallback_span: Span, - io_reg: RegId, + redirect_modes: RedirectModes, + in_reg: Option, + out_reg: RegId, ) -> Result<(), CompileError> { let mut iter = pipeline.elements.iter().peekable(); + let mut in_reg = in_reg; + let mut redirect_modes = Some(redirect_modes); while let Some(element) = iter.next() { let span = element.pipe.unwrap_or(fallback_span); // We have to get the redirection mode from either the explicit redirection in the pipeline - // element, or from the next expression if it's specified there. + // element, or from the next expression if it's specified there. If this is the last + // element, then it's from whatever is passed in as the mode to use. - let (out_mode_next, err_mode_next) = if let Some(next_element) = iter.peek() { - redirect_mode_of_expression(engine_state, &next_element.expr)? + let next_redirect_modes = if let Some(next_element) = iter.peek() { + redirect_modes_of_expression(engine_state, &next_element.expr, span)? } else { - (None, None) + redirect_modes + .take() + .expect("should only take redirect_modes once") }; - let (out_mode_spec, err_mode_spec) = match &element.redirection { + let spec_redirect_modes = match &element.redirection { Some(PipelineRedirection::Single { source, target }) => { let mode = redirection_target_to_mode(engine_state, builder, target, false)?; match source { - RedirectionSource::Stdout => (Some(mode), None), - RedirectionSource::Stderr => (None, Some(mode)), - RedirectionSource::StdoutAndStderr => (Some(mode), Some(mode)), + RedirectionSource::Stdout => RedirectModes { + out: Some(mode), + err: None, + }, + RedirectionSource::Stderr => RedirectModes { + out: None, + err: Some(mode), + }, + RedirectionSource::StdoutAndStderr => RedirectModes { + out: Some(mode), + err: Some(mode), + }, } } Some(PipelineRedirection::Separate { out, err }) => { let out = redirection_target_to_mode(engine_state, builder, out, true)?; let err = redirection_target_to_mode(engine_state, builder, err, true)?; - (Some(out), Some(err)) + RedirectModes { + out: Some(out), + err: Some(err), + } } - None => (None, None), + None => RedirectModes { + out: None, + err: None, + }, }; - let out_mode = out_mode_spec.or(out_mode_next.map(|mode| mode.into_spanned(span))); - let err_mode = err_mode_spec.or(err_mode_next.map(|mode| mode.into_spanned(span))); + let out_mode = spec_redirect_modes.out.or(next_redirect_modes.out); + let err_mode = spec_redirect_modes.err.or(next_redirect_modes.err); compile_expression( engine_state, builder, &element.expr, - out_mode, - err_mode, - Some(io_reg), - io_reg, + RedirectModes { + out: out_mode, + err: err_mode, + }, + in_reg, + out_reg, )?; + + // The next pipeline element takes input from this output + in_reg = Some(out_reg); } Ok(()) } @@ -121,8 +193,7 @@ fn redirection_target_to_mode( engine_state, builder, expr, - Some(RedirectMode::Capture.into_spanned(*redir_span)), - None, + RedirectModes::capture_out(*redir_span), None, path_reg, )?; @@ -141,15 +212,22 @@ fn redirection_target_to_mode( }) } -fn redirect_mode_of_expression( +fn redirect_modes_of_expression( engine_state: &EngineState, expression: &Expression, -) -> Result<(Option, Option), CompileError> { + redir_span: Span, +) -> Result { let (out, err) = expression.expr.pipe_redirection(&engine_state); - Ok(( - out.map(|out| out_dest_to_redirect_mode(out)).transpose()?, - err.map(|err| out_dest_to_redirect_mode(err)).transpose()?, - )) + Ok(RedirectModes { + out: out + .map(|out| out_dest_to_redirect_mode(out)) + .transpose()? + .map(|mode| mode.into_spanned(redir_span)), + err: err + .map(|err| out_dest_to_redirect_mode(err)) + .transpose()? + .map(|mode| mode.into_spanned(redir_span)), + }) } fn out_dest_to_redirect_mode(out_dest: OutDest) -> Result { @@ -166,8 +244,7 @@ fn compile_expression( engine_state: &EngineState, builder: &mut BlockBuilder, expr: &Expression, - out_mode: Option>, - err_mode: Option>, + redirect_modes: RedirectModes, in_reg: Option, out_reg: RegId, ) -> Result<(), CompileError> { @@ -207,7 +284,7 @@ fn compile_expression( builder.load_nothing(out_reg)?; } - compile_call(engine_state, builder, &call, out_mode, err_mode, out_reg) + compile_call(engine_state, builder, &call, redirect_modes, out_reg) } Expr::ExternalCall(_, _) => todo!(), Expr::Operator(_) => todo!(), @@ -228,7 +305,17 @@ fn compile_expression( Err(CompileError::UnsupportedOperatorExpression) } } - Expr::Subexpression(_) => todo!(), + Expr::Subexpression(block_id) => { + let block = engine_state.get_block(*block_id); + compile_block( + engine_state, + builder, + &block, + redirect_modes, + in_reg, + out_reg, + ) + } Expr::Block(_) => todo!(), Expr::Closure(_) => todo!(), Expr::MatchBlock(_) => todo!(), @@ -244,7 +331,33 @@ fn compile_expression( Expr::String(s) => lit(builder, Literal::String(s.as_str().into())), Expr::RawString(rs) => lit(builder, Literal::RawString(rs.as_str().into())), Expr::CellPath(path) => lit(builder, Literal::CellPath(Box::new(path.clone()))), - Expr::FullCellPath(_) => todo!(), + Expr::FullCellPath(full_cell_path) => { + compile_expression( + engine_state, + builder, + &full_cell_path.head, + RedirectModes::capture_out(expr.span), + in_reg, + out_reg, + )?; + // Only do the follow if this is actually needed + if !full_cell_path.tail.is_empty() { + let cell_path_reg = builder.literal( + Literal::CellPath(Box::new(CellPath { + members: full_cell_path.tail.clone(), + })) + .into_spanned(expr.span), + )?; + builder.push( + Instruction::FollowCellPath { + src_dst: out_reg, + path: cell_path_reg, + } + .into_spanned(expr.span), + )?; + } + Ok(()) + } Expr::ImportPattern(_) => todo!(), Expr::Overlay(_) => todo!(), Expr::Signature(_) => todo!(), @@ -258,8 +371,7 @@ fn compile_call( engine_state: &EngineState, builder: &mut BlockBuilder, call: &Call, - out_mode: Option>, - err_mode: Option>, + redirect_modes: RedirectModes, io_reg: RegId, ) -> Result<(), CompileError> { // It's important that we evaluate the args first before trying to set up the argument @@ -286,8 +398,7 @@ fn compile_call( engine_state, builder, expr, - Some(RedirectMode::Capture.into_spanned(arg.span())), - None, + RedirectModes::capture_out(arg.span()), None, arg_reg, )?; @@ -332,11 +443,11 @@ fn compile_call( } } - if let Some(mode) = out_mode { + if let Some(mode) = redirect_modes.out { builder.push(mode.map(|mode| Instruction::RedirectOut { mode }))?; } - if let Some(mode) = err_mode { + if let Some(mode) = redirect_modes.err { builder.push(mode.map(|mode| Instruction::RedirectErr { mode }))?; } @@ -373,8 +484,7 @@ fn compile_binary_op( engine_state, builder, lhs, - Some(RedirectMode::Capture.into_spanned(op.span)), - None, + RedirectModes::capture_out(op.span), in_reg, lhs_reg, )?; @@ -382,8 +492,7 @@ fn compile_binary_op( engine_state, builder, rhs, - Some(RedirectMode::Capture.into_spanned(op.span)), - None, + RedirectModes::capture_out(op.span), in_reg, rhs_reg, )?; @@ -491,7 +600,17 @@ impl BlockBuilder { } } - /// Mark a register as used, so that it can be used again by something else. + /// Mark a register as initialized. + fn mark_register(&mut self, reg_id: RegId) -> Result<(), CompileError> { + if let Some(is_allocated) = self.register_allocation_state.get_mut(reg_id.0 as usize) { + *is_allocated = true; + Ok(()) + } else { + Err(CompileError::RegisterOverflow) + } + } + + /// Mark a register as empty, so that it can be used again by something else. fn free_register(&mut self, reg_id: RegId) -> Result<(), CompileError> { let index = reg_id.0 as usize; @@ -503,6 +622,7 @@ impl BlockBuilder { self.register_allocation_state[index] = false; Ok(()) } else { + log::warn!("register {reg_id} uninitialized, builder = {self:#?}"); Err(CompileError::RegisterUninitialized(reg_id)) } } @@ -511,9 +631,12 @@ impl BlockBuilder { /// instruction. fn push(&mut self, instruction: Spanned) -> Result<(), CompileError> { match &instruction.item { - Instruction::LoadLiteral { dst: _, lit: _ } => (), - Instruction::Move { dst: _, src } => self.free_register(*src)?, - Instruction::Clone { dst: _, src: _ } => (), + Instruction::LoadLiteral { dst, lit: _ } => self.mark_register(*dst)?, + Instruction::Move { dst, src } => { + self.free_register(*src)?; + self.mark_register(*dst)?; + } + Instruction::Clone { dst, src: _ } => self.mark_register(*dst)?, Instruction::Collect { src_dst: _ } => (), Instruction::Drain { src } => self.free_register(*src)?, Instruction::PushPositional { src } => self.free_register(*src)?, @@ -543,17 +666,33 @@ impl BlockBuilder { Ok(()) } - /// Initialize a register with [`Nothing`](Literal::Nothing). - fn load_nothing(&mut self, reg_id: RegId) -> Result<(), CompileError> { + /// Load a register with a literal. + fn load_literal( + &mut self, + reg_id: RegId, + literal: Spanned, + ) -> Result<(), CompileError> { self.push( Instruction::LoadLiteral { dst: reg_id, - lit: Literal::Nothing, + lit: literal.item, } - .into_spanned(Span::unknown()), + .into_spanned(literal.span), ) } + /// Allocate a new register and load a literal into it. + fn literal(&mut self, literal: Spanned) -> Result { + let reg_id = self.next_register()?; + self.load_literal(reg_id, literal)?; + Ok(reg_id) + } + + /// Initialize a register with [`Nothing`](Literal::Nothing). + fn load_nothing(&mut self, reg_id: RegId) -> Result<(), CompileError> { + self.load_literal(reg_id, Literal::Nothing.into_spanned(Span::unknown())) + } + /// Consume the builder and produce the final [`IrBlock`]. fn finish(self) -> IrBlock { IrBlock { diff --git a/crates/nu-protocol/src/ir/mod.rs b/crates/nu-protocol/src/ir/mod.rs index 3431959c2c..fcb8da2792 100644 --- a/crates/nu-protocol/src/ir/mod.rs +++ b/crates/nu-protocol/src/ir/mod.rs @@ -74,57 +74,59 @@ pub enum Instruction { impl fmt::Display for Instruction { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + const WIDTH: usize = 20; + match self { Instruction::LoadLiteral { dst, lit } => { - write!(f, "{:15} {dst}, {lit:?}", "load-literal") + write!(f, "{:WIDTH$} {dst}, {lit:?}", "load-literal") } Instruction::Move { dst, src } => { - write!(f, "{:15} {dst}, {src}", "move") + write!(f, "{:WIDTH$} {dst}, {src}", "move") } Instruction::Clone { dst, src } => { - write!(f, "{:15} {dst}, {src}", "clone") + write!(f, "{:WIDTH$} {dst}, {src}", "clone") } Instruction::Collect { src_dst } => { - write!(f, "{:15} {src_dst}", "collect") + write!(f, "{:WIDTH$} {src_dst}", "collect") } Instruction::Drain { src } => { - write!(f, "{:15} {src}", "drain") + write!(f, "{:WIDTH$} {src}", "drain") } Instruction::PushPositional { src } => { - write!(f, "{:15} {src}", "push-positional") + write!(f, "{:WIDTH$} {src}", "push-positional") } Instruction::AppendRest { src } => { - write!(f, "{:15} {src}", "append-rest") + write!(f, "{:WIDTH$} {src}", "append-rest") } Instruction::PushFlag { name } => { - write!(f, "{:15} {name:?}", "push-flag") + write!(f, "{:WIDTH$} {name:?}", "push-flag") } Instruction::PushNamed { name, src } => { - write!(f, "{:15} {name:?}, {src}", "push-named") + write!(f, "{:WIDTH$} {name:?}, {src}", "push-named") } Instruction::RedirectOut { mode } => { - write!(f, "{:15} {mode}", "redirect-out") + write!(f, "{:WIDTH$} {mode}", "redirect-out") } Instruction::RedirectErr { mode } => { - write!(f, "{:15} {mode}", "redirect-err") + write!(f, "{:WIDTH$} {mode}", "redirect-err") } Instruction::Call { decl_id, src_dst } => { - write!(f, "{:15} decl {decl_id}, {src_dst}", "call") + write!(f, "{:WIDTH$} decl {decl_id}, {src_dst}", "call") } Instruction::BinaryOp { lhs_dst, op, rhs } => { - write!(f, "{:15} {lhs_dst}, {op:?}, {rhs}", "binary-op") + write!(f, "{:WIDTH$} {lhs_dst}, {op:?}, {rhs}", "binary-op") } Instruction::FollowCellPath { src_dst, path } => { - write!(f, "{:15} {src_dst}, {path}", "follow-cell-path") + write!(f, "{:WIDTH$} {src_dst}, {path}", "follow-cell-path") } Instruction::Jump { index } => { - write!(f, "{:15} {index}", "jump") + write!(f, "{:WIDTH$} {index}", "jump") } Instruction::BranchIf { cond, index } => { - write!(f, "{:15} {cond}, {index}", "branch-if") + write!(f, "{:WIDTH$} {cond}, {index}", "branch-if") } Instruction::Return { src } => { - write!(f, "{:15} {src}", "return") + write!(f, "{:WIDTH$} {src}", "return") } } }