Summary: | ES6: Add binary and octal literal support | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, cmarcelo, commit-queue, joepeck | ||||
Priority: | P2 | ||||||
Version: | 312.x | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Michael Saboff
2015-03-13 16:02:05 PDT
Created attachment 248627 [details]
Patch
Comment on attachment 248627 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=248627&action=review Looks good to me! Added a few suggestions for more tests. > Source/JavaScriptCore/parser/Lexer.cpp:1754 > + m_lexErrorMessage = ASCIILiteral("No space between binary literal and identifier"); I don't see the error message below. We should add a test for these error cases. 0b1myVariable => SyntaxError: No space between binary literal and identifier 0o1myVariable => SyntaxError: No space between octal literal and identifier > Source/WTF/ChangeLog:12 > +2015-03-13 Michael Saboff <msaboff@apple.com> Oops, double changelog. > LayoutTests/js/octal-literals-expected.txt:19 > +PASS 0o777777777777777777 is 18014398509481984 I think we should probably test the extreme case as well: For example "0o100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" being some very large exponent, and then eventually just "Infinity". Absurd for binary literals but still can be tested. Comment on attachment 248627 [details]
Patch
r=me although you probably want to address Joe's comments.
(In reply to comment #2) > Comment on attachment 248627 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=248627&action=review > > Looks good to me! Added a few suggestions for more tests. > > > Source/JavaScriptCore/parser/Lexer.cpp:1754 > > + m_lexErrorMessage = ASCIILiteral("No space between binary literal and identifier"); > > I don't see the error message below. We should add a test for these error > cases. > > 0b1myVariable => SyntaxError: No space between binary literal and > identifier > 0o1myVariable => SyntaxError: No space between octal literal and > identifier Added. > > Source/WTF/ChangeLog:12 > > +2015-03-13 Michael Saboff <msaboff@apple.com> > > Oops, double changelog. Fixed. > > LayoutTests/js/octal-literals-expected.txt:19 > > +PASS 0o777777777777777777 is 18014398509481984 > > I think we should probably test the extreme case as well: > > For example > "0o10000000000000000000000000000000000000000000000000000000000000000000000000 > 00000000000000000000000000000000000000000000000000000000000000000000000000000 > 00000000000000000000000000000000000000000000000000000000000000000000000000000 > 000000000000000000000000000000000000000000000000000000000" being some very > large exponent, and then eventually just "Infinity". Absurd for binary > literals but still can be tested. Added. Committed r181497: <http://trac.webkit.org/changeset/181497> |