Bug 142353 – ES6: Object Literal Extensions - Shorthand Properties (Identifiers)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Joseph Pecoraro]

* SUMMARY
Object Literal Extensions - Shorthand Properties (Identifiers).

ES6 Allows Identifiers in Object Literal Construction.
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-object-initializer

>   ObjectLiteral[Yield] :
>       { }
>       { PropertyDefinitionList[?Yield] }
>       { PropertyDefinitionList[?Yield] , }
>   
>   PropertyDefinitionList[Yield] :
>       PropertyDefinition[?Yield]
>       ...
>   
>   PropertyDefinition[Yield] :
>       IdentifierReference[?Yield]
>       ...

Works by using the identifier name as the property name and identifier value as property value:

    var x = {foo};
    // is equivalent to:
    var x = {foo: foo};

[Joseph Pecoraro]

I have a fix, just need to write tests.

[Yusuke Suzuki]

really great! Since ArrayIterator, StringIterator.prototype.next have { done:done, value: value } JS code, we can apply this enhancement to them :D

[Joseph Pecoraro]

(In reply to comment #2)
> really great! Since ArrayIterator, StringIterator.prototype.next have {
> done:done, value: value } JS code, we can apply this enhancement to them :D

Yep! And the Inspector has ~168 occurrences of this pattern.

[Joseph Pecoraro]

Created attachment 247994 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 247994 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=247994&action=review

> LayoutTests/js/object-literal-shorthand-construction-expected.txt:57
> +PASS ({get}) threw exception SyntaxError: Unexpected token '}'.
> +PASS ({set}) threw exception SyntaxError: Unexpected token '}'.

This may be wrong. Firefox allows it, Chrome does not. Let me re-read and confirm correct behavior. My guess would be this is supposed to work.

[Build Bot]

Comment on attachment 247994 [details]
[PATCH] Proposed Fix

Attachment 247994 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6125427387006976

New failing tests:
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T2.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8.1_T1.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T3.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.4_The_for_in_Statement/S12.6.4_A15.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8.1_T2.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8.1_T3.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T1.html
sputnik/Conformance/12_Statement/12.1_Block/S12.1_A4_T1.html
sputnik/Conformance/12_Statement/12.1_Block/S12.1_A4_T2.html

[Build Bot]

Created attachment 247998 [details]
Archive of layout-test-results from ews100 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 247994 [details]
[PATCH] Proposed Fix

Attachment 247994 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/4755122366185472

New failing tests:
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T2.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8.1_T1.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T3.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.4_The_for_in_Statement/S12.6.4_A15.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8.1_T2.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8.1_T3.html
sputnik/Conformance/12_Statement/12.6_Iteration_Statements/12.6.3_The_for_Statement/S12.6.3_A8_T1.html
sputnik/Conformance/12_Statement/12.1_Block/S12.1_A4_T1.html
sputnik/Conformance/12_Statement/12.1_Block/S12.1_A4_T2.html

[Build Bot]

Created attachment 248001 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Joseph Pecoraro]

Created attachment 248006 [details]
[PATCH] Proposed Fix

- Rebases the sputnik tests that are now expected to fail due to the new syntax with failing results. (Some of the error messages are slightly worse, but not sure we can improve that).
- Now allows {get}/{set} expansion to {get:get}/etc.

[Joseph Pecoraro]

Comment on attachment 248006 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=248006&action=review

> Source/JavaScriptCore/parser/Parser.cpp:1949
> +        currentScope()->useVariable(ident, m_vm->propertyNames->eval == *ident);

I'm not 100% certain what this isEval is for. The only other use of useVariable(), parsePrimaryExpression, does this, so I matched it since I assume if "x = eval" sets the flag then "x = {eval}" should as well.

[Build Bot]

Comment on attachment 248006 [details]
[PATCH] Proposed Fix

Attachment 248006 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6644674437578752

New failing tests:
js/basic-computed-property-name.html
js/property-getters-and-setters.html
js/array-from.html

[Build Bot]

Created attachment 248012 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Build Bot]

Comment on attachment 248006 [details]
[PATCH] Proposed Fix

Attachment 248006 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6518302004215808

New failing tests:
js/basic-computed-property-name.html
js/property-getters-and-setters.html
js/array-from.html

[Build Bot]

Created attachment 248014 [details]
Archive of layout-test-results from ews101 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Joseph Pecoraro]

Comment on attachment 248006 [details]
[PATCH] Proposed Fix

Broke getter/setter syntax with non-identifier names:

  get 42() {}
  get 'prop'() {}

[Joseph Pecoraro]

Created attachment 248016 [details]
[PATCH] Proposed Fix

Even simpler now. Take 3!

[Geoffrey Garen]

Comment on attachment 248016 [details]
[PATCH] Proposed Fix

r=me

[WebKit Commit Bot]

Comment on attachment 248016 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 248016

Committed r181121: <http://trac.webkit.org/changeset/181121>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Looks like this is failing some JSC tests on the bots because:

> -PASS equivalent({eval}, {eval: window.eval}) is true
> +FAIL equivalent({eval}, {eval: window.eval}) should be true. Threw exception ReferenceError: Can't find variable: window

I'll just remove window.

[Joseph Pecoraro]

Attempted to fix the bots with:
<http://trac.webkit.org/changeset/181130>