jethrogb / rust-cexpr

A C expression parser and evaluator
Apache License 2.0
45 stars 19 forks source link

upgrade to nom 3.0 #5

Closed Geal closed 7 years ago

Geal commented 7 years ago

Hello!

Here's a free upgrade to nom 3.0 :)

I couldn't run the tests, could you check that everything is alright? The parsers should be functionally equivalent to the previous ones.

The parsers should be slightly faster now, too, since there are performance improvements in error handling in nom 3.

jethrogb commented 7 years ago

Thanks!

You've changed several of the operators such that matching happens twice: once as part of alt! and once as part of fold_many0!. Is it no longer possible to write it with a single comparison?

Geal commented 7 years ago

it might be possible with a custom combinator. The complex part here was the use of mut, which was removed in do_parse. But I think we can get something faster by removing things like alt!( pair!(p!("*"), call_m!(self.unary)) | pair!(p!("/"), call_m!(self.unary)) | pair!(p!("%"), call_m!(self.unary)) ) and replacing it with pair!(one_of!("*/%"), call_m!(self.unary)) and they match in the accumulating closure. This would simplify the code, and might even be faster than the alt! based version (since there would be no backtracking.

jethrogb commented 7 years ago
running 6 tests
test int_signed ... FAILED
test fail ... ok
test floats ... FAILED
test chars ... ok
test strings ... FAILED
test int_unsigned ... FAILED

failures:

---- int_signed stdout ----
    Failed test for Int_n3, expected Int(-3), got Incomplete(Size(9))
Failed test for Int_n5, expected Int(-5), got Incomplete(Size(5))
thread 'int_signed' panicked at 'test_file', tests/clang.rs:170
note: Run with `RUST_BACKTRACE=1` for a backtrace.

---- floats stdout ----
    Failed test for Float_80, expected Float(80), got Incomplete(Size(8))
thread 'floats' panicked at 'test_file', tests/clang.rs:167

---- strings stdout ----
    Failed test for Str_concat_parens, expected Str([99, 111, 110, 99, 97, 116, 95, 112, 97, 114, 101, 110, 115]), got Incomplete(Size(5))
Failed test for Str_concat_identifier, expected Str([99, 111, 110, 99, 97, 116, 95, 105, 100, 101, 110, 116, 105, 102, 105, 101, 114]), got Incomplete(Size(5))
thread 'strings' panicked at 'test_file', tests/clang.rs:169

---- int_unsigned stdout ----
    Failed test for Int_16, expected Int(16), got Incomplete(Size(10))
Failed test for Int_13, expected Int(13), got Incomplete(Size(10))
thread 'int_unsigned' panicked at 'test_file', tests/clang.rs:171

failures:
    floats
    int_signed
    int_unsigned
    strings
jethrogb commented 7 years ago

I think this is a bug in the input generator, not the parser

jethrogb commented 7 years ago

Tests fixed in 0ea13677

jethrogb commented 7 years ago

Made the change you suggested. Thanks for the PR!