Closed Bug 1198833 Opened 9 years ago Closed 8 years ago

Redeclaration of variables should be a SyntaxError

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

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)

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: nobody → jcoppeard
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 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)
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 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+
https://hg.mozilla.org/mozilla-central/rev/d447860f4550
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: