From 9e00e8ab2d466f97f796b01ac1cf950b4dc1072f Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 9 Jul 2024 06:02:37 -0700 Subject: [PATCH] explicitly set span of binary op output --- crates/nu-engine/src/compile/builder.rs | 1 + crates/nu-engine/src/compile/expression.rs | 1 + crates/nu-engine/src/compile/operator.rs | 4 ++++ crates/nu-engine/src/eval_ir.rs | 11 ++++++++++- crates/nu-protocol/src/ir/display.rs | 3 +++ crates/nu-protocol/src/ir/mod.rs | 4 +++- crates/nu-protocol/src/pipeline/byte_stream.rs | 6 ++++++ crates/nu-protocol/src/pipeline/list_stream.rs | 6 ++++++ .../nu-protocol/src/pipeline/pipeline_data.rs | 18 ++++++++++++++++++ 9 files changed, 52 insertions(+), 2 deletions(-) diff --git a/crates/nu-engine/src/compile/builder.rs b/crates/nu-engine/src/compile/builder.rs index dd850321e5..c10df242b7 100644 --- a/crates/nu-engine/src/compile/builder.rs +++ b/crates/nu-engine/src/compile/builder.rs @@ -161,6 +161,7 @@ impl BlockBuilder { Instruction::Move { dst, src } => allocate(&[*src], &[*dst]), Instruction::Clone { dst, src } => allocate(&[*src], &[*dst, *src]), Instruction::Collect { src_dst } => allocate(&[*src_dst], &[*src_dst]), + Instruction::Span { src_dst } => allocate(&[*src_dst], &[*src_dst]), Instruction::Drop { src } => allocate(&[*src], &[]), Instruction::Drain { src } => allocate(&[*src], &[]), Instruction::LoadVariable { dst, var_id: _ } => allocate(&[], &[*dst]), diff --git a/crates/nu-engine/src/compile/expression.rs b/crates/nu-engine/src/compile/expression.rs index ff7c048447..32663a32cc 100644 --- a/crates/nu-engine/src/compile/expression.rs +++ b/crates/nu-engine/src/compile/expression.rs @@ -164,6 +164,7 @@ pub(crate) fn compile_expression( lhs, operator.clone().into_spanned(op.span), rhs, + expr.span, out_reg, ) } else { diff --git a/crates/nu-engine/src/compile/operator.rs b/crates/nu-engine/src/compile/operator.rs index 28a046f54f..e2619340d8 100644 --- a/crates/nu-engine/src/compile/operator.rs +++ b/crates/nu-engine/src/compile/operator.rs @@ -14,6 +14,7 @@ pub(crate) fn compile_binary_op( lhs: &Expression, op: Spanned, rhs: &Expression, + span: Span, out_reg: RegId, ) -> Result<(), CompileError> { if let Operator::Assignment(assign_op) = op.item { @@ -25,6 +26,7 @@ pub(crate) fn compile_binary_op( lhs, decomposed_op.into_spanned(op.span), rhs, + span, out_reg, )?; } else { @@ -136,6 +138,8 @@ pub(crate) fn compile_binary_op( )?; } + builder.push(Instruction::Span { src_dst: out_reg }.into_spanned(span))?; + Ok(()) } } diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 02f3ec3683..28d66c03f3 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -284,6 +284,8 @@ fn eval_instruction( ) -> Result { use self::InstructionResult::*; + // See the docs for `Instruction` for more information on what these instructions are supposed + // to do. match instruction { Instruction::Unreachable => Err(ShellError::IrEvalError { msg: "Reached unreachable code".into(), @@ -310,6 +312,12 @@ fn eval_instruction( ctx.put_reg(*src_dst, value); Ok(Continue) } + Instruction::Span { src_dst } => { + let data = ctx.take_reg(*src_dst); + let spanned = data.with_span(*span); + ctx.put_reg(*src_dst, spanned); + Ok(Continue) + } Instruction::Drop { src } => { ctx.take_reg(*src); Ok(Continue) @@ -893,7 +901,8 @@ fn binary_op( return Err(*error); } - // FIXME: there should be a span for both the operator and for the expr? + // We only have access to one span here, but the generated code usually adds a `span` + // instruction to set the output span to the right span. let op_span = span; let result = match op { diff --git a/crates/nu-protocol/src/ir/display.rs b/crates/nu-protocol/src/ir/display.rs index 3bc7a94c9d..7da914b4c1 100644 --- a/crates/nu-protocol/src/ir/display.rs +++ b/crates/nu-protocol/src/ir/display.rs @@ -80,6 +80,9 @@ impl<'a> fmt::Display for FmtInstruction<'a> { Instruction::Collect { src_dst } => { write!(f, "{:WIDTH$} {src_dst}", "collect") } + Instruction::Span { src_dst } => { + write!(f, "{:WIDTH$} {src_dst}", "span") + } Instruction::Drop { src } => { write!(f, "{:WIDTH$} {src}", "drop") } diff --git a/crates/nu-protocol/src/ir/mod.rs b/crates/nu-protocol/src/ir/mod.rs index 8f1a387107..8cccb6c5cf 100644 --- a/crates/nu-protocol/src/ir/mod.rs +++ b/crates/nu-protocol/src/ir/mod.rs @@ -110,7 +110,9 @@ pub enum Instruction { Clone { dst: RegId, src: RegId }, /// Collect a stream in a register to a value Collect { src_dst: RegId }, - /// Drop the value/straem in a register, without draining + /// Change the span of the contents of a register to the span of this instruction. + Span { src_dst: RegId }, + /// Drop the value/stream in a register, without draining Drop { src: RegId }, /// Drain the value/stream in a register and discard (e.g. semicolon). /// diff --git a/crates/nu-protocol/src/pipeline/byte_stream.rs b/crates/nu-protocol/src/pipeline/byte_stream.rs index babd195a9e..6d282a220e 100644 --- a/crates/nu-protocol/src/pipeline/byte_stream.rs +++ b/crates/nu-protocol/src/pipeline/byte_stream.rs @@ -357,6 +357,12 @@ impl ByteStream { self.span } + /// Changes the [`Span`] associated with the [`ByteStream`]. + pub fn with_span(mut self, span: Span) -> Self { + self.span = span; + self + } + /// Returns the [`ByteStreamType`] associated with the [`ByteStream`]. pub fn type_(&self) -> ByteStreamType { self.type_ diff --git a/crates/nu-protocol/src/pipeline/list_stream.rs b/crates/nu-protocol/src/pipeline/list_stream.rs index ab4d107149..dab2339a36 100644 --- a/crates/nu-protocol/src/pipeline/list_stream.rs +++ b/crates/nu-protocol/src/pipeline/list_stream.rs @@ -34,6 +34,12 @@ impl ListStream { self.span } + /// Changes the [`Span`] associated with this [`ListStream`]. + pub fn with_span(mut self, span: Span) -> Self { + self.span = span; + self + } + /// Convert a [`ListStream`] into its inner [`Value`] `Iterator`. pub fn into_inner(self) -> ValueIterator { self.stream diff --git a/crates/nu-protocol/src/pipeline/pipeline_data.rs b/crates/nu-protocol/src/pipeline/pipeline_data.rs index 54a231120b..435b475f40 100644 --- a/crates/nu-protocol/src/pipeline/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline/pipeline_data.rs @@ -99,6 +99,24 @@ impl PipelineData { } } + /// Change the span of the [`PipelineData`]. + /// + /// Returns `Value(Nothing)` with the given span if it was [`PipelineData::Empty`]. + pub fn with_span(self, span: Span) -> Self { + match self { + PipelineData::Empty => PipelineData::Value(Value::nothing(span), None), + PipelineData::Value(value, metadata) => { + PipelineData::Value(value.with_span(span), metadata) + } + PipelineData::ListStream(stream, metadata) => { + PipelineData::ListStream(stream.with_span(span), metadata) + } + PipelineData::ByteStream(stream, metadata) => { + PipelineData::ByteStream(stream.with_span(span), metadata) + } + } + } + /// Get a type that is representative of the `PipelineData`. /// /// The type returned here makes no effort to collect a stream, so it may be a different type