Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement trivial Copy and ToOwned functions for Error<_> #1788

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

koshell
Copy link

@koshell koshell commented Nov 17, 2024

I've been using nom in some of my projects. I've yet to dig into using VerboseError yet but I've been using the simple Error quite a lot for &str parsing. Using it inside FromStr has been quite ergonomic.

However one annoyance I discovered was when returning an error from parsing a &str.
Doing so returns a Result<_, Error<&str> which is awkward to work with since now you need to add lifetime specifiers to everything if you want to return the error as is. Assuming the trait even has the flexibility to add lifetime specifiers.

In my case I end up rebuilding the entire Error before returning it like so:

// result = Result<_, Error<&str>>
// fn -> Result<_, Error<String>>
return result.map_err(|Error { input, code }| Error::new(input.to_owned(), code));

I thought it would be trivial to add a more ergonomic method and instead of creating a feature request I thought I would just implement it myself. I ended up adding two methods to the Error struct:

#[cfg(feature = "alloc")]
impl<I: ToOwned + ?Sized> Error<&I> {
  /// Converts `Error<&I>` into `Error<I::Owned>` by cloning.
  pub fn cloned(self) -> Error<I::Owned> { .. }
  }
}
impl<I: Copy> Error<&I> {
  /// Converts `Error<&I>` into `Error<I>` by copying.
  pub fn copied(self) -> Error<I> { .. }
}

(I implemented them for &mut as well but omitted that above for brevity.)
This would allow my earlier conversion to be shortened to:

// result = Result<_, Error<&str>>
// fn -> Result<_, Error<String>>
return result.map_err(Error::cloned);

The actual implementation is exceedingly simple, I have made some tests just to ensure they have the expected behaviour but there really isn't much to test.

I noticed the #[cfg(feature = "alloc")] attribute on the import of ToOwned and couldn't confidently work out the reasoning behind it. Therefore I carried that attribute across to the implementations that use ToOwned. Feel free to remove that attribute if the restriction is unnecessary.

@koshell koshell requested a review from Geal as a code owner November 17, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant