Bug 197603 – [JSC] Invalid AssignmentTargetType should be an early error.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Ross Kirsling]

Expressions like 0++, ++0, 0 = 0, and 0 += 0 are all specified as early errors, while JSC handles them as late errors:
https://tc39.github.io/ecma262/#sec-update-expressions-static-semantics-early-errors
https://tc39.github.io/ecma262/#sec-assignment-operators-static-semantics-early-errors

In particular, they're all early ReferenceErrors, a notion that JSC doesn't currently have; however, as I noted in bug 177218 comment 2, I am in the midst of proposing that we drop this notion and let all early errors be SyntaxErrors:
https://github.com/tc39/ecma262/pull/1527

With the hope that the above PR will meet with consensus in one month's time, I'd like to go ahead and turn these into early SyntaxErrors.
Then if all goes well, we can simply do a Test262 import after the spec update.

[Ross Kirsling]

*** Bug 190889 has been marked as a duplicate of this bug. ***

[Ross Kirsling]

Created attachment 369099 [details]
Patch

[Ross Kirsling]

Apologies if I'm jumping the gun a bit on this. ;)

[EWS Watchlist]

Comment on attachment 369099 [details]
Patch

Attachment 369099 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12109134

New failing tests:
fast/events/window-onerror4.html
ietestcenter/Javascript/11.13.1-1-2.html
ietestcenter/Javascript/11.13.1-1-3.html
js/dom/prefix-syntax.html
ietestcenter/Javascript/11.13.1-1-4.html
js/dom/postfix-syntax.html
js/dom/assign.html
ietestcenter/Javascript/11.13.1-1-1.html
js/dom/line-column-numbers.html

[EWS Watchlist]

Created attachment 369103 [details]
Archive of layout-test-results from ews100 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369099 [details]
Patch

Attachment 369099 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12109146

New failing tests:
fast/events/window-onerror4.html
ietestcenter/Javascript/11.13.1-1-2.html
ietestcenter/Javascript/11.13.1-1-3.html
js/dom/prefix-syntax.html
ietestcenter/Javascript/11.13.1-1-4.html
js/dom/postfix-syntax.html
js/dom/assign.html
ietestcenter/Javascript/11.13.1-1-1.html
js/dom/line-column-numbers.html

[EWS Watchlist]

Created attachment 369106 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369099 [details]
Patch

Attachment 369099 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/12109232

New failing tests:
fast/events/window-onerror4.html
ietestcenter/Javascript/11.13.1-1-2.html
ietestcenter/Javascript/11.13.1-1-3.html
js/dom/prefix-syntax.html
ietestcenter/Javascript/11.13.1-1-4.html
js/dom/postfix-syntax.html
js/dom/assign.html
ietestcenter/Javascript/11.13.1-1-1.html
js/dom/line-column-numbers.html

[EWS Watchlist]

Created attachment 369110 [details]
Archive of layout-test-results from ews115 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-highsierra  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 369099 [details]
Patch

Attachment 369099 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12109286

New failing tests:
fast/events/window-onerror4.html
ietestcenter/Javascript/11.13.1-1-2.html
ietestcenter/Javascript/11.13.1-1-3.html
js/dom/prefix-syntax.html
ietestcenter/Javascript/11.13.1-1-4.html
js/dom/postfix-syntax.html
js/dom/assign.html
ietestcenter/Javascript/11.13.1-1-1.html

[EWS Watchlist]

Created attachment 369112 [details]
Archive of layout-test-results from ews213 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews213  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[EWS Watchlist]

Comment on attachment 369099 [details]
Patch

Attachment 369099 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/12109265

New failing tests:
fast/events/window-onerror4.html
ietestcenter/Javascript/11.13.1-1-2.html
ietestcenter/Javascript/11.13.1-1-3.html
js/dom/prefix-syntax.html
ietestcenter/Javascript/11.13.1-1-4.html
js/dom/postfix-syntax.html
js/dom/assign.html
ietestcenter/Javascript/11.13.1-1-1.html

[EWS Watchlist]

Created attachment 369113 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.14.4

[Ross Kirsling]

Created attachment 369140 [details]
Patch

[Ross Kirsling]

Created attachment 369147 [details]
Patch

[Ross Kirsling]

Comment on attachment 369147 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=369147&action=review

> Source/JavaScriptCore/parser/Parser.cpp:3749
>          if (UNLIKELY(context.isMetaProperty(lhs)))
>              internalFailWithMessage(false, metaPropertyName(context, lhs), " can't be the left hand side of an assignment expression");
> +        semanticFailIfFalse(isSimpleAssignmentTarget(context, lhs), "Left side of assignment is not a reference");

Note:
All of these isSimpleAssignmentTarget checks are colocated with !isMetaProperty checks -- the former subsumes the latter, so it might be nice to merge them.
I've avoided touching them for now simply because I wanted to preserve error messages in the initial patch for review.
(Notice that the only time an error *message* changes here is when hitting a requiresLExpr check (e.g. ++--x) in loose mode, since those checks used to be strict-mode-only and have their own unique messaging.)

[Ross Kirsling]

Created attachment 369224 [details]
Patch

[EWS Watchlist]

Comment on attachment 369224 [details]
Patch

Attachment 369224 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12128645

New failing tests:
svg/repaint/remove-border-property-on-root.html
http/tests/css/filters-on-iframes.html

[EWS Watchlist]

Created attachment 369350 [details]
Archive of layout-test-results from ews213 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews213  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Ross Kirsling]

Reminder that this is ready for review. :)

[Ross Kirsling]

Ping?

[Keith Miller]

Comment on attachment 369224 [details]
Patch

r=me. I'm not 100% sure we should do SyntaxError until we have agreement at tc-39 but since I think that's the right choice anyway I'm ok with it. Can you file a bugzilla to track the PR on tc-39 though?

[Ross Kirsling]

(In reply to Keith Miller from comment #22)
> Comment on attachment 369224 [details]
> Patch
> 
> r=me. I'm not 100% sure we should do SyntaxError until we have agreement at
> tc-39 but since I think that's the right choice anyway I'm ok with it. Can
> you file a bugzilla to track the PR on tc-39 though?

Thanks Keith! I'm leaving bug 177218 open for that purpose.

[WebKit Commit Bot]

Comment on attachment 369224 [details]
Patch

Clearing flags on attachment: 369224

Committed r245406: <https://trac.webkit.org/changeset/245406>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/50864605>