Bug 193050

Summary:

[JSC] Identifier validity should be based on ID_Start / ID_Continue properties

Product:

WebKit

Reporter:

Ross Kirsling <ross.kirsling>

Component:

JavaScriptCore

Assignee:

Ross Kirsling <ross.kirsling>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

blessedmonthly, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, rniwa, saam, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews100 for mac-sierra	none
Archive of layout-test-results from ews106 for mac-sierra-wk2	none
Archive of layout-test-results from ews200 for win-future	none
Archive of layout-test-results from ews112 for mac-sierra	none
Archive of layout-test-results from ews121 for ios-simulator-wk2	none
Patch	none

Ross Kirsling

Reported 2018-12-28 16:35:56 PST

[JSC] Identifier validity should be based on ID_Start / ID_Continue properties

Attachments
Patch (12.31 KB, patch) 2018-12-28 16:49 PST, Ross Kirsling	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews100 for mac-sierra (2.44 MB, application/zip) 2018-12-28 17:42 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews106 for mac-sierra-wk2 (3.12 MB, application/zip) 2018-12-28 17:57 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews200 for win-future (12.95 MB, application/zip) 2018-12-28 18:17 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews112 for mac-sierra (1.73 MB, application/zip) 2018-12-28 18:19 PST, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.46 MB, application/zip) 2018-12-28 18:38 PST, EWS Watchlist	no flags	Details
Patch (27.58 KB, patch) 2018-12-28 23:48 PST, Ross Kirsling	no flags	Details Formatted Diff Diff
Show Obsolete (6) View All Add attachment proposed patch, testcase, etc.

Ross Kirsling

Comment 1 2018-12-28 16:49:15 PST

Created attachment 358123 [details] Patch

EWS Watchlist

Comment 2 2018-12-28 17:42:54 PST Comment hidden (obsolete)

Comment on attachment 358123 [details] Patch Attachment 358123 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/10571568 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 3 2018-12-28 17:42:56 PST Comment hidden (obsolete)

Created attachment 358124 [details] Archive of layout-test-results from ews100 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-sierra Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 4 2018-12-28 17:57:22 PST Comment hidden (obsolete)

Comment on attachment 358123 [details] Patch Attachment 358123 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/10571593 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 5 2018-12-28 17:57:23 PST Comment hidden (obsolete)

Created attachment 358125 [details] Archive of layout-test-results from ews106 for mac-sierra-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 6 2018-12-28 18:17:28 PST Comment hidden (obsolete)

Comment on attachment 358123 [details] Patch Attachment 358123 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/10571689 New failing tests: sputnik/Unicode/Unicode_410/S7.6_A5.2_T6.html sputnik/Unicode/Unicode_218/S7.6_A5.3_T2.html sputnik/Unicode/Unicode_410/S7.6_A1.1_T6.html sputnik/Unicode/Unicode_218/S7.6_A5.2_T2.html sputnik/Unicode/Unicode_500/S7.6_A1.1_T6.html sputnik/Unicode/Unicode_320/S7.6_A1.1_T6.html sputnik/Unicode/Unicode_320/S7.6_A5.2_T9.html sputnik/Unicode/Unicode_218/S7.6_A5.2_T6.html sputnik/Unicode/Unicode_410/S7.6_A5.3_T2.html sputnik/Unicode/Unicode_500/S7.6_A3.2.html sputnik/Unicode/Unicode_510/S7.6_A5.2_T6.html sputnik/Unicode/Unicode_410/S7.6_A3.2.html sputnik/Unicode/Unicode_320/S7.6_A5.3_T2.html sputnik/Unicode/Unicode_510/S7.6_A5.2_T4.html sputnik/Unicode/Unicode_218/S7.6_A1.1_T2.html sputnik/Unicode/Unicode_510/S7.6_A1.1_T6.html sputnik/Unicode/Unicode_320/S7.6_A3.2.html sputnik/Unicode/Unicode_500/S7.6_A5.2_T6.html sputnik/Unicode/Unicode_510/S7.6_A1.1_T4.html sputnik/Unicode/Unicode_410/S7.6_A5.3_T1.html sputnik/Unicode/Unicode_320/S7.6_A5.2_T6.html sputnik/Unicode/Unicode_500/S7.6_A3.1.html sputnik/Unicode/Unicode_500/S7.6_A5.3_T2.html sputnik/Unicode/Unicode_320/S7.6_A5.3_T1.html sputnik/Unicode/Unicode_320/S7.6_A3.1.html sputnik/Unicode/Unicode_218/S7.6_A5.3_T1.html sputnik/Unicode/Unicode_218/S7.6_A1.1_T6.html

EWS Watchlist

