Last Comment Bug 694306 - Keywords should not be permitted in destructuring shorthand
: Keywords should not be permitted in destructuring shorthand
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: x86 Mac OS X
-- normal (vote)
: mozilla10
Assigned To: Jason Orendorff [:jorendorff]
: Jason Orendorff [:jorendorff]
Depends on: 699682
  Show dependency treegraph
Reported: 2011-10-13 06:41 PDT by Jason Orendorff [:jorendorff]
Modified: 2011-11-03 18:25 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

v1 (7.97 KB, patch)
2011-10-13 11:08 PDT, Jason Orendorff [:jorendorff]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description User image Jason Orendorff [:jorendorff] 2011-10-13 06:41:25 PDT
js> var {if} = {if:1}; print(this.if);

I think this should be a SyntaxError rather than declare a variable named 'if'.
Comment 1 User image Jason Orendorff [:jorendorff] 2011-10-13 06:45:22 PDT
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:
Comment 2 User image Jason Orendorff [:jorendorff] 2011-10-13 11:08:41 PDT
Created attachment 566885 [details] [diff] [review]
Comment 3 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-10-19 16:41:19 PDT
Comment on attachment 566885 [details] [diff] [review]

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.  :-\
Comment 4 User image Jason Orendorff [:jorendorff] 2011-11-02 11:41:28 PDT
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.
Comment 5 User image Jason Orendorff [:jorendorff] 2011-11-02 12:33:02 PDT
Comment 6 User image Marco Bonardo [::mak] 2011-11-03 08:43:15 PDT

Note You need to log in before you can comment on or make changes to this bug.