Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,21 @@ impl<I, C, E: AddContext<I, C>> AddContext<I, C> for ErrMode<E> {
}
}

impl<E: MergeContext> MergeContext for ErrMode<E> {
#[inline(always)]
fn merge_context(self, other: Self) -> Self {
match other.into_inner() {
Some(other) => self.map(|err| err.merge_context(other)),
None => self,
}
}

#[inline]
fn clear_context(self) -> Self {
self.map(MergeContext::clear_context)
}
}

impl<T: Clone> ErrMode<InputError<T>> {
/// Maps `ErrMode<InputError<T>>` to `ErrMode<InputError<U>>` with the given `F: T -> U`
pub fn map_input<U: Clone, F>(self, f: F) -> ErrMode<InputError<U>>
Expand Down Expand Up @@ -313,6 +328,15 @@ pub trait AddContext<I, C = &'static str>: Sized {
}
}

/// Merge contexts while backtracking.
pub trait MergeContext: Sized {
/// Apply the context from `other` into `self`
fn merge_context(self, _other: Self) -> Self;

/// Remove all context
fn clear_context(self) -> Self;
}

/// Create a new error with an external error, from [`std::str::FromStr`]
///
/// This trait is required by the [`Parser::try_map`] combinator.
Expand Down Expand Up @@ -536,6 +560,32 @@ impl<C, I> AddContext<I, C> for ContextError<C> {
}
}

impl<C> MergeContext for ContextError<C> {
#[inline]
fn merge_context(mut self, other: Self) -> Self {
// self and other get consumed to produce the new Context error.
// We choose the vector with the larger capacity to reduce the chance of reallocations.
#[cfg(feature = "alloc")]
{
let (mut context, other) = if self.context.capacity() >= other.context.capacity() {
(self.context, other.context)
} else {
(other.context, self.context)
};
context.extend(other);
Comment on lines +574 to +579
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this could lead to confusing ordering.

Should we instead check if one is empty and instead pick the other?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really care about order if there are multiple parsers with the same longest match, but I see how this might be confusing to developers or mess eventually mess with tests when they change parsers..

Should we instead check if one is empty and instead pick the other?

👍

self.context = context;
}
self
}

#[inline]
fn clear_context(mut self) -> Self {
#[cfg(feature = "alloc")]
self.context.clear();
self
}
}

#[cfg(feature = "std")]
impl<C, I, E: std::error::Error + Send + Sync + 'static> FromExternalError<I, E>
for ContextError<C>
Expand Down
41 changes: 41 additions & 0 deletions src/stream/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,31 @@ where
}
}

/// Used to compare checkpoints
pub trait AsOrd {
/// The type used to compare checkpoint positions
type Ord: Ord + Clone + core::cmp::Ord + crate::lib::std::fmt::Debug;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Ord + core::cmp::Ord redundant?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, some oversight when combining the commits into one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Clone and Debug, if this is because LongestMatch impls those traits, wouldn't that be dependent on whether the Ord does so, meaning we don't need these bounds?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove them. Clone was required because I initially stored the ord in LongestMatch. I updated LongestMatch to store the actual checkpoint now.


/// Get comparable value
fn as_ord(&self) -> Self::Ord;
}

impl<'a, T> AsOrd for &'a [T] {
type Ord = *const T;

fn as_ord(&self) -> Self::Ord {
self.as_ptr()
}
}

impl<'a> AsOrd for &'a str {
type Ord = *const u8;

fn as_ord(&self) -> Self::Ord {
self.as_ptr()
}
}

/// Helper trait for types that can be viewed as a byte slice
pub trait AsBytes {
/// Casts the input type to a byte slice
Expand Down Expand Up @@ -2200,6 +2225,22 @@ where
#[derive(Copy, Clone, Debug)]
pub struct Checkpoint<T>(T);

impl<T: PartialEq> PartialEq for Checkpoint<T> {
fn eq(&self, other: &Self) -> bool {
self.0.eq(&other.0)
}
}

impl<T: Eq> Eq for Checkpoint<T> {}
Comment on lines +2228 to +2234
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need these?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are currently used by the tests. We can update the tests to compare the err.into_inner() or compare via format!(...).


impl<T: AsOrd> AsOrd for Checkpoint<T> {
type Ord = T::Ord;

fn as_ord(&self) -> Self::Ord {
self.0.as_ord()
}
}

/// A range bounded inclusively for counting parses performed
#[derive(PartialEq, Eq)]
pub struct Range {
Expand Down