float, float_s, double and double_s can not recognize numbers without dots #437

Closed
c410-f3r opened this Issue Feb 7, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@c410-f3r

c410-f3r commented Feb 7, 2017

Current implementation does not recognize numbers like 123e-10 or 123.
I am also closing #435 since this matter needs more discussion.

@federicomenaquintero

This comment has been minimized.

Show comment
Hide comment
@federicomenaquintero

federicomenaquintero Feb 21, 2017

I just ran into this for librsvg. I added some tests:

  #[test]
  fn float_test() {
    assert_eq!(float(&b""[..]),            Incomplete(Needed::Size(1)));

    assert_eq!(float(&b"0"[..]),           Done(&b""[..], 0.0));
    assert_eq!(float_s(&"0"[..]),          Done(&""[..], 0.0));

    assert_eq!(float(&b"-"[..]),           Incomplete(Needed::Unknown));
    assert_eq!(float_s(&"-"[..]),          Incomplete(Needed::Unknown));

    assert_eq!(float(&b"-0"[..]),          Done(&b""[..], 0.0));
    assert_eq!(float_s(&"-1"[..]),         Done(&""[..], -1.0));

    assert_eq!(float(&b"123"[..]),         Done(&b""[..], 123.0));
    assert_eq!(float_s(&"123"[..]),        Done(&""[..], 123.0));

    assert_eq!(float(&b"-123"[..]),        Done(&b""[..], -123.0));
    assert_eq!(float_s(&"-123"[..]),       Done(&""[..], -123.0));

    assert_eq!(float(&b"+3."[..]),         Done(&b""[..], 3.0));
    assert_eq!(float_s(&"+3."[..]),        Done(&""[..], 3.0));

    assert_eq!(float(&b"+3.14"[..]),       Done(&b""[..], 3.14));
    assert_eq!(float_s(&"3.14"[..]),       Done(&""[..], 3.14));

    assert_eq!(float(&b".5"[..]),          Done(&b""[..], 0.5));
    assert_eq!(float_s(&".5"[..]),         Done(&""[..], 0.5));

    assert_eq!(float(&b"-.5"[..]),         Done(&b""[..], -0.5));
    assert_eq!(float_s(&"-.5"[..]),        Done(&""[..], -0.5));

    assert_eq!(float(&b"-1.234E12"[..]),   Done(&b""[..], -1.234E12));
    assert_eq!(float_s(&"-1.234E12"[..]),  Done(&""[..], -1.234E12));

    assert_eq!(float(&b"-1.234E-12"[..]),  Done(&b""[..], -1.234E-12));
    assert_eq!(float_s(&"-1.234E-12"[..]), Done(&""[..], -1.234E-12));
  }

and then tried what seemed to me like the obvious fix:

