1556818 - Throw early SyntaxError instead of ReferenceError for 0 = 0

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Tom S [:evilpie]]

For cases like 0 = 0 or 0 += 1 we need to start throwing SyntaxErrors.

This is changing with https://github.com/tc39/ecma262/pull/1527
https://github.com/tc39/test262/pull/2147

Comment 1

[Ross Kirsling [:rkirsling]]

Attachment: Bug 1556818 - Throw early SyntaxError instead of ReferenceError for 0 = 0

Comment 2

[Ross Kirsling [:rkirsling]]

Thought I'd try my hand at this. Here's hoping I followed patch submission procedures correctly. :D

Comment 3

[Ross Kirsling [:rkirsling]]

Attachment: Bug 1556818 - Remove unnecessary change

Depends on D35298

Comment 4

[Ross Kirsling [:rkirsling]]

Ugh. I didn't see any other way to update a diff in the moz-phab documentation, but I'm pretty certain I've done the wrong thing here in having created a second commit to amend the first.

Comment 5

[Tom S [:evilpie]]

Yes, you can just amend the original commit and run moz-phab submit again, that should update the revision on phabricator.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eb8646af425c
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

Comment 7

[Noemi Erli[:noemi_erli]]

Backed out changeset eb8646af425c (Bug 1556818) for bustages in invalid-asssignmenttargettype-reference-error-14-lhs-assignment-operator-assignment-expression.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=252655561&resultStatus=testfailed%2Cbusted%2Cexception&revision=eb8646af425c3f6410f41c4226ec70dfb60b5a44

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=252655561&repo=autoland&lineNumber=38399

Backout: https://hg.mozilla.org/integration/autoland/rev/d7f86b3e6ca64fb7a5a3b098db0f4f7d274aa6ed

Comment 8

[Ashley Hauck [:khyperia]]

Tests failed because the test262 update hadn't landed yet (it has now): https://bugzilla.mozilla.org/show_bug.cgi?id=1560206

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59f7de9c3032
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

Comment 10

[Alexandru Michis [:malexandru]]

Backed out changeset 59f7de9c3032 for causing bustages.

Backout link: https://hg.mozilla.org/integration/autoland/rev/70070eea78d11d41970dfc4c731e0079df128ee7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel%2Crunnable&revision=59f7de9c3032d921a71860840fc31c9ab6944843&selectedJob=253070522

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=253070522&repo=autoland&lineNumber=40354

[task 2019-06-24T09:07:08.453Z] 09:07:08 INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoPrintWhenVerboseNotExplicit PASSED
[task 2019-06-24T09:07:08.453Z] 09:07:08 INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportForeignObject PASSED
[task 2019-06-24T09:07:08.453Z] 09:07:08 WARNING - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportNonSyntaxError TEST-UNEXPECTED-FAIL
[task 2019-06-24T09:07:08.453Z] 09:07:08 INFO - ../testing/xpcshell/selftest.py::XPCShellTestsTests::testDoReportRefError PASSED

Comment 11

[Ross Kirsling [:rkirsling]]

Looks like this breaks a test outside of SpiderMonkey too.

Comment 12

[Jason Orendorff [:jorendorff]]

OK, the problem is that this line is expected to trigger a ReferenceError.

Try changing the script to say:

# A test for failure to load a test due to an error other than a syntax error
LOAD_ERROR_OTHER_ERROR = '''
"use strict";
no_such_var = "foo"; // assignment to undeclared variable
'''

and adjust the assertion here to expect the new error message, "ReferenceError: assignment to undeclared variable".

Comment 13

[Jason Orendorff [:jorendorff]]

I really don't know if that will work!

Previously this was testing for a static ReferenceError. There's no longer any such thing, so now we'll be testing for a run-time ReferenceError. I didn't see an existing test for that, so it's good to be adding one.

Comment 14

[Ross Kirsling [:rkirsling]]

Sounds good to me—I'll give that a shot as soon as I can!

Comment 15

[Jason Orendorff [:jorendorff]]

ni?myself to make sure this doesn't fall through the cracks while Ashley is AFK

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/789bacca1219
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

Comment 17

[Natalia Csoregi [:nataliaCs]]

Backed out for web platform failures on module-error-reporting.html.

backout: https://hg.mozilla.org/integration/autoland/rev/3a6df9d54eaa812e74eea1fa2bb5ad32a9748f56

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=789bacca12192e71b88705e3f1eab63f0228743e&searchStr=wpt11&selectedJob=254061970

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=254061970&repo=autoland&lineNumber=7747

09:03:05 INFO - TEST-START | /_mozilla/html/semantics/scripting-1/the-script-element/module-error-reporting.html
09:03:05 INFO - Clearing pref dom.moduleScripts.enabled
09:03:05 INFO - Setting pref dom.moduleScripts.enabled (true)
09:03:05 INFO - Closing window 6442450953
09:03:05 INFO -
09:03:05 INFO - TEST-UNEXPECTED-FAIL | /_mozilla/html/semantics/scripting-1/the-script-element/module-error-reporting.html | Test error event properties - assert_true: expected true got false
09:03:05 INFO - handleError/<@http://web-platform.test:8000/_mozilla/html/semantics/scripting-1/the-script-element/module-error-reporting.html:58:25
09:03:05 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:25
09:03:05 INFO - handleError@http://web-platform.test:8000/_mozilla/html/semantics/scripting-1/the-script-element/module-error-reporting.html:58:14
09:03:05 INFO - EventListener.handleEvent*@http://web-platform.test:8000/_mozilla/html/semantics/scripting-1/the-script-element/module-error-reporting.html:13:10
09:03:05 INFO - TEST-OK | /_mozilla/html/semantics/scripting-1/the-script-element/module-error-reporting.html | took 513ms

Comment 18

[Ross Kirsling [:rkirsling]]

Evidently a custom web platform test needed updating as well.
(Doesn't look like this test was ever submitted to upstream WPT? So I guess it suffices to address it here.)

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b8dfb4b00154
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia,jorendorff

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b8dfb4b00154

Comment 21

[Jason Orendorff [:jorendorff]]

It stuck! Yay!