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

Update breaks 11-bit parser- always Incomplete #768

Closed
leshow opened this issue May 16, 2018 · 6 comments
Closed

Update breaks 11-bit parser- always Incomplete #768

leshow opened this issue May 16, 2018 · 6 comments
Milestone

Comments

@leshow
Copy link

leshow commented May 16, 2018

Prerequisites

I saw your blog post today about the new version, congrats! I have a small bit of nom from a while ago I'm trying to upgrade but it appears the behaviour is different.

In your 'upgrading to 4' design doc, you mentioned that 4 is stricter with 'Incomplete' however in this case the data is already here, I am not waiting on anything (in the real program the Vec isn't pasted in but already allocated, the result is the same). I've made the appropriate syntax changes.

  • Rust version : rustc -V 1.27
  • nom version : 1.2 works 4 doesn't
  • nom compilation features used:
    macros

Test case

working (will print an idx for every 11 bits in the Vec):

#[macro_use]
extern crate nom;
use nom::IResult;
fn main() {
        named!(bit_vec<Vec<u16>>, bits!(many0!(take_bits!(u16, 11))));

        let m: Vec<u8> = vec![70, 97, 106, 121, 86, 66, 105, 98, 86, 106, 101, 72, 57, 101, 52, 118, 73, 72, 80, 115, 83, 71, 52, 78, 104, 67, 56, 122, 49, 98, 76, 53, 141];
        let mut mnem_words = Vec::new();
        if let IResult::Done(_, bit_sequence) = bit_vec(m.as_slice()) {
            for idx in &bit_sequence {
                 println!("{:?}", idx); 
            }
        }
}

not working, says Incomplete, will not print any idx values:

#[macro_use]
extern crate nom;
use nom::IResult;
fn main() {
        named!(bit_vec<Vec<u16>>, bits!(many0!(take_bits!(u16, 11))));

        let m: Vec<u8> = vec![70, 97, 106, 121, 86, 66, 105, 98, 86, 106, 101, 72, 57, 101, 52, 118, 73, 72, 80, 115, 83, 71, 52, 78, 104, 67, 56, 122, 49, 98, 76, 53, 141];
        let mut mnem_words = Vec::new();
        if let Ok((_, bit_sequence)) = bit_vec(m.as_slice()) {
            for idx in &bit_sequence {
                 println!("{:?}", idx); 
            }
        }
}
@leshow leshow changed the title Update breaks parser Update breaks 11-bit parser- always Incomplete May 16, 2018
@Geal
Copy link
Collaborator

Geal commented May 19, 2018

oh, thanks for catching this one. Not sure I'll be able to fix it right now, but it shouldn't be too hard :)

@khernyo
Copy link
Contributor

khernyo commented May 23, 2018

It seems to me that this is the expected behavior: https://github.com/Geal/nom/blob/6000cce44d044ddc2a90d2adbe34fa932d7ceeed/src/multi.rs#L1219-L1231

With a bit more digging this is what I found:

  • many0! returns Ok in two cases only:
    1. the sub-parser returns an Error (not a Failure or Incomplete)
    2. the sub-parser returns Ok and input.at_eof() is true
  • take_bits! only returns Error if input.at_eof() is true
  • input.at_eof() is always false for a slice

So, you either need to replace take_bits! with a parser which rejects some input (e.g. a delimiter), or you need to use an input for which at_eof() is true when appropriate (e.g. CompleteByteSlice).

Unfortunately, take_bits! only takes &[u8] as input, so the latter solution won't work in the current version.

If the above is a correct summary, I can take a crack at generalizing the bit parsers to handle CompleteByteSlice and other types.

khernyo added a commit to khernyo/nom that referenced this issue May 25, 2018
khernyo added a commit to khernyo/nom that referenced this issue May 25, 2018
@leshow
Copy link
Author

leshow commented May 31, 2018

Apologies for taking so long to get to this I missed the notification. If I understand what you're saying, there's currently no way to slice a byte (or bytes) into an n-bit iterator?

@khernyo
Copy link
Contributor

khernyo commented May 31, 2018

As far as I know, there is no way.

@leshow
Copy link
Author

leshow commented Jun 1, 2018

Thank you for your PR, I've gone back to and tested in nom 1, 2 and 3 and my code parses properly in 1&2, but not 3 or 4. So it appears to be a regression introduced in nom 3

@leshow
Copy link
Author

leshow commented Jul 26, 2018

Any movement on this?

@Geal Geal closed this as completed in c6ff198 Mar 4, 2019
Geal added a commit that referenced this issue Mar 4, 2019
Fixes #768: Generalize bits.rs to handle inputs other than &[u8]
@Geal Geal added this to the 5.0 milestone Jun 17, 2019
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

3 participants