Comment 7 2018-12-28 18:17:39 PST Comment hidden (obsolete)

Created attachment 358126 [details] Archive of layout-test-results from ews200 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews200 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

EWS Watchlist

Comment 8 2018-12-28 18:18:58 PST Comment hidden (obsolete)

Comment on attachment 358123 [details] Patch Attachment 358123 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/10571607 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 9 2018-12-28 18:19:00 PST Comment hidden (obsolete)

Created attachment 358127 [details] Archive of layout-test-results from ews112 for mac-sierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews112 Port: mac-sierra Platform: Mac OS X 10.12.6

EWS Watchlist

Comment 10 2018-12-28 18:38:24 PST Comment hidden (obsolete)

Comment on attachment 358123 [details] Patch Attachment 358123 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/10571668 Number of test failures exceeded the failure limit.

EWS Watchlist

Comment 11 2018-12-28 18:38:25 PST Comment hidden (obsolete)

Created attachment 358128 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.6

Ross Kirsling

Comment 12 2018-12-28 23:48:30 PST

Created attachment 358134 [details] Patch

Yusuke Suzuki

Comment 13 2018-12-29 02:18:30 PST

Could you ensure that this patch does not cause perf regression by Octane/CodeLoad?

Yusuke Suzuki

Comment 14 2018-12-29 02:19:18 PST

Use run-jsc-benchmarks w/ Octane

Ross Kirsling

Comment 15 2018-12-29 12:53:30 PST

Before After encrypt 0.16460+-0.00032 0.16435+-0.00154 decrypt 3.29525+-0.01817 ? 3.29565+-0.02666 ? deltablue x2 0.12919+-0.01639 ? 0.14072+-0.01554 ? might be 1.0893x slower earley 0.28898+-0.00391 0.28864+-0.00190 boyer 4.64579+-0.25275 ? 4.79528+-0.07162 ? might be 1.0322x slower navier-stokes x2 5.15525+-0.02930 ? 5.17400+-0.02981 ? raytrace x2 0.95944+-0.00448 ? 0.96148+-0.01091 ? richards x2 0.09460+-0.00215 0.09414+-0.00064 splay x2 0.26329+-0.01758 0.26018+-0.02029 might be 1.0120x faster regexp x2 17.43834+-0.67396 ? 17.45390+-0.34534 ? pdfjs x2 36.92053+-0.35143 ? 37.09441+-0.33873 ? mandreel x2 44.16759+-0.26156 44.14174+-0.15724 gbemu x2 31.62589+-0.40317 ? 31.70962+-0.23330 ? closure 0.52461+-0.00206 ? 0.52802+-0.01309 ? jquery 7.05201+-0.07046 7.01159+-0.05275 box2d x2 8.74566+-0.17569 ? 8.86146+-0.06800 ? might be 1.0132x slower zlib x2 311.75838+-2.98803 ? 312.19987+-0.37864 ? typescript x2 652.24139+-13.43518 650.57477+-17.68439 splay-latency 2.29484+-0.57967 2.16220+-0.33481 might be 1.0613x faster <geometric> 4.97333+-0.05942 ? 4.99993+-0.07612 ? might be 1.0053x slower

Yusuke Suzuki

Comment 16 2018-12-30 01:00:22 PST

Comment on attachment 358134 [details] Patch r=me. Before landing, please ensure that perf regression does not happen in UChar path by adding some non latin characters in Octane jquery/closure and ensure Lexer<UChar> is used at that time (instead of Lexer<LChar>.

Ross Kirsling

Comment 17 2018-12-30 09:10:57 PST

(In reply to Yusuke Suzuki from comment #16) > Comment on attachment 358134 [details] > Patch > > r=me. Before landing, please ensure that perf regression does not happen in > UChar path by adding some non latin characters in Octane jquery/closure and > ensure Lexer<UChar> is used at that time (instead of Lexer<LChar>. Sure -- if I scatter six non-Latin variable declarations among the source of each, the results are similar: closure 0.55140+-0.02087 0.53967+-0.00117 might be 1.0217x faster jquery 7.33271+-0.43245 7.21572+-0.14073 might be 1.0162x faster

Yusuke Suzuki

Comment 18 2018-12-30 09:31:32 PST

OK, cool. Nice work!

WebKit Commit Bot

Comment 19 2018-12-30 09:39:05 PST

Comment on attachment 358134 [details] Patch Clearing flags on attachment: 358134 Committed r239559: <https://trac.webkit.org/changeset/239559>

WebKit Commit Bot

Comment 20 2018-12-30 09:39:07 PST

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 21 2018-12-30 09:40:31 PST

<rdar://problem/46982697>

Note You need to log in before you can comment on or make changes to this bug.