Closed
Bug 1198833
Opened 9 years ago
Closed 9 years ago
Redeclaration of variables should be a SyntaxError
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
Details
(Keywords: dev-doc-complete, site-compat)
Attachments
(1 file, 1 obsolete file)
12.95 KB,
patch
|
shu
:
review+
|
Details | Diff | Splinter Review |
For scripts, modules and function bodies the spec says it is a syntax error if:
- LexicallyDeclaredNames contains duplicate entries
- any element of the LexicallyDeclaredNames also occurs in the VarDeclaredNames
Currently we report a TypeError for this.
The comment in tests/ecma_6/LexicalEnvironment/for-loop.js makes me slightly leery of just changing this though:
// This is wrong! Per 13.2.1.1, "It is a Syntax Error if the BoundNames of
// BindingList contains any duplicate entries." But we don't implement this
// yet, so it becomes a TypeError at runtime.
I don't understand what the last bit is driving at.
Attachment #8652939 -
Flags: feedback?(efaustbmo)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → jcoppeard
Comment 1•9 years ago
|
||
Let's wait on this until I finish global lexicals, which will turn up more redecl errors.
We can plug the holes after that. Mixing the two efforts is going to be confusing.
Comment 2•9 years ago
|
||
Comment on attachment 8652939 [details] [diff] [review]
make-redeclaration-a-syntax-error
Review of attachment 8652939 [details] [diff] [review]:
-----------------------------------------------------------------
Cancelling feedback based on Comment 1. Feel free to rerequest if this is in error. This patch at least looks very strange, because it makes no changes to the existing system, though I admit it looks like that error is already only thrown in the parser.
Attachment #8652939 -
Flags: feedback?(efaustbmo)
Assignee | ||
Comment 3•9 years ago
|
||
Posting an updated patch now global lexical support has landed.
The patch just changes the exception type of JSMSG_REDECLARED_VAR to be JSEXN_SYNTAXERR rather than JSEXN_TYPEERR and updates the tests.
Is there anything else I need to do to make this work?
Looking at js.msg again it seems there a couple of other errors (JSMSG_REDECLARED_PARAM and JSMSG_REDECLARED_CATCH_IDENTIFIER) that should also be syntax errors according to the spec.
Attachment #8652939 -
Attachment is obsolete: true
Attachment #8692613 -
Flags: feedback?(efaustbmo)
Comment 4•9 years ago
|
||
Comment on attachment 8692613 [details] [diff] [review]
make-redeclaration-a-syntax-error v2
Review of attachment 8692613 [details] [diff] [review]:
-----------------------------------------------------------------
We should land this now.
Attachment #8692613 -
Flags: feedback?(efaustbmo) → review+
Comment 6•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Comment 7•9 years ago
|
||
Updated the site compatibility docs:
https://www.fxsitecompat.com/en-CA/docs/2014/es6-compatible-let-disallows-variable-redeclaration/
https://www.fxsitecompat.com/en-CA/docs/2015/variables-defined-with-const-and-let-are-no-longer-properties-on-window-redeclaration-with-let-will-throw/
Keywords: dev-doc-needed,
site-compat
Comment 8•9 years ago
|
||
Added to
https://developer.mozilla.org/en-US/Firefox/Releases/46#JavaScript
Updated
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/const
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•