Bug 127667 - Decimal::fromString's EBNF documentation does not match implementation
Summary: Decimal::fromString's EBNF documentation does not match implementation
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2014-01-26 19:02 PST by Joseph Pecoraro
Modified: 2014-01-27 10:18 PST (History)
2 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2014-01-26 19:02:22 PST
Current documentation for Decimal::fromString says:

    // fromString supports following syntax EBNF:
    //  number ::= sign? digit+ ('.' digit*) (exponent-marker sign? digit+)?
    //          | sign? '.' digit+ (exponent-marker sign? digit+)?
    //  sign ::= '+' | '-'
    //  exponent-marker ::= 'e' | 'E'
    //  digit ::= '0' | '1' | ... | '9'
    // Note: fromString doesn't support "infinity" and "nan".
    static Decimal fromString(const String&);

However in the implementation:

    1. It does not look possible to have "sign? '.'" at all.
    Test: "-.1"

    2. The documentation says "sign? '.' digit+" but it looks like "sign? '.' digit*" is possible. If so what happens? What should happen?
    Test: ".e1"

What should this code actually be doing? Where did that EBNF come from?

There used to be a chromium test for this. We should consider adding a test for this and testing more cases.
Comment 1 Joseph Pecoraro 2014-01-26 19:15:07 PST
NOTE: <http://www.w3.org/TR/2011/WD-html5-20110525/common-microsyntaxes.html#real-numbers>

Sounds like: "sign? digit+ ('.' digit+ )? (('e'|'E') sign? digit+)?"

Because it always says "A series of one or more characters in the range".

If that is the case, then the implicit fallthrough in the current implementation is wrong, and the documentation should not mention "digit*" anywhere. Also, where did the idea to support strings without a number before a '.' come from? Was it an older version of the spec? Should we continue to support that syntax?

So many questions!