Bug 152304 - [JSC] fix error message for eval/arguments CoverInitializedName in strict code
Summary: [JSC] fix error message for eval/arguments CoverInitializedName in strict code
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-15 10:42 PST by Caitlin Potter (:caitp)
Modified: 2015-12-18 01:15 PST (History)
10 users (show)

See Also:


Attachments
Patch (bigger/works) (7.14 KB, patch)
2015-12-15 10:44 PST, Caitlin Potter (:caitp)
no flags Details | Formatted Diff | Diff
Patch (smaller/broken) (6.32 KB, patch)
2015-12-15 11:33 PST, Caitlin Potter (:caitp)
buildbot: commit-queue-
Details | Formatted Diff | Diff
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 Details
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 Details
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 Details

Note You need to log in before you can comment on or make changes to this bug.
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.