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

cargo features (verbose-errors may be other?) should be additive #544

Closed
Dushistov opened this issue Jul 25, 2017 · 8 comments
Closed

cargo features (verbose-errors may be other?) should be additive #544

Dushistov opened this issue Jul 25, 2017 · 8 comments
Milestone

Comments

@Dushistov
Copy link

As desribed rust-lang/cargo#4323, see @kennytm comment here:

The problem here is that, when using "simple errors", the type nom::simple_errors::Err<E> is simply a type alias of nom::ErrorKind<E>, while with "verbose errors" the type nom::verbose_errors::Err<E> is a dedicated enum, so the "with-feature" and "without-feature" interfaces are incompatible.

@kennytm
Copy link

kennytm commented Jul 25, 2017

#457 😢

@Geal
Copy link
Collaborator

Geal commented Jul 26, 2017

yes, that situation is annoying, but I really don't see how to make that particular feature additive. Others (std, stream, regex*) are fine, but I don't know for this one.

@kennytm
Copy link

kennytm commented Jul 26, 2017

@Geal You could turn simple_errors::Err into an enum containing only the Code variant (preferably a non-exhaustive enum). It is fine as long as the API of simple_errors is a strict subset of verbose_errors.

Yes the fix is going to be a breaking change 😞

@Geal Geal added this to the 4.0 milestone Sep 6, 2017
@Geal
Copy link
Collaborator

Geal commented Oct 3, 2017

so this is now done as you said, with an enum. It's much cleaner to use :)

@rustonaut
Copy link

seems I have to switch to nom4, this just broke my crate 🤕 well at last I did learn something new

@leoschwarz
Copy link

This ended up biting me now too.

The docs here https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md, at least pub type Err<E=u32> = ErrorKind<E>; should be updated to reflect the change made but I'm not sure if there is more that still needs updating.

@Geal Geal closed this as completed May 14, 2018
@dtolnay
Copy link

dtolnay commented Jun 27, 2018

I don't think this has been fixed. I still see this in master:

https://github.com/Geal/nom/blob/a88a6f8d3fe3cf41ef682bbfec1fd72f08edcf95/src/lib.rs#L434-L436

That means the following code would break if verbose-errors is enabled anywhere else in our dependency graph.

extern crate nom;

use nom::simple_errors;

Some cases of this causing breakage:

@rustonaut
Copy link

rustonaut commented Jun 27, 2018 via email

akshayknarayan added a commit to ccp-project/portus that referenced this issue Sep 4, 2018
As a workaround to rust-bakery/nom#544, migrate to
nom 4 to ensure that the verbose-errors feature becomes additive and
therefore portus compiles when used as a dependency.

There were two classes of changes to the parser structure:
1. Error handling, as predicted and outlined here:
https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md#replacing-parser-result-matchers

2. Migration to "CompleteByteSlice".
This was predicted in the migration notes:
https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md#dealing-with-incomplete-usage
but caused underlying errors as reported here:
rust-bakery/nom#790 (comment)

To address this, shadow nom's `named!` macro with our own,
`named_complete!`, which replaces the types appropriately. This is the
solution proposed here:
rust-bakery/nom#795 (comment)
akshayknarayan added a commit to ccp-project/portus that referenced this issue Sep 6, 2018
As a workaround to rust-bakery/nom#544, migrate to
nom 4 to ensure that the verbose-errors feature becomes additive and
therefore portus compiles when used as a dependency.

There were two classes of changes to the parser structure:
1. Error handling, as predicted and outlined here:
https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md#replacing-parser-result-matchers

2. Migration to "CompleteByteSlice".
This was predicted in the migration notes:
https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md#dealing-with-incomplete-usage
but caused underlying errors as reported here:
rust-bakery/nom#790 (comment)

To address this, shadow nom's `named!` macro with our own,
`named_complete!`, which replaces the types appropriately. This is the
solution proposed here:
rust-bakery/nom#795 (comment)
akshayknarayan added a commit to ccp-project/portus that referenced this issue Sep 7, 2018
As a workaround to rust-bakery/nom#544, migrate to
nom 4 to ensure that the verbose-errors feature becomes additive and
therefore portus compiles when used as a dependency.

There were two classes of changes to the parser structure:
1. Error handling, as predicted and outlined here:
https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md#replacing-parser-result-matchers

2. Migration to "CompleteByteSlice".
This was predicted in the migration notes:
https://github.com/Geal/nom/blob/master/doc/upgrading_to_nom_4.md#dealing-with-incomplete-usage
but caused underlying errors as reported here:
rust-bakery/nom#790 (comment)

To address this, shadow nom's `named!` macro with our own,
`named_complete!`, which replaces the types appropriately. This is the
solution proposed here:
rust-bakery/nom#795 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants