Bug 199984

Summary: Legacy numeric literals should not permit separators or BigInt
Product: WebKit Reporter: Ross Kirsling <ross.kirsling>
Component: New BugsAssignee: Ross Kirsling <ross.kirsling>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, keith_miller, leo, mark.lam, msaboff, saam, ticaiolima, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=196351
https://bugs.webkit.org/show_bug.cgi?id=179000
https://bugs.webkit.org/show_bug.cgi?id=200244
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Ross Kirsling 2019-07-20 18:42:55 PDT
Legacy numeric literals should not permit separators or BigInt
Comment 1 Ross Kirsling 2019-07-20 18:43:30 PDT
Created attachment 374564 [details]
Patch
Comment 2 Ross Kirsling 2019-07-20 18:47:21 PDT
An easily-overlooked piece of the spec -- Test262 cases were added for separators yesterday (https://github.com/tc39/test262/pull/2245) but I think they're still needed for BigInt.
Comment 3 Ross Kirsling 2019-07-22 13:27:00 PDT
(In reply to Ross Kirsling from comment #2)
> An easily-overlooked piece of the spec -- Test262 cases were added for
> separators yesterday (https://github.com/tc39/test262/pull/2245) but I think
> they're still needed for BigInt.

The rest of the test cases are in https://github.com/tc39/test262/pull/2253 (with additional BigInt + separator ones in https://github.com/tc39/test262/pull/2252).

One way or another, the key is that neither proposal is meant to add to Annex B.1.1's LegacyOctalIntegerLiteral and NonOctalDecimalIntegerLiteral.
Comment 4 Caio Lima 2019-07-23 14:25:14 PDT
Comment on attachment 374564 [details]
Patch

LGTM
Comment 5 Keith Miller 2019-07-25 10:37:40 PDT
Comment on attachment 374564 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374564&action=review

r=me.

> Source/JavaScriptCore/parser/Lexer.cpp:1628
> +    bool isLegacyLiteral = m_buffer8.size();

For clarity can you add:

ASSERT(!m_buffer8.size() || (m_buffer8.size() == 1 && m_buffer[0] == '0'));
Comment 6 Keith Miller 2019-07-25 10:38:52 PDT
Comment on attachment 374564 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=374564&action=review

> JSTests/stress/numeric-literal-separators.js:75
> +shouldThrow('010000000000_1', SyntaxError);
> +shouldThrow('010000000000_9', SyntaxError);
> +shouldThrow('090000000000_1', SyntaxError);

Can you add tests for 0100_004 and 0_000 etc?
Comment 7 Ross Kirsling 2019-07-25 13:19:59 PDT
Created attachment 374900 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2019-07-25 16:55:51 PDT
Comment on attachment 374900 [details]
Patch for landing

Clearing flags on attachment: 374900

Committed r247845: <https://trac.webkit.org/changeset/247845>
Comment 9 WebKit Commit Bot 2019-07-25 16:55:53 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-07-25 16:58:30 PDT
<rdar://problem/53566528>