Reviewers: Dmitry Lomov (chromium),
Description:
Computed property names for object literals in TurboFan.
[email protected]
TEST=mjsunit/harmony/computed-property-names
Please review this at https://codereview.chromium.org/860033002/
Base URL:
https://chromium.googlesource.com/v8/v8.git@local_graph-builder-computed-names-1
Affected files (+70, -8 lines):
M src/compiler/ast-graph-builder.cc
M test/mjsunit/harmony/computed-property-names.js
M test/mjsunit/mjsunit.status
Index: src/compiler/ast-graph-builder.cc
diff --git a/src/compiler/ast-graph-builder.cc
b/src/compiler/ast-graph-builder.cc
index
8443cfafac50cf6aff02651acfb4aa55e51fc24e..e4b22601eca6020ca374b21e529e49490e5ccd8d
100644
--- a/src/compiler/ast-graph-builder.cc
+++ b/src/compiler/ast-graph-builder.cc
@@ -1027,9 +1027,11 @@ void
AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
expr->CalculateEmitStore(zone());
// Create nodes to store computed values into the literal.
+ int property_index = 0;
AccessorTable accessor_table(zone());
- for (int i = 0; i < expr->properties()->length(); i++) {
- ObjectLiteral::Property* property = expr->properties()->at(i);
+ for (; property_index < expr->properties()->length(); property_index++) {
+ ObjectLiteral::Property* property =
expr->properties()->at(property_index);
+ if (property->is_computed_name()) break;
if (property->IsCompileTimeValue()) continue;
Literal* key = property->key()->AsLiteral();
@@ -1111,6 +1113,64 @@ void
AstGraphBuilder::VisitObjectLiteral(ObjectLiteral* expr) {
PrepareFrameState(call, BailoutId::None());
}
+ // Object literals have two parts. The "static" part on the left
contains no
+ // computed property names, and so we can compute its map ahead of time;
see
+ // Runtime_CreateObjectLiteralBoilerplate. The second "dynamic" part
starts
+ // with the first computed property name and continues with all
properties to
+ // its right. All the code from above initializes the static component
of the
+ // object literal, and arranges for the map of the result to reflect the
+ // static order in which the keys appear. For the dynamic properties, we
+ // compile them into a series of "SetOwnProperty" runtime calls. This
will
+ // preserve insertion order.
+ for (; property_index < expr->properties()->length(); property_index++) {
+ ObjectLiteral::Property* property =
expr->properties()->at(property_index);
+
+ environment()->Push(literal); // Duplicate receiver.
+ VisitForValue(property->key());
+ environment()->Push(BuildToName(environment()->Pop()));
+ // TODO(mstarzinger): For ObjectLiteral::Property::PROTOTYPE the key
should
+ // not be on the operand stack while the value is being evaluated.
Come up
+ // with a repro for this and fix it. Also find a nice way to do so. :)
+ VisitForValue(property->value());
+ Node* value = environment()->Pop();
+ Node* key = environment()->Pop();
+ Node* receiver = environment()->Pop();
+
+ switch (property->kind()) {
+ case ObjectLiteral::Property::CONSTANT:
+ case ObjectLiteral::Property::COMPUTED:
+ case ObjectLiteral::Property::MATERIALIZED_LITERAL: {
+ Node* attr = jsgraph()->Constant(NONE);
+ const Operator* op =
+
javascript()->CallRuntime(Runtime::kDefineDataPropertyUnchecked, 4);
+ Node* call = NewNode(op, receiver, key, value, attr);
+ PrepareFrameState(call, BailoutId::None());
+ break;
+ }
+ case ObjectLiteral::Property::PROTOTYPE: {
+ const Operator* op =
+ javascript()->CallRuntime(Runtime::kInternalSetPrototype, 2);
+ Node* call = NewNode(op, receiver, value);
+ PrepareFrameState(call, BailoutId::None());
+ break;
+ }
+ case ObjectLiteral::Property::GETTER: {
+ const Operator* op = javascript()->CallRuntime(
+ Runtime::kDefineGetterPropertyUnchecked, 3);
+ Node* call = NewNode(op, receiver, key, value);
+ PrepareFrameState(call, BailoutId::None());
+ break;
+ }
+ case ObjectLiteral::Property::SETTER: {
+ const Operator* op = javascript()->CallRuntime(
+ Runtime::kDefineSetterPropertyUnchecked, 3);
+ Node* call = NewNode(op, receiver, key, value);
+ PrepareFrameState(call, BailoutId::None());
+ break;
+ }
+ }
+ }
+
// Transform literals that contain functions to fast properties.
if (expr->has_function()) {
const Operator* op =
Index: test/mjsunit/harmony/computed-property-names.js
diff --git a/test/mjsunit/harmony/computed-property-names.js
b/test/mjsunit/harmony/computed-property-names.js
index
1ec8aab51c28fcc62984bbcb1b80cc590daa184f..69360771c149006cfbde1fed952a49da27fe2608
100644
--- a/test/mjsunit/harmony/computed-property-names.js
+++ b/test/mjsunit/harmony/computed-property-names.js
@@ -263,10 +263,17 @@ function ID(x) {
};
assertEquals(proto, Object.getPrototypeOf(object));
- var object = {
+ object = {
['__proto__']: proto
};
assertEquals(Object.prototype, Object.getPrototypeOf(object));
assertEquals(proto, object.__proto__);
assertTrue(object.hasOwnProperty('__proto__'));
+
+ object = {
+ [ID('x')]: 'X',
+ __proto__: proto
+ };
+ assertEquals('X', object.x);
+ assertEquals(proto, Object.getPrototypeOf(object));
})();
Index: test/mjsunit/mjsunit.status
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index
86632b7789b8317fdc844eb44ed6d650b7ef8c40..c8d2126a1f03bddbcf215e946997fcb3a5ff02b4
100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -84,11 +84,6 @@
# not work, but we expect it to not crash.
'debug-step-turbofan': [PASS, FAIL],
- # TODO(mstarzinger): Investigate failures with computed property names.
- # Note that this happens in GC-stress mode only.
- 'harmony/computed-property-names': [PASS, NO_VARIANTS],
- 'harmony/computed-property-names-object-literals-methods': [PASS,
NO_VARIANTS],
-
# TODO(jarin/mstarzinger): Investigate debugger issues with TurboFan.
'debug-evaluate-const': [PASS, NO_VARIANTS],
'debug-evaluate-locals': [PASS, NO_VARIANTS],
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.