Bug 142943
Summary: | ES6: Computed Properties should always produce string/symbol keys never numeric indexes | ||
---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> |
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | ggaren, joepeck, ysuzuki |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified |
Joseph Pecoraro
* SUMMARY
https://people.mozilla.org/~jorendorff/es6-draft.html#sec-object-initializer-runtime-semantics-evaluation
> Runtime Semantics: Evaluation
>
> ComputedPropertyName : [ AssignmentExpression ]
> Let exprValue be the result of evaluating AssignmentExpression.
> Let propName be GetValue(exprValue).
> ReturnIfAbrupt(propName).
> Return ToPropertyKey(propName).
> ---
>
> ToPropertyKey ( argument )
>
> The abstract operation ToPropertyKey converts argument to a value that can
> be used as a property key by performing the following steps:
>
> Let key be ToPrimitive(argument, hint String).
> ReturnIfAbrupt(key).
> If Type(key) is Symbol, then
> Return key.
> Return ToString(key).
* TEST (LayoutTests/js/script-tests/basic-computed-property-name.js)
var a = 0;
runTest("{[a]: true, get '0'(){ return false; }}[0])")
As written, this should actually be `false`, but currently it is `true`.
It seems we are treating the index key as an index instead of a string, and it goes down a different path.
* NOTES
- Firefox behaves correctly here
- Chrome does not currently have computed properties
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Joseph Pecoraro
So it seems "op_put_by_val_direct" is used in lots of places:
- computed properties (PropertyListNode::emitBytecode)
- array literal with spread (ArrayNode::emitBytecode)
- and any put by val used by built-ins
The implementation of the op code does special case integer indexes.
I'm not sure we can reconcile the difference that only computed properties has with the generic put by value case.
It is worth keeping in mind that we will soon have computed getter/setter names, that will also have this same to string conversion of the property.
- It seems wasteful to extend the existing opcode because it can be common
- It seems wasteful to add a new op code just for this
- but it would be easy (just call JSValue::toPropertyKey)
I don't see any existing opcodes that would do exactly what we want here without changes. The closest is op_to_index_string (assumes int property provided).
Yusuke Suzuki
This is now fixed by http://trac.webkit.org/changeset/187464!