RESOLVED FIXED 172660
Prevent async methods named 'function' in Object literal
https://bugs.webkit.org/show_bug.cgi?id=172660
Summary Prevent async methods named 'function' in Object literal
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.