Closed Bug 1556818 Opened 3 years ago Closed 2 years ago

Throw early SyntaxError instead of ReferenceError for 0 = 0


(Core :: JavaScript Engine, enhancement, P2)




Tracking Status
firefox69 --- fixed


(Reporter: evilpie, Assigned: rkirsling)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

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

This is changing with

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
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):

Flags: needinfo?(khyperia)
Pushed by
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

Backed out changeset 59f7de9c3032 for causing bustages.

Backout link:

Push with failures:

Failure log:

[task 2019-06-24T09:07:08.453Z] 09:07:08 INFO - ../testing/xpcshell/ PASSED
[task 2019-06-24T09:07:08.453Z] 09:07:08 INFO - ../testing/xpcshell/ PASSED
[task 2019-06-24T09:07:08.453Z] 09:07:08 WARNING - ../testing/xpcshell/ TEST-UNEXPECTED-FAIL
[task 2019-06-24T09:07:08.453Z] 09:07:08 INFO - ../testing/xpcshell/ 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
"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
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia

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



failure log:

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
Throw early SyntaxError instead of ReferenceError for 0 = 0 r=khyperia,jorendorff
Closed: 2 years 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.