@@ -603,7 +603,8 @@ pub fn float(input: &[u8]) -> IResult<&[u8],f32> {
       tuple!(
         opt!(alt!(tag!("+") | tag!("-"))),
         alt!(
-          delimited!(digit, tag!("."), opt!(digit))
+          digit
+          | delimited!(digit, tag!("."), opt!(digit))
           | delimited!(opt!(digit), tag!("."), digit)
         ),
         opt!(complete!(tuple!(

but that doesn't work - it stops parsing at the dot:

	thread 'nom::tests::float_test' panicked at 'assertion failed: `(left == right)` (left: `Done([46], 3)`, right: `Done([], 3)`)', nom.rs:1156

That is the test for "+3.".

I just ran into this for librsvg. I added some tests:

  #[test]
  fn float_test() {
    assert_eq!(float(&b""[..]),            Incomplete(Needed::Size(1)));

    assert_eq!(float(&b"0"[..]),           Done(&b""[..], 0.0));
    assert_eq!(float_s(&"0"[..]),          Done(&""[..], 0.0));

    assert_eq!(float(&b"-"[..]),           Incomplete(Needed::Unknown));
    assert_eq!(float_s(&"-"[..]),          Incomplete(Needed::Unknown));

    assert_eq!(float(&b"-0"[..]),          Done(&b""[..], 0.0));
    assert_eq!(float_s(&"-1"[..]),         Done(&""[..], -1.0));

    assert_eq!(float(&b"123"[..]),         Done(&b""[..], 123.0));
    assert_eq!(float_s(&"123"[..]),        Done(&""[..], 123.0));

    assert_eq!(float(&b"-123"[..]),        Done(&b""[..], -123.0));
    assert_eq!(float_s(&"-123"[..]),       Done(&""[..], -123.0));

    assert_eq!(float(&b"+3."[..]),         Done(&b""[..], 3.0));
    assert_eq!(float_s(&"+3."[..]),        Done(&""[..], 3.0));

    assert_eq!(float(&b"+3.14"[..]),       Done(&b""[..], 3.14));
    assert_eq!(float_s(&"3.14"[..]),       Done(&""[..], 3.14));

    assert_eq!(float(&b".5"[..]),          Done(&b""[..], 0.5));
    assert_eq!(float_s(&".5"[..]),         Done(&""[..], 0.5));

    assert_eq!(float(&b"-.5"[..]),         Done(&b""[..], -0.5));
    assert_eq!(float_s(&"-.5"[..]),        Done(&""[..], -0.5));

    assert_eq!(float(&b"-1.234E12"[..]),   Done(&b""[..], -1.234E12));
    assert_eq!(float_s(&"-1.234E12"[..]),  Done(&""[..], -1.234E12));

    assert_eq!(float(&b"-1.234E-12"[..]),  Done(&b""[..], -1.234E-12));
    assert_eq!(float_s(&"-1.234E-12"[..]), Done(&""[..], -1.234E-12));
  }

and then tried what seemed to me like the obvious fix:

@@ -603,7 +603,8 @@ pub fn float(input: &[u8]) -> IResult<&[u8],f32> {
       tuple!(
         opt!(alt!(tag!("+") | tag!("-"))),
         alt!(
-          delimited!(digit, tag!("."), opt!(digit))
+          digit
+          | delimited!(digit, tag!("."), opt!(digit))
           | delimited!(opt!(digit), tag!("."), digit)
         ),
         opt!(complete!(tuple!(

but that doesn't work - it stops parsing at the dot:

	thread 'nom::tests::float_test' panicked at 'assertion failed: `(left == right)` (left: `Done([46], 3)`, right: `Done([], 3)`)', nom.rs:1156

That is the test for "+3.".

federicomenaquintero added a commit to federicomenaquintero/nom that referenced this issue Feb 22, 2017

float() / float_s() / double() / double_s(): Accept numbers without d…
…ecimals


Fixes Geal#437

We add more detailed tests, and add support for floating-point numbers
that are written without decimals.
@federicomenaquintero

This comment has been minimized.

Show comment
Hide comment
@federicomenaquintero

federicomenaquintero Feb 22, 2017

I've just submitted a pull request that fixes this, along with better tests: #448

I've just submitted a pull request that fixes this, along with better tests: #448

@Geal Geal added this to the 3.3 milestone Sep 6, 2017

@Geal Geal modified the milestones: 3.3, 4.0 Nov 26, 2017

@veprbl

This comment has been minimized.

Show comment
Hide comment
@veprbl

veprbl Dec 29, 2017

Forgot to google the issue. I made a PR with a (slightly) different approach where I abandon the complete!: #646

veprbl commented Dec 29, 2017

Forgot to google the issue. I made a PR with a (slightly) different approach where I abandon the complete!: #646

@Geal

This comment has been minimized.

Show comment
Hide comment
@Geal

Geal Jan 13, 2018

Owner

ok, so, we can restart the discussion around floats, since nom 4 simplifies some things. It introduces the AtEof trait to indicate whether the input can get more data or not, and the CompleteStr and CompleteByteSlice input types, that always indicate there's no more data.
So the float numbers parsers could be rewritten without the complete!() calls that mess up things.

Owner

Geal commented Jan 13, 2018

ok, so, we can restart the discussion around floats, since nom 4 simplifies some things. It introduces the AtEof trait to indicate whether the input can get more data or not, and the CompleteStr and CompleteByteSlice input types, that always indicate there's no more data.
So the float numbers parsers could be rewritten without the complete!() calls that mess up things.

@Geal

This comment has been minimized.

Show comment
Hide comment
@Geal

Geal Jan 14, 2018

Owner

So, this should be fixed by 99e4cea now
Additionally, the code is a bit easier to modify, since I have refactored the float recognizer in one parser that is called by the float/double functions

Owner

Geal commented Jan 14, 2018

So, this should be fixed by 99e4cea now
Additionally, the code is a bit easier to modify, since I have refactored the float recognizer in one parser that is called by the float/double functions

@Geal Geal added the needs testing label Jan 14, 2018

@c410-f3r

This comment has been minimized.

Show comment
Hide comment

Thanks @Geal !

@c410-f3r c410-f3r closed this Jan 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment