Bug 172660

Summary: Prevent async methods named 'function' in Object literal
Product: WebKit Reporter: GSkachkov <gskachkov>
Component: JavaScriptCoreAssignee: GSkachkov <gskachkov>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, caitp, commit-queue, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 172598    
Bug Blocks: 172776    
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews121 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews121 for ios-simulator-wk2 none

GSkachkov
Reported 2017-05-26 13:15:30 PDT
Attachments
Patch (3.52 KB, patch)
2017-05-28 01:22 PDT, GSkachkov
no flags
Archive of layout-test-results from ews121 for ios-simulator-wk2 (912.28 KB, application/zip)
2017-05-28 02:45 PDT, Build Bot
no flags
Patch (3.82 KB, patch)
2017-05-29 05:58 PDT, GSkachkov
no flags
Archive of layout-test-results from ews121 for ios-simulator-wk2 (11.00 MB, application/zip)
2017-05-29 07:23 PDT, Build Bot
no flags
GSkachkov
Comment 1 2017-05-28 01:22:10 PDT
Created attachment 311433 [details] Patch Patch
Build Bot
Comment 2 2017-05-28 02:45:21 PDT
Comment on attachment 311433 [details] Patch Attachment 311433 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3831230 New failing tests: http/tests/cache/cancel-during-revalidation-succeeded.html
Build Bot
Comment 3 2017-05-28 02:45:23 PDT
Created attachment 311434 [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.12.5
Saam Barati
Comment 4 2017-05-28 13:09:33 PDT
Comment on attachment 311433 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311433&action=review > Source/JavaScriptCore/parser/Parser.cpp:3724 > + semanticFailIfTrue(isAsyncMethod && !isGenerator && *ident == m_vm->propertyNames->functionKeyword, "Cannot declare an async method named 'function'"); This does not apply to generators? Also, why do this here and not in parsePropertyMethod?
GSkachkov
Comment 5 2017-05-29 05:58:32 PDT
Created attachment 311469 [details] Patch Patch with fixed comments
Build Bot
Comment 6 2017-05-29 07:23:05 PDT
Comment on attachment 311469 [details] Patch Attachment 311469 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3837098 New failing tests: webrtc/peer-connection-audio-mute.html
Build Bot
Comment 7 2017-05-29 07:23:06 PDT
Created attachment 311471 [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.12.5
GSkachkov
Comment 8 2017-05-29 09:12:05 PDT
Comment on attachment 311433 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311433&action=review >> Source/JavaScriptCore/parser/Parser.cpp:3724 >> + semanticFailIfTrue(isAsyncMethod && !isGenerator && *ident == m_vm->propertyNames->functionKeyword, "Cannot declare an async method named 'function'"); > > This does not apply to generators? Also, why do this here and not in parsePropertyMethod? According to PR, it make changes only in Async function, but as for now we do not have asyncGenerator, so we can remove !isGenerator. I've checked that parsePropertyMethod used 3 times, so I decided to add condition in place that used to parse Object literal, to prevent checking when it definitely not our case.In new patch I added to parsePropertyMethod
Saam Barati
Comment 9 2017-05-30 13:49:34 PDT
Comment on attachment 311469 [details] Patch r=me
WebKit Commit Bot
Comment 10 2017-05-30 14:35:52 PDT
Comment on attachment 311469 [details] Patch Clearing flags on attachment: 311469 Committed r217578: <http://trac.webkit.org/changeset/217578>
WebKit Commit Bot
Comment 11 2017-05-30 14:35:54 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 12 2017-05-30 20:20:45 PDT
Mark Lam
Comment 13 2017-05-31 10:37:35 PDT
We probably want to roll this out. See https://bugs.webkit.org/show_bug.cgi?id=172598#c14
GSkachkov
Comment 14 2017-05-31 11:31:16 PDT
(In reply to Mark Lam from comment #13) > We probably want to roll this out. See > https://bugs.webkit.org/show_bug.cgi?id=172598#c14 Yeah, I see. Will create patch to rollback this changes.
Note You need to log in before you can comment on or make changes to this bug.