From 78ca94b57ca0218322b58cf1755870df748ff690 Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Tue, 9 Jul 2024 18:17:35 -0700 Subject: [PATCH] make sure compile errors generally have a span --- crates/nu-engine/src/compile/builder.rs | 24 +++++++--- crates/nu-engine/src/compile/mod.rs | 2 +- crates/nu-engine/src/compile/redirect.rs | 29 +++++++----- .../nu-protocol/src/errors/compile_error.rs | 46 ++++++++++--------- crates/nu-protocol/src/span.rs | 16 +++++++ 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/crates/nu-engine/src/compile/builder.rs b/crates/nu-engine/src/compile/builder.rs index 0d4d557858..d77075cc3f 100644 --- a/crates/nu-engine/src/compile/builder.rs +++ b/crates/nu-engine/src/compile/builder.rs @@ -10,6 +10,7 @@ pub(crate) struct LabelId(pub usize); /// Builds [`IrBlock`]s progressively by consuming instructions and handles register allocation. #[derive(Debug)] pub(crate) struct BlockBuilder { + pub(crate) block_span: Option, pub(crate) instructions: Vec, pub(crate) spans: Vec, /// The actual instruction index that a label refers to. While building IR, branch targets are @@ -28,8 +29,9 @@ pub(crate) struct BlockBuilder { impl BlockBuilder { /// Starts a new block, with the first register (`%0`) allocated as input. - pub(crate) fn new() -> Self { + pub(crate) fn new(block_span: Option) -> Self { BlockBuilder { + block_span, instructions: vec![], spans: vec![], labels: vec![], @@ -62,7 +64,9 @@ impl BlockBuilder { self.register_allocation_state.push(true); Ok(reg_id) } else { - Err(CompileError::RegisterOverflow) + Err(CompileError::RegisterOverflow { + block_span: self.block_span, + }) } } @@ -79,7 +83,9 @@ impl BlockBuilder { *is_allocated = true; Ok(()) } else { - Err(CompileError::RegisterOverflow) + Err(CompileError::RegisterOverflow { + block_span: self.block_span, + }) } } @@ -366,7 +372,9 @@ impl BlockBuilder { self.data.extend_from_slice(data); Ok(slice) } else { - Err(CompileError::DataOverflow) + Err(CompileError::DataOverflow { + block_span: self.block_span, + }) } } @@ -425,7 +433,9 @@ impl BlockBuilder { self.file_count = self .file_count .checked_add(1) - .ok_or(CompileError::FileOverflow)?; + .ok_or(CompileError::FileOverflow { + block_span: self.block_span, + })?; Ok(next) } @@ -481,7 +491,9 @@ impl BlockBuilder { if ended_loop == loop_ { Ok(()) } else { - Err(CompileError::IncoherentLoopState) + Err(CompileError::IncoherentLoopState { + block_span: self.block_span, + }) } } diff --git a/crates/nu-engine/src/compile/mod.rs b/crates/nu-engine/src/compile/mod.rs index c608646868..8f6ae22682 100644 --- a/crates/nu-engine/src/compile/mod.rs +++ b/crates/nu-engine/src/compile/mod.rs @@ -23,7 +23,7 @@ const BLOCK_INPUT: RegId = RegId(0); /// Compile Nushell pipeline abstract syntax tree (AST) to internal representation (IR) instructions /// for evaluation. pub fn compile(working_set: &StateWorkingSet, block: &Block) -> Result { - let mut builder = BlockBuilder::new(); + let mut builder = BlockBuilder::new(block.span); let span = block.span.unwrap_or(Span::unknown()); diff --git a/crates/nu-engine/src/compile/redirect.rs b/crates/nu-engine/src/compile/redirect.rs index 1064275b60..15af1a9f8c 100644 --- a/crates/nu-engine/src/compile/redirect.rs +++ b/crates/nu-engine/src/compile/redirect.rs @@ -72,13 +72,13 @@ pub(crate) fn redirect_modes_of_expression( let (out, err) = expression.expr.pipe_redirection(working_set); Ok(RedirectModes { out: out + .map(|r| r.into_spanned(redir_span)) .map(out_dest_to_redirect_mode) - .transpose()? - .map(|mode| mode.into_spanned(redir_span)), + .transpose()?, err: err + .map(|r| r.into_spanned(redir_span)) .map(out_dest_to_redirect_mode) - .transpose()? - .map(|mode| mode.into_spanned(redir_span)), + .transpose()?, }) } @@ -141,12 +141,17 @@ pub(crate) fn finish_redirection( Ok(()) } -pub(crate) fn out_dest_to_redirect_mode(out_dest: OutDest) -> Result { - match out_dest { - OutDest::Pipe => Ok(RedirectMode::Pipe), - OutDest::Capture => Ok(RedirectMode::Capture), - OutDest::Null => Ok(RedirectMode::Null), - OutDest::Inherit => Ok(RedirectMode::Inherit), - OutDest::File(_) => Err(CompileError::InvalidRedirectMode), - } +pub(crate) fn out_dest_to_redirect_mode( + out_dest: Spanned, +) -> Result, CompileError> { + let span = out_dest.span; + out_dest + .map(|out_dest| match out_dest { + OutDest::Pipe => Ok(RedirectMode::Pipe), + OutDest::Capture => Ok(RedirectMode::Capture), + OutDest::Null => Ok(RedirectMode::Null), + OutDest::Inherit => Ok(RedirectMode::Inherit), + OutDest::File(_) => Err(CompileError::InvalidRedirectMode { span }), + }) + .transpose() } diff --git a/crates/nu-protocol/src/errors/compile_error.rs b/crates/nu-protocol/src/errors/compile_error.rs index 9214289f42..cc805a73ed 100644 --- a/crates/nu-protocol/src/errors/compile_error.rs +++ b/crates/nu-protocol/src/errors/compile_error.rs @@ -8,11 +8,11 @@ use thiserror::Error; #[derive(Debug, Clone, Error, Diagnostic, PartialEq, Serialize, Deserialize)] pub enum CompileError { #[error("Register overflow.")] - #[diagnostic( - code(nu::compile::register_overflow), - help("the code being compiled is probably too large") - )] - RegisterOverflow, + #[diagnostic(code(nu::compile::register_overflow))] + RegisterOverflow { + #[label("the code being compiled is probably too large")] + block_span: Option, + }, #[error("Register {reg_id} was uninitialized when used, possibly reused.")] #[diagnostic( @@ -39,18 +39,30 @@ pub enum CompileError { code(nu::compile::data_overflow), help("try loading the string data from a file instead") )] - DataOverflow, + DataOverflow { + #[label("while compiling this block")] + block_span: Option, + }, #[error("Block contains too many files.")] #[diagnostic( code(nu::compile::register_overflow), help("try using fewer file redirections") )] - FileOverflow, + FileOverflow { + #[label("while compiling this block")] + block_span: Option, + }, #[error("Invalid redirect mode: File should not be specified by commands.")] - #[diagnostic(code(nu::compile::invalid_redirect_mode), help("this is a command bug. Please report it at https://github.com/nushell/nushell/issues/new"))] - InvalidRedirectMode, + #[diagnostic( + code(nu::compile::invalid_redirect_mode), + help("this is a command bug. Please report it at https://github.com/nushell/nushell/issues/new") + )] + InvalidRedirectMode { + #[label("while compiling this expression")] + span: Span, + }, #[error("Encountered garbage, likely due to parse error.")] #[diagnostic(code(nu::compile::garbage))] @@ -92,10 +104,6 @@ pub enum CompileError { span: Span, }, - #[error("Instruction index out of range: {index}.")] - #[diagnostic(code(nu::compile::instruction_index_out_of_range))] - InstructionIndexOutOfRange { index: usize }, - /// You're trying to run an unsupported external command. /// /// ## Resolution @@ -212,7 +220,10 @@ pub enum CompileError { code(nu::compile::incoherent_loop_state), help("this is a compiler bug. Please report it at https://github.com/nushell/nushell/issues/new"), )] - IncoherentLoopState, + IncoherentLoopState { + #[label("while compiling this block")] + block_span: Option, + }, #[error("Undefined label `{label_id}`.")] #[diagnostic( @@ -224,11 +235,4 @@ pub enum CompileError { #[label("label was used while compiling this code")] span: Option, }, - - #[error("Offset overflow: tried to add {offset} to {here}.")] - #[diagnostic( - code(nu::compile::offset_overflow), - help("this is a compiler bug. Please report it at https://github.com/nushell/nushell/issues/new"), - )] - OffsetOverflow { here: usize, offset: isize }, } diff --git a/crates/nu-protocol/src/span.rs b/crates/nu-protocol/src/span.rs index 0d280eaa9d..f5bcebc543 100644 --- a/crates/nu-protocol/src/span.rs +++ b/crates/nu-protocol/src/span.rs @@ -53,6 +53,22 @@ impl Spanned { } } +impl Spanned> { + /// Move the `Result` to the outside, resulting in a spanned `Ok` or unspanned `Err`. + pub fn transpose(self) -> Result, E> { + match self { + Spanned { + item: Ok(item), + span, + } => Ok(Spanned { item, span }), + Spanned { + item: Err(err), + span: _, + } => Err(err), + } + } +} + /// Helper trait to create [`Spanned`] more ergonomically. pub trait IntoSpanned: Sized { /// Wrap items together with a span into [`Spanned`].