Bug 152304

Summary: [JSC] fix error message for eval/arguments CoverInitializedName in strict code
Product: WebKit Reporter: Caitlin Potter (:caitp) <caitp>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: buildbot, commit-queue, ddkilzer, keith_miller, kling, koivisto, mark.lam, msaboff, rniwa, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch (bigger/works)
none
Patch (smaller/broken)
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-yosemite
none
Archive of layout-test-results from ews105 for mac-yosemite-wk2
none
Archive of layout-test-results from ews117 for mac-yosemite none

Description Caitlin Potter (:caitp) 2015-12-15 10:42:10 PST
[JSC] fix error message for eval/arguments CoverInitializedName in strict code
Comment 1 Caitlin Potter (:caitp) 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
Comment 2 Caitlin Potter (:caitp) 2015-12-15 11:33:18 PST
Created attachment 267380 [details]
Patch (smaller/broken)
Comment 3 Build Bot 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
Comment 4 Build Bot 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
Comment 5 Build Bot 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
Comment 6 Build Bot 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
Comment 7 Build Bot 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
Comment 8 Build Bot 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
Comment 9 Caitlin Potter (:caitp) 2015-12-15 12:31:46 PST
first version is a bit bigger and more complicated, but doesn't break the test
Comment 10 Caitlin Potter (:caitp) 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.
Comment 11 WebKit Commit Bot 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>
Comment 12 Benjamin Poulain 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.