127667 2014-01-26 19:02:22 -0800 Decimal::fromString's EBNF documentation does not match implementation 2023-06-22 18:31:09 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=258418 P2 Normal --- 1 joepeck webkit-unassigned ahmad.saleem792 annevk ap joepeck oldest_to_newest 972299 0 joepeck 2014-01-26 19:02:22 -0800 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. 972304 1 joepeck 2014-01-26 19:15:07 -0800 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! 1910827 2 ahmad.saleem792 2022-11-07 11:04:59 -0800 This FIXME is still present: https://github.com/WebKit/WebKit/blob/06829a72816b3507674af7be8a740a4964ae3518/Source/WebCore/platform/Decimal.cpp#L759