Clean up object literal handling with integral indices

* Object literals with array indices are now created with one
  run-time call, instead of an initial one for non-integral keys
  followed by sub-sequent define_builtin_property calls.

* Cleaned up propert name retrieval. Instead of using a visitor,
  it's easier to define a virtual method on the PropertyName type.  The visitor
  doesn't buy us much as it's not possible to recurse within property names, and
  this way we can use it also from the function scanner to correctly determine
  the number of arguments needed for object literal initalizations.

* Similarly the duplicated/common name member for all property assignments
  has been moved into PropertyName, for convenient access without AST casts.

* Removed now unused builtin_define_property/settergetter functions from IR,
  run-time and moth.

Change-Id: I90d54c81ea5f3f500f4f4a9c14f7caf5135e7f9f
Reviewed-by: Lars Knoll <lars.knoll@digia.com>

1 files changed, 79 insertions, 120 deletions

diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp
index 6685336b5f..f08ebaf6cd 100644
--- a/src/qml/compiler/qv4codegen.cpp
+++ b/src/qml/compiler/qv4codegen.cpp
@@ -261,6 +261,9 @@ bool Codegen::ScanFunctions::visit(ObjectLiteral *ast)
 {
     int argc = 0;
     for (PropertyAssignmentList *it = ast->properties; it; it = it->next) {
+        QString key = it->assignment->name->asString();
+        if (QV4::String::toArrayIndex(key) != UINT_MAX)
+            ++argc;
         ++argc;
         if (AST::cast<AST::PropertyGetterSetter *>(it->assignment))
             ++argc;
@@ -776,17 +779,6 @@ Codegen::Result Codegen::expression(ExpressionNode *ast)
     return r;
 }
 
-QString Codegen::propertyName(PropertyName *ast)
-{
-    QString p;
-    if (ast) {
-        qSwap(_property, p);
-        accept(ast);
-        qSwap(_property, p);
-    }
-    return p;
-}
-
 Codegen::Result Codegen::sourceElement(SourceElement *ast)
 {
     Result r(nx);
@@ -1597,8 +1589,8 @@ bool Codegen::visit(ObjectLiteral *ast)
     QMap<QString, ObjectPropertyValue> valueMap;
 
     for (PropertyAssignmentList *it = ast->properties; it; it = it->next) {
+        QString name = it->assignment->name->asString();
         if (PropertyNameAndValue *nv = AST::cast<AST::PropertyNameAndValue *>(it->assignment)) {
-            QString name = propertyName(nv->name);
             Result value = expression(nv->value);
             ObjectPropertyValue &v = valueMap[name];
             if (v.hasGetter() || v.hasSetter() || (_function->isStrict && v.value)) {
@@ -1609,7 +1601,6 @@ bool Codegen::visit(ObjectLiteral *ast)
 
             valueMap[name].value = *value;
         } else if (PropertyGetterSetter *gs = AST::cast<AST::PropertyGetterSetter *>(it->assignment)) {
-            QString name = propertyName(gs->name);
             const int function = defineFunction(name, gs, gs->formals, gs->functionBody ? gs->functionBody->elements : 0);
             ObjectPropertyValue &v = valueMap[name];
             if (v.value ||
@@ -1628,104 +1619,99 @@ bool Codegen::visit(ObjectLiteral *ast)
         }
     }
 
-    IR::ExprList *args = 0;
+    // The linked-list arguments to builtin_define_object_literal
+    // begin with a CONST counting the number of key/value pairs, followed by the
+    // key value pairs, followed by the array entries.
+    IR::ExprList *args = _function->New<IR::ExprList>();
 
-    if (!valueMap.isEmpty()) {
-        IR::ExprList *current;
-        for (QMap<QString, ObjectPropertyValue>::iterator it = valueMap.begin(); it != valueMap.end(); ) {
-            if (QV4::String::toArrayIndex(it.key()) != UINT_MAX) {
-                ++it;
-                continue;
+    IR::Const *entryCountParam = _function->New<IR::Const>();
+    entryCountParam->init(IR::SInt32Type, 0);
+    args->expr = entryCountParam;
+    args->next = 0;
+
+    IR::ExprList *keyValueEntries = 0;
+    IR::ExprList *currentKeyValueEntry = 0;
+    int keyValueEntryCount = 0;
+    IR::ExprList *arrayEntries = 0;
+
+    IR::ExprList *currentArrayEntry = 0;
+
+    for (QMap<QString, ObjectPropertyValue>::iterator it = valueMap.begin(); it != valueMap.end(); ) {
+        IR::ExprList **currentPtr = 0;
+        uint keyAsIndex = QV4::String::toArrayIndex(it.key());
+        if (keyAsIndex != UINT_MAX) {
+            if (!arrayEntries) {
+                arrayEntries = _function->New<IR::ExprList>();
+                currentArrayEntry = arrayEntries;
+            } else {
+                currentArrayEntry->next = _function->New<IR::ExprList>();
+                currentArrayEntry = currentArrayEntry->next;
             }
-
-            if (!args) {
-                args = _function->New<IR::ExprList>();
-                current = args;
+            currentPtr = &currentArrayEntry;
+            IR::Const *idx = _function->New<IR::Const>();
+            idx->init(IR::UInt32Type, keyAsIndex);
+            (*currentPtr)->expr = idx;
+        } else {
+            if (!keyValueEntries) {
+                keyValueEntries = _function->New<IR::ExprList>();
+                currentKeyValueEntry = keyValueEntries;
             } else {
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
+                currentKeyValueEntry->next = _function->New<IR::ExprList>();
+                currentKeyValueEntry = currentKeyValueEntry->next;
             }
+            currentPtr = &currentKeyValueEntry;
+            (*currentPtr)->expr = _block->NAME(it.key(), 0, 0);
+            keyValueEntryCount++;
+        }
 
-            current->expr = _block->NAME(it.key(), 0, 0);
-
-            if (it->value) {
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
-                current->expr = _block->CONST(IR::BoolType, true);
-
-                unsigned value = _block->newTemp();
-                move(_block->TEMP(value), it->value);
+        IR::ExprList *&current = *currentPtr;
+        if (it->value) {
+            current->next = _function->New<IR::ExprList>();
+            current = current->next;
+            current->expr = _block->CONST(IR::BoolType, true);
 
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
-                current->expr = _block->TEMP(value);
-            } else {
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
-                current->expr = _block->CONST(IR::BoolType, false);
-
-                unsigned getter = _block->newTemp();
-                unsigned setter = _block->newTemp();
-                move(_block->TEMP(getter), it->hasGetter() ? _block->CLOSURE(it->getter) : _block->CONST(IR::UndefinedType, 0));
-                move(_block->TEMP(setter), it->hasSetter() ? _block->CLOSURE(it->setter) : _block->CONST(IR::UndefinedType, 0));
-
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
-                current->expr = _block->TEMP(getter);
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
-                current->expr = _block->TEMP(setter);
-            }
+            unsigned value = _block->newTemp();
+            move(_block->TEMP(value), it->value);
 
-            it = valueMap.erase(it);
-        }
-    }
+            current->next = _function->New<IR::ExprList>();
+            current = current->next;
+            current->expr = _block->TEMP(value);
+        } else {
+            current->next = _function->New<IR::ExprList>();
+            current = current->next;
+            current->expr = _block->CONST(IR::BoolType, false);
 
-    const unsigned t = _block->newTemp();
-    move(_block->TEMP(t), _block->CALL(_block->NAME(IR::Name::builtin_define_object_literal,
-         ast->firstSourceLocation().startLine, ast->firstSourceLocation().startColumn), args));
+            unsigned getter = _block->newTemp();
+            unsigned setter = _block->newTemp();
+            move(_block->TEMP(getter), it->hasGetter() ? _block->CLOSURE(it->getter) : _block->CONST(IR::UndefinedType, 0));
+            move(_block->TEMP(setter), it->hasSetter() ? _block->CLOSURE(it->setter) : _block->CONST(IR::UndefinedType, 0));
 
-    // What's left are array entries
-    if (!valueMap.isEmpty()) {
-        unsigned value = 0;
-        unsigned getter = 0;
-        unsigned setter = 0;
-        for (QMap<QString, ObjectPropertyValue>::const_iterator it = valueMap.constBegin(); it != valueMap.constEnd(); ++it) {
-            IR::ExprList *args = _function->New<IR::ExprList>();
-            IR::ExprList *current = args;
-            current->expr = _block->TEMP(t);
             current->next = _function->New<IR::ExprList>();
             current = current->next;
-            current->expr = _block->NAME(it.key(), 0, 0);
+            current->expr = _block->TEMP(getter);
             current->next = _function->New<IR::ExprList>();
             current = current->next;
+            current->expr = _block->TEMP(setter);
+        }
 
-            if (it->value) {
-                if (!value)
-                    value = _block->newTemp();
-                move(_block->TEMP(value), it->value);
-                // __qmljs_builtin_define_property(Value object, String *name, Value val, ExecutionContext *ctx)
-                current->expr = _block->TEMP(value);
-                _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_define_property, 0, 0), args));
-            } else {
-                if (!getter) {
-                    getter = _block->newTemp();
-                    setter = _block->newTemp();
-                }
-                move(_block->TEMP(getter), it->hasGetter() ? _block->CLOSURE(it->getter) : _block->CONST(IR::UndefinedType, 0));
-                move(_block->TEMP(setter), it->hasSetter() ? _block->CLOSURE(it->setter) : _block->CONST(IR::UndefinedType, 0));
+        it = valueMap.erase(it);
+    }
 
+    entryCountParam->value = keyValueEntryCount;
 
-                // __qmljs_builtin_define_getter_setter(Value object, String *name, Value getter, Value setter, ExecutionContext *ctx);
-                current->expr = _block->TEMP(getter);
-                current->next = _function->New<IR::ExprList>();
-                current = current->next;
-                current->expr = _block->TEMP(setter);
-                _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_define_getter_setter, 0, 0), args));
-            }
-        }
+    if (keyValueEntries)
+        args->next = keyValueEntries;
+    if (arrayEntries) {
+        if (currentKeyValueEntry)
+            currentKeyValueEntry->next = arrayEntries;
+        else
+            args->next = arrayEntries;
     }
 
+    const unsigned t = _block->newTemp();
+    move(_block->TEMP(t), _block->CALL(_block->NAME(IR::Name::builtin_define_object_literal,
+         ast->firstSourceLocation().startLine, ast->firstSourceLocation().startColumn), args));
+
     _expr.code = _block->TEMP(t);
     return false;
 }
@@ -2072,33 +2058,6 @@ int Codegen::defineFunction(const QString &name, AST::Node *ast,
     return functionIndex;
 }
 
-bool Codegen::visit(IdentifierPropertyName *ast)
-{
-    if (hasError)
-        return false;
-
-    _property = ast->id.toString();
-    return false;
-}
-
-bool Codegen::visit(NumericLiteralPropertyName *ast)
-{
-    if (hasError)
-        return false;
-
-    _property = QString::number(ast->id, 'g', 16);
-    return false;
-}
-
-bool Codegen::visit(StringLiteralPropertyName *ast)
-{
-    if (hasError)
-        return false;
-
-    _property = ast->id.toString();
-    return false;
-}
-
 bool Codegen::visit(FunctionSourceElement *ast)
 {
     if (hasError)