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

RFC: more robust E notation parser in BigDecimal #9580

Open
stevegeek opened this issue Jul 6, 2020 · 4 comments
Open

RFC: more robust E notation parser in BigDecimal #9580

stevegeek opened this issue Jul 6, 2020 · 4 comments

Comments

@stevegeek
Copy link
Contributor

stevegeek commented Jul 6, 2020

Related to #9547

Maybe a more robust parser of the scientific or E notation inputs in BigDecimal is worth the effort? Eg I started writing up this EBNF-ish grammar below:

<scientific> :== [<sign>] <number>
<number> :== <digits> [ <fractional-optionaldigits> ] [ <exponent> ]   |  <fractional> [ <exponent> ]
<fractional-optionaldigits> :== '.' [ <digits> ]
<fractional> :== '.' <digits>
<exponent> :== <exp> [ <sign> ] <digits>
<digits> :== <digit>+
<digit> :== '0' | '1' ... | '9'
<exp> :== 'E' | 'e'
<sign> :== '+' | '-'

then created the productions below (syntax below works in http://hackingoff.com/compilers/ll-1-parser-generator)

Scientific -> OptionalSign Number
OptionalSign -> s |
Number -> Digits OptionalFractional OptionalExponent | Fractional OptionalExponent
OptionalFractional -> '.' OptionalDigits  
Fractional -> '.' Digits
OptionalExponent -> e OptionalSign Digits | 
OptionalDigits -> Digits |  
Digits -> Digit RepeatDigit
RepeatDigit -> Digit |  
Digit -> n 

Here is a proposal that implements a first pass at this (not stack based to avoid needing a stack, just a translation of the parse table into logic):

PR which is still a work in progress and would benefit from your inputs should we wish to progress with this RFC: #9581

I also note overflowing exponent values will raise Invalid UInt64 exceptions (e.g BigDecimal.new("1e18446744073709551616")). Maybe this is an implementation detail which could be more neatly wrapped in a ArgumentError/InvalidBigDecimalException on parsing?

(originally part of #9547)

@asterite
Copy link
Member

asterite commented Jul 6, 2020

@stevegeek Please send a PR with the changes. Otherwise it's impossible to review.

@asterite
Copy link
Member

asterite commented Jul 6, 2020

Oh, nevermind, I though this was it. I see there's a PR...

@asterite
Copy link
Member

asterite commented Jul 6, 2020

I was confused because you mentioned master...stevegeek:feature_e_notation_big_decimal_parser so I thought we had to look into your fork.

@stevegeek
Copy link
Contributor Author

Sorry about that was a copy paste from the previous issue I opened!

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

3 participants