NEW 152304
[JSC] fix error message for eval/arguments CoverInitializedName in strict code
https://bugs.webkit.org/show_bug.cgi?id=152304
Summary [JSC] fix error message for eval/arguments CoverInitializedName in strict code
Caitlin Potter (:caitp)
Reported 2015-12-15 10:42:10 PST
[JSC] fix error message for eval/arguments CoverInitializedName in strict code
Attachments
Patch (bigger/works) (7.14 KB, patch)
2015-12-15 10:44 PST, Caitlin Potter (:caitp)
no flags
Patch (smaller/broken) (6.32 KB, patch)
2015-12-15 11:33 PST, Caitlin Potter (:caitp)
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-yosemite (1021.81 KB, application/zip)
2015-12-15 12:21 PST, Build Bot
no flags
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (833.26 KB, application/zip)
2015-12-15 12:25 PST, Build Bot
no flags
Archive of layout-test-results from ews117 for mac-yosemite (791.81 KB, application/zip)
2015-12-15 12:26 PST, Build Bot
no flags
Caitlin Potter (:caitp)
Comment 1 2015-12-15 10:44:42 PST
Created attachment 267378 [details] Patch (bigger/works) Report correct error when CoverInitializedName has name 'eval' or 'arguments' in strict code
Caitlin Potter (:caitp)
Comment 2 2015-12-15 11:33:18 PST
Created attachment 267380 [details] Patch (smaller/broken)
Build Bot
Comment 3 2015-12-15 12:21:23 PST
Comment on attachment 267380 [details] Patch (smaller/broken) Attachment 267380 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/563468 New failing tests: sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T3.html
Build Bot
Comment 4 2015-12-15 12:21:26 PST
Created attachment 267385 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
Build Bot
Comment 5 2015-12-15 12:25:32 PST
Comment on attachment 267380 [details] Patch (smaller/broken) Attachment 267380 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/563470 New failing tests: sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T3.html
Build Bot
Comment 6 2015-12-15 12:25:35 PST
Created attachment 267386 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5
Build Bot
Comment 7 2015-12-15 12:26:04 PST
Comment on attachment 267380 [details] Patch (smaller/broken) Attachment 267380 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/563460 New failing tests: sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T3.html
Build Bot
Comment 8 2015-12-15 12:26:08 PST
Created attachment 267387 [details] Archive of layout-test-results from ews117 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews117 Port: mac-yosemite Platform: Mac OS X 10.10.5
Caitlin Potter (:caitp)
Comment 9 2015-12-15 12:31:46 PST
first version is a bit bigger and more complicated, but doesn't break the test
Caitlin Potter (:caitp)
Comment 10 2015-12-15 13:35:31 PST
Comment on attachment 267380 [details] Patch (smaller/broken) View in context: https://bugs.webkit.org/attachment.cgi?id=267380&action=review self-review: > Source/JavaScriptCore/parser/Parser.cpp:2892 > + if (classifier.indicatesPossiblePattern() && (pattern && !match(EQUAL))) This fixes the case `({ eval = 0 } = {})`, but the `for({index=0; index+=1;} index++<=10; index*2;) { arr.add(""+index);};` Sputnik test fails as well, since it's n'either a valid pattern nor a valid expression. Probably the thing to do is avoid unnecessary re-parsing in this case, and just update the expected error to be the unexpected semicolon, and avoid re-parsing as an expression. This probably means replacing all of the `if (kind == DestructureToExpressions) return 0;` bits with a proper error message, and most likely fixing a bunch of layout tests. Doing this means the simpler fix in code should work just fine.
WebKit Commit Bot
Comment 11 2015-12-16 09:49:40 PST
Comment on attachment 267378 [details] Patch (bigger/works) Clearing flags on attachment: 267378 Committed r194153: <http://trac.webkit.org/changeset/194153>
Benjamin Poulain
Comment 12 2015-12-18 01:15:28 PST
Comment on attachment 267380 [details] Patch (smaller/broken) Looks like this has landed. Clearing the review flag to remove it from the review queue.
Note You need to log in before you can comment on or make changes to this bug.