Closed
Bug 694306
Opened 10 years ago
Closed 9 years ago
Keywords should not be permitted in destructuring shorthand
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: jorendorff, Assigned: jorendorff)
References
Details
Attachments
(1 file)
7.97 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
js> var {if} = {if:1}; print(this.if); 1 I think this should be a SyntaxError rather than declare a variable named 'if'.
Assignee | ||
Comment 1•10 years ago
|
||
In strict mode, we ban it, but the error message is odd. js> "use strict"; var {if} = {if: 1}; typein:2: SyntaxError: redefining if is deprecated:
Assignee | ||
Comment 2•10 years ago
|
||
Assignee: general → jorendorff
Attachment #566885 -
Flags: review?(jwalden+bmo)
Comment 3•10 years ago
|
||
Comment on attachment 566885 [details] [diff] [review] v1 Review of attachment 566885 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsscan.h @@ +508,5 @@ > + * and topp are null, report a SyntaxError ("if is a reserved identifier") > + * and return false. If ttp and topp are non-null, return true with the > + * keyword's TokenKind in *ttp and its JSOp in *topp. > + * > + * Preconditions: Assert that these hold, please. @@ +512,5 @@ > + * Preconditions: > + * s[length] must be valid to access and must not be an IdentifierPart. > + * ttp and topp must be either both null or both non-null. > + */ > + bool checkForKeyword(const jschar *s, size_t length, TokenKind *ttp, JSOp *topp); These semantics are so hairy. But maybe they're the cleanest ones. :-\
Attachment #566885 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 4•9 years ago
|
||
I'm dumb. FindKeyword never actually accesses s[length]. I don't know why I thought that. Because I didn't see explicit length checks scattered through jsautokw.h I guess; but because of the switch on length at the top, that isn't necessary. So that's good. I removed the comment about that precondition, and added an assertion for the other precondition.
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/50f566a3b449
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/50f566a3b449
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in
before you can comment on or make changes to this bug.
Description
•