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

nom is not compatible with pub(crate) and the unreachable-pub lint #807

Closed
marmistrz opened this issue Jun 28, 2018 · 3 comments
Closed

Comments

@marmistrz
Copy link

Prerequisites

Here are a few things you should provide to help me understand the issue:

  • Rust version : rustc -V: rustc 1.28.0-nightly (cd494c1f0 2018-06-27)
  • nom version : 4.0.0
  • nom compilation features used: default

Test case

This is the initial code. It triggers the rustc warning about unreachable-pub items.
nom-incompat.zip

The compiler suggests to restrict its visibility, by using pub(crate). After changing the line

named!(pub par, tag!("xD"));

to

named!(pub(crate) par, tag!("xD"));

The example doesn't build anymore

error: no rules expected the token `par`
 --> src/protocol/parser.rs:3:19
  |
3 | named!(pub(crate) par, tag!("xD"));
  |                   ^^^

error: aborting due to previous error
@Zarenor
Copy link

Zarenor commented Jun 28, 2018

To me, this looks like an issue with the way named! parses function names. It's probably not capable of handling pub(crate). However, since you're on 1.28 nightly, I think you can instead use the 'crate' access modifier - it's equivalent.

Perhaps Geal has more insight, however.

kodawah pushed a commit to hedgewars/hw that referenced this issue Jul 2, 2018
It appears that nom cannot correctly handle `pub(crate)`,
so the warnings cannot be fixed at the moment.
See rust-bakery/nom#807
@agentsim
Copy link

The 'crate' access modifier doesn't work either. These two snippets produce the same error (on a current nightly rustc):

named!(pub(crate) par, tag!("xD"));

named!(crate par, tag!("xD"));

@Geal
Copy link
Collaborator

Geal commented Oct 6, 2018

#566 was merged, this will be available in the next nom version

@Geal Geal closed this as completed Oct 6, 2018
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

No branches or pull requests

4 participants