From 8d86b10368067e8ccbfe28694325ba1d5ad121b9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 12 Feb 2024 13:25:45 -0600 Subject: [PATCH 1/5] fix(stream)!: Be consistent in using refs for Checkpoint --- src/_tutorial/chapter_3.rs | 6 +++--- src/ascii/mod.rs | 10 +++++----- src/binary/bits/mod.rs | 2 +- src/combinator/branch.rs | 8 ++++---- src/combinator/core.rs | 8 ++++---- src/combinator/multi.rs | 36 ++++++++++++++++++------------------ src/combinator/parser.rs | 18 +++++++++--------- src/error.rs | 2 +- src/parser.rs | 2 +- src/stream/mod.rs | 30 +++++++++++++++--------------- 10 files changed, 61 insertions(+), 61 deletions(-) diff --git a/src/_tutorial/chapter_3.rs b/src/_tutorial/chapter_3.rs index 4310b3c7..abb86af0 100644 --- a/src/_tutorial/chapter_3.rs +++ b/src/_tutorial/chapter_3.rs @@ -124,17 +124,17 @@ //! return Ok(output); //! } //! -//! input.reset(start); +//! input.reset(&start); //! if let Ok(output) = ("0o", parse_oct_digits).parse_next(input) { //! return Ok(output); //! } //! -//! input.reset(start); +//! input.reset(&start); //! if let Ok(output) = ("0d", parse_dec_digits).parse_next(input) { //! return Ok(output); //! } //! -//! input.reset(start); +//! input.reset(&start); //! ("0x", parse_hex_digits).parse_next(input) //! } //! diff --git a/src/ascii/mod.rs b/src/ascii/mod.rs index baf3d438..fdfa236a 100644 --- a/src/ascii/mod.rs +++ b/src/ascii/mod.rs @@ -1498,7 +1498,7 @@ where Some(_) => { if input.eof_offset() == current_len { let offset = input.offset_from(&start); - input.reset(start); + input.reset(&start); return Ok(input.next_slice(offset)); } } @@ -1507,7 +1507,7 @@ where let _ = escapable.parse_next(input)?; } else { let offset = input.offset_from(&start); - input.reset(start); + input.reset(&start); return Ok(input.next_slice(offset)); } } @@ -1540,7 +1540,7 @@ where Some(_) => { if input.eof_offset() == current_len { let offset = input.offset_from(&start); - input.reset(start); + input.reset(&start); return Ok(input.next_slice(offset)); } } @@ -1549,14 +1549,14 @@ where let _ = escapable.parse_next(input)?; } else { let offset = input.offset_from(&start); - input.reset(start); + input.reset(&start); return Ok(input.next_slice(offset)); } } } } - input.reset(start); + input.reset(&start); Ok(input.finish()) } diff --git a/src/binary/bits/mod.rs b/src/binary/bits/mod.rs index 91faa454..4045b18f 100644 --- a/src/binary/bits/mod.rs +++ b/src/binary/bits/mod.rs @@ -320,7 +320,7 @@ where if pattern == o { Ok(o) } else { - input.reset(start); + input.reset(&start); Err(ErrMode::Backtrack(E::from_error_kind( input, ErrorKind::Tag, diff --git a/src/combinator/branch.rs b/src/combinator/branch.rs index 5c1882be..33816e71 100644 --- a/src/combinator/branch.rs +++ b/src/combinator/branch.rs @@ -126,7 +126,7 @@ impl, P: Parser> Alt { error = match error { @@ -204,7 +204,7 @@ macro_rules! succ ( macro_rules! alt_trait_inner( ($it:tt, $self:expr, $input:expr, $start:ident, $err:expr, $head:ident $($id:ident)+) => ({ - $input.reset($start.clone()); + $input.reset(&$start); match $self.$it.parse_next($input) { Err(ErrMode::Backtrack(e)) => { let err = $err.or(e); @@ -266,7 +266,7 @@ macro_rules! permutation_trait_impl( // or errored on the remaining input if let Some(err) = err { // There are remaining parsers, and all errored on the remaining input - input.reset(start.clone()); + input.reset(&start); return Err(ErrMode::Backtrack(err.append(input, ErrorKind::Alt))); } @@ -284,7 +284,7 @@ macro_rules! permutation_trait_impl( macro_rules! permutation_trait_inner( ($it:tt, $self:expr, $input:ident, $start:ident, $res:expr, $err:expr, $head:ident $($id:ident)*) => ( if $res.$it.is_none() { - $input.reset($start.clone()); + $input.reset(&$start); match $self.$it.parse_next($input) { Ok(o) => { $res.$it = Some(o); diff --git a/src/combinator/core.rs b/src/combinator/core.rs index 5d620776..a100fd6d 100644 --- a/src/combinator/core.rs +++ b/src/combinator/core.rs @@ -79,7 +79,7 @@ where match f.parse_next(input) { Ok(o) => Ok(Some(o)), Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); Ok(None) } Err(e) => Err(e), @@ -148,7 +148,7 @@ where trace("peek", move |input: &mut I| { let start = input.checkpoint(); let res = f.parse_next(input); - input.reset(start); + input.reset(&start); res }) } @@ -211,7 +211,7 @@ where trace("not", move |input: &mut I| { let start = input.checkpoint(); let res = parser.parse_next(input); - input.reset(start); + input.reset(&start); match res { Ok(_) => Err(ErrMode::from_error_kind(input, ErrorKind::Not)), Err(ErrMode::Backtrack(_)) => Ok(()), @@ -408,7 +408,7 @@ where Some(o) } Err(ErrMode::Backtrack(_)) => { - self.input.reset(start); + self.input.reset(&start); self.state = Some(State::Done); None } diff --git a/src/combinator/multi.rs b/src/combinator/multi.rs index 3a30803d..192f00ee 100644 --- a/src/combinator/multi.rs +++ b/src/combinator/multi.rs @@ -311,7 +311,7 @@ where let len = i.eof_offset(); match f.parse_next(i) { Err(ErrMode::Backtrack(_)) => { - i.reset(start); + i.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -345,7 +345,7 @@ where let len = i.eof_offset(); match f.parse_next(i) { Err(ErrMode::Backtrack(_)) => { - i.reset(start); + i.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -426,7 +426,7 @@ where if count < min { return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); } else { - input.reset(start); + input.reset(&start); return Ok(res); } } @@ -507,7 +507,7 @@ where match g.parse_next(i) { Ok(o) => return Ok((res, o)), Err(ErrMode::Backtrack(_)) => { - i.reset(start); + i.reset(&start); match f.parse_next(i) { Err(e) => return Err(e.append(i, ErrorKind::Many)), Ok(o) => { @@ -566,7 +566,7 @@ where if count == max { return Err(ErrMode::Backtrack(err)); } - i.reset(start); + i.reset(&start); match f.parse_next(i) { Err(e) => { return Err(e.append(i, ErrorKind::Many)); @@ -735,7 +735,7 @@ where let start = input.checkpoint(); match parser.parse_next(input) { Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -749,7 +749,7 @@ where let len = input.eof_offset(); match separator.parse_next(input) { Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -764,7 +764,7 @@ where match parser.parse_next(input) { Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -804,7 +804,7 @@ where let len = input.eof_offset(); match separator.parse_next(input) { Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -819,7 +819,7 @@ where match parser.parse_next(input) { Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); return Ok(acc); } Err(e) => return Err(e), @@ -917,7 +917,7 @@ where match parser.parse_next(input) { Err(ErrMode::Backtrack(e)) => { if min == 0 { - input.reset(start); + input.reset(&start); return Ok(acc); } else { return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); @@ -937,7 +937,7 @@ where if index < min { return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); } else { - input.reset(start); + input.reset(&start); return Ok(acc); } } @@ -958,7 +958,7 @@ where if index < min { return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); } else { - input.reset(start); + input.reset(&start); return Ok(acc); } } @@ -1017,7 +1017,7 @@ where let len = i.eof_offset(); match sep.parse_next(i) { Err(ErrMode::Backtrack(_)) => { - i.reset(start); + i.reset(&start); return Ok(ol); } Err(e) => return Err(e), @@ -1029,7 +1029,7 @@ where match parser.parse_next(i) { Err(ErrMode::Backtrack(_)) => { - i.reset(start); + i.reset(&start); return Ok(ol); } Err(e) => return Err(e), @@ -1175,7 +1175,7 @@ where res = g(res, o); } Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); return Ok(res); } Err(e) => { @@ -1210,7 +1210,7 @@ where let len = input.eof_offset(); match f.parse_next(input) { Err(ErrMode::Backtrack(_)) => { - input.reset(start); + input.reset(&start); break; } Err(e) => return Err(e), @@ -1276,7 +1276,7 @@ where if count < min { return Err(ErrMode::Backtrack(err.append(input, ErrorKind::Many))); } else { - input.reset(start); + input.reset(&start); break; } } diff --git a/src/combinator/parser.rs b/src/combinator/parser.rs index 9ffdb3c2..9b38d79c 100644 --- a/src/combinator/parser.rs +++ b/src/combinator/parser.rs @@ -132,7 +132,7 @@ where let start = input.checkpoint(); let o = self.parser.parse_next(input)?; let res = (self.map)(o).map_err(|err| { - input.reset(start); + input.reset(&start); ErrMode::from_external_error(input, ErrorKind::Verify, err) }); trace_result("verify", &res); @@ -189,7 +189,7 @@ where let start = input.checkpoint(); let o = self.parser.parse_next(input)?; let res = (self.map)(o).ok_or_else(|| { - input.reset(start); + input.reset(&start); ErrMode::from_error_kind(input, ErrorKind::Verify) }); trace_result("verify", &res); @@ -247,7 +247,7 @@ where let mut o = self.outer.parse_next(i)?; let _ = o.complete(); let o2 = self.inner.parse_next(&mut o).map_err(|err| { - i.reset(start); + i.reset(&start); err })?; Ok(o2) @@ -301,7 +301,7 @@ where let start = i.checkpoint(); let o = self.p.parse_next(i)?; let res = o.parse_slice().ok_or_else(|| { - i.reset(start); + i.reset(&start); ErrMode::from_error_kind(i, ErrorKind::Verify) }); trace_result("verify", &res); @@ -447,7 +447,7 @@ where let start = input.checkpoint(); let o = self.parser.parse_next(input)?; let res = (self.filter)(o.borrow()).then_some(o).ok_or_else(|| { - input.reset(start); + input.reset(&start); ErrMode::from_error_kind(input, ErrorKind::Verify) }); trace_result("verify", &res); @@ -616,7 +616,7 @@ where match (self.parser).parse_next(input) { Ok(_) => { let offset = input.offset_from(&checkpoint); - input.reset(checkpoint); + input.reset(&checkpoint); let recognized = input.next_slice(offset); Ok(recognized) } @@ -665,7 +665,7 @@ where match (self.parser).parse_next(input) { Ok(result) => { let offset = input.offset_from(&checkpoint); - input.reset(checkpoint); + input.reset(&checkpoint); let recognized = input.next_slice(offset); Ok((result, recognized)) } @@ -997,7 +997,7 @@ where } } - i.reset(err_start.clone()); + i.reset(&err_start); err = err.map(|err| E::from_recoverable_error(&token_start, &err_start, i, err)); return Err(err); } @@ -1091,7 +1091,7 @@ where } } - i.reset(err_start.clone()); + i.reset(&err_start); err = err.map(|err| E::from_recoverable_error(&token_start, &err_start, i, err)); Err(err) } diff --git a/src/error.rs b/src/error.rs index 68555700..a9f49b7d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1117,7 +1117,7 @@ pub struct ParseError { impl> ParseError { pub(crate) fn new(mut input: I, start: I::Checkpoint, inner: E) -> Self { let offset = input.offset_from(&start); - input.reset(start); + input.reset(&start); Self { input, offset, diff --git a/src/parser.rs b/src/parser.rs index abdc8220..e3a1d305 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1051,7 +1051,7 @@ where }; let (mut input, mut errs) = input.into_parts(); - input.reset(start); + input.reset(&start); if let Some(err) = err { errs.push(err); } diff --git a/src/stream/mod.rs b/src/stream/mod.rs index 74710986..b1ad69cd 100644 --- a/src/stream/mod.rs +++ b/src/stream/mod.rs @@ -630,7 +630,7 @@ pub trait Stream: Offset<::Checkpoint> + crate::lib::std::fmt::D /// # Panic /// /// May panic if an invalid [`Self::Checkpoint`] is provided - fn reset(&mut self, checkpoint: Self::Checkpoint); + fn reset(&mut self, checkpoint: &Self::Checkpoint); /// Return the inner-most stream fn raw(&self) -> &dyn crate::lib::std::fmt::Debug; @@ -690,7 +690,7 @@ where Checkpoint::<_, Self>::new(*self) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { + fn reset(&mut self, checkpoint: &Self::Checkpoint) { *self = checkpoint.inner; } @@ -765,7 +765,7 @@ impl<'i> Stream for &'i str { Checkpoint::<_, Self>::new(*self) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { + fn reset(&mut self, checkpoint: &Self::Checkpoint) { *self = checkpoint.inner; } @@ -830,7 +830,7 @@ impl<'i> Stream for &'i Bytes { Checkpoint::<_, Self>::new(*self) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { + fn reset(&mut self, checkpoint: &Self::Checkpoint) { *self = checkpoint.inner; } @@ -895,7 +895,7 @@ impl<'i> Stream for &'i BStr { Checkpoint::<_, Self>::new(*self) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { + fn reset(&mut self, checkpoint: &Self::Checkpoint) { *self = checkpoint.inner; } @@ -972,8 +972,8 @@ where Checkpoint::<_, Self>::new((self.0.checkpoint(), self.1)) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { - self.0.reset(checkpoint.inner.0); + fn reset(&mut self, checkpoint: &Self::Checkpoint) { + self.0.reset(&checkpoint.inner.0); self.1 = checkpoint.inner.1; } @@ -1071,8 +1071,8 @@ impl Stream for Located { Checkpoint::<_, Self>::new(self.input.checkpoint()) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { - self.input.reset(checkpoint.inner); + fn reset(&mut self, checkpoint: &Self::Checkpoint) { + self.input.reset(&checkpoint.inner); } #[inline(always)] @@ -1128,8 +1128,8 @@ where Checkpoint::<_, Self>::new(self.input.checkpoint()) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { - self.input.reset(checkpoint.inner); + fn reset(&mut self, checkpoint: &Self::Checkpoint) { + self.input.reset(&checkpoint.inner); } #[inline(always)] @@ -1181,8 +1181,8 @@ impl Stream for Stateful { Checkpoint::<_, Self>::new(self.input.checkpoint()) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { - self.input.reset(checkpoint.inner); + fn reset(&mut self, checkpoint: &Self::Checkpoint) { + self.input.reset(&checkpoint.inner); } #[inline(always)] @@ -1234,8 +1234,8 @@ impl Stream for Partial { Checkpoint::<_, Self>::new(self.input.checkpoint()) } #[inline(always)] - fn reset(&mut self, checkpoint: Self::Checkpoint) { - self.input.reset(checkpoint.inner); + fn reset(&mut self, checkpoint: &Self::Checkpoint) { + self.input.reset(&checkpoint.inner); } #[inline(always)] From 13c855c4307156fca920ce851a348e8312ce0eed Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 12 Feb 2024 13:29:18 -0600 Subject: [PATCH 2/5] fix(error): Add missing feature gate for unstable-recover --- src/error.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/error.rs b/src/error.rs index a9f49b7d..fd7401fb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -314,6 +314,7 @@ pub trait AddContext: Sized { } /// Capture context from when an error was recovered +#[cfg(feature = "unstable-recover")] pub trait FromRecoverableError { /// Capture context from when an error was recovered fn from_recoverable_error( @@ -398,6 +399,7 @@ impl ParserError for InputError { impl AddContext for InputError {} +#[cfg(feature = "unstable-recover")] impl FromRecoverableError for InputError { #[inline] fn from_recoverable_error( @@ -469,6 +471,7 @@ impl ParserError for () { impl AddContext for () {} +#[cfg(feature = "unstable-recover")] impl FromRecoverableError for () { #[inline] fn from_recoverable_error( @@ -570,6 +573,7 @@ impl AddContext for ContextError { } } +#[cfg(feature = "unstable-recover")] impl FromRecoverableError for ContextError { #[inline] fn from_recoverable_error( @@ -910,6 +914,7 @@ where } #[cfg(feature = "std")] +#[cfg(feature = "unstable-recover")] impl FromRecoverableError for TreeError { #[inline] fn from_recoverable_error( From 9bb8a6ed7f6a923ab7ad0df8c638c5246f0acdab Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 12 Feb 2024 13:35:53 -0600 Subject: [PATCH 3/5] fix(debug): Always show context --- src/combinator/parser.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/combinator/parser.rs b/src/combinator/parser.rs index 9b38d79c..71d7b038 100644 --- a/src/combinator/parser.rs +++ b/src/combinator/parser.rs @@ -1,5 +1,6 @@ use crate::combinator::trace; use crate::combinator::trace_result; +use crate::combinator::DisplayDebug; #[cfg(feature = "unstable-recover")] use crate::error::FromRecoverableError; use crate::error::{AddContext, ErrMode, ErrorKind, FromExternalError, ParserError}; @@ -895,14 +896,11 @@ where { #[inline] fn parse_next(&mut self, i: &mut I) -> PResult { - #[cfg(feature = "debug")] - let name = format!("context={:?}", self.context); - #[cfg(not(feature = "debug"))] - let name = "context"; - trace(name, move |i: &mut I| { + let context = self.context.clone(); + trace(DisplayDebug(self.context.clone()), move |i: &mut I| { (self.parser) .parse_next(i) - .map_err(|err| err.add_context(i, self.context.clone())) + .map_err(|err| err.add_context(i, context.clone())) }) .parse_next(i) } From 3abee5ee9cf1bca32793cc049f77d884ccf0c5b9 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 12 Feb 2024 13:56:42 -0600 Subject: [PATCH 4/5] fix(error)!: Pass start token to AddContext Fixes #384 --- src/combinator/parser.rs | 3 +- src/error.rs | 62 +++++++++++++++++++++++----------------- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/combinator/parser.rs b/src/combinator/parser.rs index 71d7b038..ac361c38 100644 --- a/src/combinator/parser.rs +++ b/src/combinator/parser.rs @@ -898,9 +898,10 @@ where fn parse_next(&mut self, i: &mut I) -> PResult { let context = self.context.clone(); trace(DisplayDebug(self.context.clone()), move |i: &mut I| { + let start = i.checkpoint(); (self.parser) .parse_next(i) - .map_err(|err| err.add_context(i, context.clone())) + .map_err(|err| err.add_context(i, &start, context.clone())) }) .parse_next(i) } diff --git a/src/error.rs b/src/error.rs index fd7401fb..b8e1b0bc 100644 --- a/src/error.rs +++ b/src/error.rs @@ -220,10 +220,10 @@ where } } -impl> AddContext for ErrMode { +impl> AddContext for ErrMode { #[inline(always)] - fn add_context(self, input: &I, ctx: C) -> Self { - self.map(|err| err.add_context(input, ctx)) + fn add_context(self, input: &I, token_start: &::Checkpoint, context: C) -> Self { + self.map(|err| err.add_context(input, token_start, context)) } } @@ -302,13 +302,18 @@ pub trait ParserError: Sized { /// Used by [`Parser::context`] to add custom data to error while backtracking /// /// May be implemented multiple times for different kinds of context. -pub trait AddContext: Sized { +pub trait AddContext: Sized { /// Append to an existing error custom data /// /// This is used mainly by [`Parser::context`], to add user friendly information /// to errors when backtracking through a parse tree #[inline] - fn add_context(self, _input: &I, _ctx: C) -> Self { + fn add_context( + self, + _input: &I, + _token_start: &::Checkpoint, + _context: C, + ) -> Self { self } } @@ -397,7 +402,7 @@ impl ParserError for InputError { } } -impl AddContext for InputError {} +impl AddContext for InputError {} #[cfg(feature = "unstable-recover")] impl FromRecoverableError for InputError { @@ -469,7 +474,7 @@ impl ParserError for () { fn append(self, _: &I, _: ErrorKind) -> Self {} } -impl AddContext for () {} +impl AddContext for () {} #[cfg(feature = "unstable-recover")] impl FromRecoverableError for () { @@ -564,11 +569,16 @@ impl ParserError for ContextError { } } -impl AddContext for ContextError { +impl AddContext for ContextError { #[inline] - fn add_context(mut self, _input: &I, ctx: C) -> Self { + fn add_context( + mut self, + _input: &I, + _token_start: &::Checkpoint, + context: C, + ) -> Self { #[cfg(feature = "alloc")] - self.context.push(ctx); + self.context.push(context); self } } @@ -797,6 +807,7 @@ pub struct TreeErrorContext { #[cfg(feature = "std")] impl<'i, I: ToOwned, C> TreeError<&'i I, C> where + &'i I: Stream + Clone, ::Owned: Clone, { /// Obtaining ownership @@ -808,7 +819,7 @@ where #[cfg(feature = "std")] impl TreeError where - I: Clone, + I: Stream + Clone, { /// Translate the input type pub fn map_input I2>(self, op: O) -> TreeError { @@ -861,7 +872,7 @@ where #[cfg(feature = "std")] impl ParserError for TreeError where - I: Clone, + I: Stream + Clone, { fn from_error_kind(input: &I, kind: ErrorKind) -> Self { TreeError::Base(TreeErrorBase { @@ -902,20 +913,19 @@ where #[cfg(feature = "std")] impl AddContext for TreeError where - I: Clone, + I: Stream + Clone, { - fn add_context(self, input: &I, context: C) -> Self { - let frame = TreeErrorFrame::Context(TreeErrorContext { - input: input.clone(), - context, - }); + fn add_context(self, input: &I, token_start: &::Checkpoint, context: C) -> Self { + let mut input = input.clone(); + input.reset(token_start); + let frame = TreeErrorFrame::Context(TreeErrorContext { input, context }); self.append_frame(frame) } } #[cfg(feature = "std")] #[cfg(feature = "unstable-recover")] -impl FromRecoverableError for TreeError { +impl FromRecoverableError for TreeError { #[inline] fn from_recoverable_error( _token_start: &::Checkpoint, @@ -930,7 +940,7 @@ impl FromRecoverableError for TreeError { #[cfg(feature = "std")] impl FromExternalError for TreeError where - I: Clone, + I: Stream + Clone, { fn from_external_error(input: &I, kind: ErrorKind, e: E) -> Self { TreeError::Base(TreeErrorBase { @@ -944,7 +954,7 @@ where #[cfg(feature = "std")] impl TreeError where - I: Clone + crate::lib::std::fmt::Display, + I: Stream + Clone + crate::lib::std::fmt::Display, C: fmt::Display, { fn write(&self, f: &mut fmt::Formatter<'_>, indent: usize) -> fmt::Result { @@ -979,7 +989,7 @@ where } #[cfg(feature = "std")] -impl fmt::Display for TreeErrorBase { +impl fmt::Display for TreeErrorBase { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if let Some(cause) = self.cause.as_ref() { write!(f, "caused by {cause}")?; @@ -994,7 +1004,7 @@ impl fmt::Display for TreeErrorBase { } #[cfg(feature = "std")] -impl fmt::Display for TreeErrorContext { +impl fmt::Display for TreeErrorContext { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let context = &self.context; let input = abbreviate(self.input.to_string()); @@ -1005,7 +1015,7 @@ impl fmt::Display for TreeErrorContext #[cfg(feature = "std")] impl< - I: Clone + fmt::Debug + fmt::Display + Sync + Send + 'static, + I: Stream + Clone + fmt::Debug + fmt::Display + Sync + Send + 'static, C: fmt::Display + fmt::Debug, > std::error::Error for TreeError { @@ -1035,7 +1045,7 @@ fn abbreviate(input: String) -> String { } #[cfg(feature = "std")] -impl fmt::Display for TreeError { +impl fmt::Display for TreeError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.write(f, 0) } @@ -1091,7 +1101,7 @@ impl ParserError for ErrorKind { } } -impl AddContext for ErrorKind {} +impl AddContext for ErrorKind {} impl FromExternalError for ErrorKind { /// Create a new error from an input position and an external error From 8cec29cf716c4cbbf8f99018b71ac9d781edd228 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 12 Feb 2024 14:15:44 -0600 Subject: [PATCH 5/5] fix(error)!: Pass start token to ParserError::append --- examples/custom_error.rs | 5 +-- src/binary/bits/mod.rs | 1 + src/combinator/branch.rs | 8 ++--- src/combinator/multi.rs | 43 ++++++++++++++++-------- src/combinator/tests.rs | 12 +++---- src/error.rs | 56 +++++++++++++++++++++++--------- src/parser.rs | 4 +-- tests/testsuite/custom_errors.rs | 8 ++++- 8 files changed, 92 insertions(+), 45 deletions(-) diff --git a/examples/custom_error.rs b/examples/custom_error.rs index 998e5ad6..30aa9e88 100644 --- a/examples/custom_error.rs +++ b/examples/custom_error.rs @@ -2,6 +2,7 @@ use winnow::error::ErrMode; use winnow::error::ErrorKind; use winnow::error::ParserError; use winnow::prelude::*; +use winnow::stream::Stream; #[derive(Debug, PartialEq, Eq)] pub enum CustomError { @@ -9,12 +10,12 @@ pub enum CustomError { Nom(I, ErrorKind), } -impl ParserError for CustomError { +impl ParserError for CustomError { fn from_error_kind(input: &I, kind: ErrorKind) -> Self { CustomError::Nom(input.clone(), kind) } - fn append(self, _: &I, _: ErrorKind) -> Self { + fn append(self, _: &I, _: &::Checkpoint, _: ErrorKind) -> Self { self } } diff --git a/src/binary/bits/mod.rs b/src/binary/bits/mod.rs index 4045b18f..8df1dc82 100644 --- a/src/binary/bits/mod.rs +++ b/src/binary/bits/mod.rs @@ -50,6 +50,7 @@ pub fn bits(mut parser: P) -> impl Parser where E1: ParserError<(I, usize)> + ErrorConvert, E2: ParserError, + (I, usize): Stream, I: Stream + Clone, P: Parser<(I, usize), O, E1>, { diff --git a/src/combinator/branch.rs b/src/combinator/branch.rs index 33816e71..7c0064b6 100644 --- a/src/combinator/branch.rs +++ b/src/combinator/branch.rs @@ -139,7 +139,7 @@ impl, P: Parser> Alt Err(ErrMode::Backtrack(e.append(input, ErrorKind::Alt))), + Some(e) => Err(ErrMode::Backtrack(e.append(input, &start, ErrorKind::Alt))), None => Err(ErrMode::assert(input, "`alt` needs at least one parser")), } } @@ -214,14 +214,14 @@ macro_rules! alt_trait_inner( } }); ($it:tt, $self:expr, $input:expr, $start:ident, $err:expr, $head:ident) => ({ - Err(ErrMode::Backtrack($err.append($input, ErrorKind::Alt))) + Err(ErrMode::Backtrack($err.append($input, &$start, ErrorKind::Alt))) }); ); alt_trait!(Alt2 Alt3 Alt4 Alt5 Alt6 Alt7 Alt8 Alt9 Alt10 Alt11 Alt12 Alt13 Alt14 Alt15 Alt16 Alt17 Alt18 Alt19 Alt20 Alt21 Alt22); // Manually implement Alt for (A,), the 1-tuple type -impl, A: Parser> Alt for (A,) { +impl, A: Parser> Alt for (A,) { fn choice(&mut self, input: &mut I) -> PResult { self.0.parse_next(input) } @@ -267,7 +267,7 @@ macro_rules! permutation_trait_impl( if let Some(err) = err { // There are remaining parsers, and all errored on the remaining input input.reset(&start); - return Err(ErrMode::Backtrack(err.append(input, ErrorKind::Alt))); + return Err(ErrMode::Backtrack(err.append(input, &start, ErrorKind::Alt))); } // All parsers were applied diff --git a/src/combinator/multi.rs b/src/combinator/multi.rs index 192f00ee..3e95c94a 100644 --- a/src/combinator/multi.rs +++ b/src/combinator/multi.rs @@ -334,8 +334,9 @@ where F: Parser, E: ParserError, { + let start = i.checkpoint(); match f.parse_next(i) { - Err(e) => Err(e.append(i, ErrorKind::Many)), + Err(e) => Err(e.append(i, &start, ErrorKind::Many)), Ok(o) => { let mut acc = C::initial(None); acc.accumulate(o); @@ -373,6 +374,7 @@ where let mut res = C::initial(Some(count)); for _ in 0..count { + let start = i.checkpoint(); let len = i.eof_offset(); match f.parse_next(i) { Ok(o) => { @@ -384,7 +386,7 @@ where res.accumulate(o); } Err(e) => { - return Err(e.append(i, ErrorKind::Many)); + return Err(e.append(i, &start, ErrorKind::Many)); } } } @@ -424,7 +426,7 @@ where } Err(ErrMode::Backtrack(e)) => { if count < min { - return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); + return Err(ErrMode::Backtrack(e.append(input, &start, ErrorKind::Many))); } else { input.reset(&start); return Ok(res); @@ -509,7 +511,7 @@ where Err(ErrMode::Backtrack(_)) => { i.reset(&start); match f.parse_next(i) { - Err(e) => return Err(e.append(i, ErrorKind::Many)), + Err(e) => return Err(e.append(i, &start, ErrorKind::Many)), Ok(o) => { // infinite loop check: the parser must always consume if i.eof_offset() == len { @@ -547,13 +549,15 @@ where } let mut res = C::initial(Some(min)); + + let start = i.checkpoint(); for _ in 0..min { match f.parse_next(i) { Ok(o) => { res.accumulate(o); } Err(e) => { - return Err(e.append(i, ErrorKind::Many)); + return Err(e.append(i, &start, ErrorKind::Many)); } } } @@ -569,7 +573,7 @@ where i.reset(&start); match f.parse_next(i) { Err(e) => { - return Err(e.append(i, ErrorKind::Many)); + return Err(e.append(i, &start, ErrorKind::Many)); } Ok(o) => { // infinite loop check: the parser must always consume @@ -851,9 +855,10 @@ where return Ok(acc); } + let start = input.checkpoint(); match parser.parse_next(input) { Err(e) => { - return Err(e.append(input, ErrorKind::Many)); + return Err(e.append(input, &start, ErrorKind::Many)); } Ok(o) => { acc.accumulate(o); @@ -861,10 +866,11 @@ where } for _ in 1..count { + let start = input.checkpoint(); let len = input.eof_offset(); match separator.parse_next(input) { Err(e) => { - return Err(e.append(input, ErrorKind::Many)); + return Err(e.append(input, &start, ErrorKind::Many)); } Ok(_) => { // infinite loop check @@ -877,7 +883,7 @@ where match parser.parse_next(input) { Err(e) => { - return Err(e.append(input, ErrorKind::Many)); + return Err(e.append(input, &start, ErrorKind::Many)); } Ok(o) => { acc.accumulate(o); @@ -920,7 +926,7 @@ where input.reset(&start); return Ok(acc); } else { - return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); + return Err(ErrMode::Backtrack(e.append(input, &start, ErrorKind::Many))); } } Err(e) => return Err(e), @@ -935,7 +941,7 @@ where match separator.parse_next(input) { Err(ErrMode::Backtrack(e)) => { if index < min { - return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); + return Err(ErrMode::Backtrack(e.append(input, &start, ErrorKind::Many))); } else { input.reset(&start); return Ok(acc); @@ -956,7 +962,11 @@ where match parser.parse_next(input) { Err(ErrMode::Backtrack(e)) => { if index < min { - return Err(ErrMode::Backtrack(e.append(input, ErrorKind::Many))); + return Err(ErrMode::Backtrack(e.append( + input, + &start, + ErrorKind::Many, + ))); } else { input.reset(&start); return Ok(acc); @@ -1130,12 +1140,13 @@ where { trace("fill", move |i: &mut I| { for elem in buf.iter_mut() { + let start = i.checkpoint(); match f.parse_next(i) { Ok(o) => { *elem = o; } Err(e) => { - return Err(e.append(i, ErrorKind::Many)); + return Err(e.append(i, &start, ErrorKind::Many)); } } } @@ -1274,7 +1285,11 @@ where //FInputXMError: handle failure properly Err(ErrMode::Backtrack(err)) => { if count < min { - return Err(ErrMode::Backtrack(err.append(input, ErrorKind::Many))); + return Err(ErrMode::Backtrack(err.append( + input, + &start, + ErrorKind::Many, + ))); } else { input.reset(&start); break; diff --git a/src/combinator/tests.rs b/src/combinator/tests.rs index 093fd432..7026cbb1 100644 --- a/src/combinator/tests.rs +++ b/src/combinator/tests.rs @@ -90,12 +90,12 @@ impl From for CustomError { } } -impl ParserError for CustomError { +impl ParserError for CustomError { fn from_error_kind(_: &I, _: ErrorKind) -> Self { CustomError } - fn append(self, _: &I, _: ErrorKind) -> Self { + fn append(self, _: &I, _: &::Checkpoint, _: ErrorKind) -> Self { CustomError } } @@ -543,12 +543,12 @@ fn alt_test() { } #[cfg(feature = "alloc")] - impl ParserError for ErrorStr { + impl ParserError for ErrorStr { fn from_error_kind(input: &I, kind: ErrorKind) -> Self { ErrorStr(format!("custom error message: ({:?}, {:?})", input, kind)) } - fn append(self, input: &I, kind: ErrorKind) -> Self { + fn append(self, input: &I, _: &::Checkpoint, kind: ErrorKind) -> Self { ErrorStr(format!( "custom error message: ({:?}, {:?}) - {:?}", input, kind, self @@ -1184,11 +1184,11 @@ impl From<(I, ErrorKind)> for NilError { } } -impl ParserError for NilError { +impl ParserError for NilError { fn from_error_kind(_: &I, _: ErrorKind) -> NilError { NilError } - fn append(self, _: &I, _: ErrorKind) -> NilError { + fn append(self, _: &I, _: &::Checkpoint, _: ErrorKind) -> NilError { NilError } } diff --git a/src/error.rs b/src/error.rs index b8e1b0bc..d2956126 100644 --- a/src/error.rs +++ b/src/error.rs @@ -178,7 +178,7 @@ impl ErrMode { } } -impl> ParserError for ErrMode { +impl> ParserError for ErrMode { #[inline(always)] fn from_error_kind(input: &I, kind: ErrorKind) -> Self { ErrMode::Backtrack(E::from_error_kind(input, kind)) @@ -194,9 +194,9 @@ impl> ParserError for ErrMode { } #[inline] - fn append(self, input: &I, kind: ErrorKind) -> Self { + fn append(self, input: &I, token_start: &::Checkpoint, kind: ErrorKind) -> Self { match self { - ErrMode::Backtrack(e) => ErrMode::Backtrack(e.append(input, kind)), + ErrMode::Backtrack(e) => ErrMode::Backtrack(e.append(input, token_start, kind)), e => e, } } @@ -267,7 +267,7 @@ where /// /// It provides methods to create an error from some combinators, /// and combine existing errors in combinators like `alt`. -pub trait ParserError: Sized { +pub trait ParserError: Sized { /// Creates an error from the input position and an [`ErrorKind`] fn from_error_kind(input: &I, kind: ErrorKind) -> Self; @@ -287,7 +287,7 @@ pub trait ParserError: Sized { /// /// This is useful when backtracking through a parse tree, accumulating error context on the /// way. - fn append(self, input: &I, kind: ErrorKind) -> Self; + fn append(self, input: &I, token_start: &::Checkpoint, kind: ErrorKind) -> Self; /// Combines errors from two different parse branches. /// @@ -387,7 +387,7 @@ where } } -impl ParserError for InputError { +impl ParserError for InputError { #[inline] fn from_error_kind(input: &I, kind: ErrorKind) -> Self { Self { @@ -397,7 +397,12 @@ impl ParserError for InputError { } #[inline] - fn append(self, _: &I, _: ErrorKind) -> Self { + fn append( + self, + _input: &I, + _token_start: &::Checkpoint, + _kind: ErrorKind, + ) -> Self { self } } @@ -466,12 +471,18 @@ impl std::error::E { } -impl ParserError for () { +impl ParserError for () { #[inline] fn from_error_kind(_: &I, _: ErrorKind) -> Self {} #[inline] - fn append(self, _: &I, _: ErrorKind) -> Self {} + fn append( + self, + _input: &I, + _token_start: &::Checkpoint, + _kind: ErrorKind, + ) -> Self { + } } impl AddContext for () {} @@ -552,14 +563,19 @@ impl Default for ContextError { } } -impl ParserError for ContextError { +impl ParserError for ContextError { #[inline] fn from_error_kind(_input: &I, _kind: ErrorKind) -> Self { Self::new() } #[inline] - fn append(self, _input: &I, _kind: ErrorKind) -> Self { + fn append( + self, + _input: &I, + _token_start: &::Checkpoint, + _kind: ErrorKind, + ) -> Self { self } @@ -882,9 +898,11 @@ where }) } - fn append(self, input: &I, kind: ErrorKind) -> Self { + fn append(self, input: &I, token_start: &::Checkpoint, kind: ErrorKind) -> Self { + let mut input = input.clone(); + input.reset(token_start); let frame = TreeErrorFrame::Kind(TreeErrorBase { - input: input.clone(), + input, kind, cause: None, }); @@ -1089,14 +1107,19 @@ impl ErrorKind { } } -impl ParserError for ErrorKind { +impl ParserError for ErrorKind { #[inline] fn from_error_kind(_input: &I, kind: ErrorKind) -> Self { kind } #[inline] - fn append(self, _: &I, _: ErrorKind) -> Self { + fn append( + self, + _input: &I, + _token_start: &::Checkpoint, + _kind: ErrorKind, + ) -> Self { self } } @@ -1370,6 +1393,7 @@ macro_rules! error_position( #[cfg(test)] macro_rules! error_node_position( ($input:expr, $code:expr, $next:expr) => ({ - $crate::error::ParserError::append($next, $input, $code) + let start = $input.checkpoint(); + $crate::error::ParserError::append($next, $input, &start, $code) }); ); diff --git a/src/parser.rs b/src/parser.rs index e3a1d305..22c0839c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -956,7 +956,7 @@ where } } -impl> Parser for () { +impl> Parser for () { #[inline(always)] fn parse_next(&mut self, _i: &mut I) -> PResult<(), E> { Ok(()) @@ -966,7 +966,7 @@ impl> Parser for () { macro_rules! impl_parser_for_tuple { ($($parser:ident $output:ident),+) => ( #[allow(non_snake_case)] - impl, $($parser),+> Parser for ($($parser),+,) + impl, $($parser),+> Parser for ($($parser),+,) where $($parser: Parser),+ { diff --git a/tests/testsuite/custom_errors.rs b/tests/testsuite/custom_errors.rs index 63063ffd..d68dab0a 100644 --- a/tests/testsuite/custom_errors.rs +++ b/tests/testsuite/custom_errors.rs @@ -6,6 +6,7 @@ use winnow::combinator::repeat; use winnow::combinator::terminated; use winnow::error::{ErrorKind, ParserError}; use winnow::prelude::*; +use winnow::stream::Stream; use winnow::unpeek; use winnow::IResult; use winnow::Partial; @@ -24,7 +25,12 @@ impl<'a> ParserError> for CustomError { CustomError(format!("error code was: {:?}", kind)) } - fn append(self, _: &Partial<&'a str>, kind: ErrorKind) -> Self { + fn append( + self, + _: &Partial<&'a str>, + _: & as Stream>::Checkpoint, + kind: ErrorKind, + ) -> Self { CustomError(format!("{:?}\nerror code was: {:?}", self, kind)) } }