Closed Bug 1556818 Opened 5 months ago Closed 4 months ago

Throw early SyntaxError instead of ReferenceError for 0 = 0

Categories

(Core :: JavaScript Engine, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: evilpie, Assigned: rkirsling)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

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

Type: task → enhancement
Priority: -- → P2

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

Depends on D35298

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.

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

Attachment #9072761 - Attachment is obsolete: true
Pushed by ahauck@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eb8646af425c
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia
Flags: needinfo?(evilpies) → needinfo?(khyperia)

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

Flags: needinfo?(khyperia)
Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/59f7de9c3032
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

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

Flags: needinfo?(rkirsling)

Looks like this breaks a test outside of SpiderMonkey too.

Flags: needinfo?(rkirsling)

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".

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.

Flags: needinfo?(rkirsling)

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

Flags: needinfo?(rkirsling)

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

Flags: needinfo?(jorendorff)
Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/789bacca1219
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

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

Flags: needinfo?(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.)

Flags: needinfo?(rkirsling)
Pushed by jorendorff@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b8dfb4b00154
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia,jorendorff
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Assignee: nobody → rkirsling

It stuck! Yay!

Flags: needinfo?(jorendorff)
You need to log in before you can comment on or make changes to this bug.