Fix the remaining test failures with non strict arguments objects

Change-Id: Ib50f602263dd0146d792fb3d12bd5971585fda30
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

6 files changed, 99 insertions, 248 deletions

diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index 7b501b9fbb..cc2cb83b6b 100644
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -50,28 +50,8 @@ using namespace QV4;
 DEFINE_OBJECT_VTABLE(ArgumentsObject);
 DEFINE_OBJECT_VTABLE(StrictArgumentsObject);
 
-void Heap::ArgumentsObject::init(QV4::CppStackFrame *frame)
-{
-    ExecutionEngine *v4 = internalClass->engine;
-
-    int nFormals = frame->v4Function->nFormals;
-    QV4::CallContext *context = static_cast<QV4::CallContext *>(frame->context());
-
-    Object::init();
-    fullyCreated = false;
-    this->nFormals = nFormals;
-    this->context.set(v4, context->d());
-    Q_ASSERT(vtable() == QV4::ArgumentsObject::staticVTable());
-
-    Q_ASSERT(CalleePropertyIndex == internalClass->find(v4->id_callee()->propertyKey()));
-    setProperty(v4, CalleePropertyIndex, context->d()->function);
-    Q_ASSERT(LengthPropertyIndex == internalClass->find(v4->id_length()->propertyKey()));
-    setProperty(v4, LengthPropertyIndex, Primitive::fromInt32(context->argc()));
-    Q_ASSERT(SymbolIteratorPropertyIndex == internalClass->find(v4->symbol_iterator()->propertyKey()));
-    setProperty(v4, SymbolIteratorPropertyIndex, *v4->arrayProtoValues());
-}
-
 void Heap::StrictArgumentsObject::init(QV4::CppStackFrame *frame)
+
 {
     Q_ASSERT(vtable() == QV4::StrictArgumentsObject::staticVTable());
     ExecutionEngine *v4 = internalClass->engine;
@@ -93,164 +73,138 @@ void Heap::StrictArgumentsObject::init(QV4::CppStackFrame *frame)
     setProperty(v4, LengthPropertyIndex, Primitive::fromInt32(frame->originalArgumentsCount));
 }
 
+void Heap::ArgumentsObject::init(QV4::CppStackFrame *frame)
+{
+    ExecutionEngine *v4 = internalClass->engine;
+
+    QV4::CallContext *context = static_cast<QV4::CallContext *>(frame->context());
+
+    Object::init();
+    this->context.set(v4, context->d());
+    Q_ASSERT(vtable() == QV4::ArgumentsObject::staticVTable());
+
+    Q_ASSERT(CalleePropertyIndex == internalClass->find(v4->id_callee()->propertyKey()));
+    setProperty(v4, CalleePropertyIndex, context->d()->function);
+    Q_ASSERT(LengthPropertyIndex == internalClass->find(v4->id_length()->propertyKey()));
+    setProperty(v4, LengthPropertyIndex, Primitive::fromInt32(context->argc()));
+    Q_ASSERT(SymbolIteratorPropertyIndex == internalClass->find(v4->symbol_iterator()->propertyKey()));
+    setProperty(v4, SymbolIteratorPropertyIndex, *v4->arrayProtoValues());
+
+    fullyCreated = false;
+    argCount = frame->originalArgumentsCount;
+    uint nFormals = frame->v4Function->nFormals;
+    mapped = nFormals > 63 ? std::numeric_limits<quint64>::max() : (1ull << nFormals) - 1;
+}
+
 void ArgumentsObject::fullyCreate()
 {
-    if (fullyCreated())
+    if (d()->fullyCreated)
         return;
 
     Scope scope(engine());
 
-    int argCount = context()->argc();
-    uint numAccessors = qMin(d()->nFormals, argCount);
-    ArrayData::realloc(this, Heap::ArrayData::Sparse, argCount, true);
-    scope.engine->requireArgumentsAccessors(numAccessors);
-
-    Scoped<MemberData> md(scope, d()->mappedArguments);
-    if (numAccessors) {
-        d()->mappedArguments.set(scope.engine, md->allocate(scope.engine, numAccessors));
-        for (uint i = 0; i < numAccessors; ++i) {
-            d()->mappedArguments->values.set(scope.engine, i, context()->args()[i]);
-            arraySet(i, scope.engine->argumentsAccessors + i, Attr_Accessor);
-        }
-    }
-    arrayPut(numAccessors, context()->args() + numAccessors, argCount - numAccessors);
-    for (int i = int(numAccessors); i < argCount; ++i)
-        setArrayAttributes(i, Attr_Data);
+    arrayReserve(d()->argCount);
+    arrayPut(0, context()->args(), d()->argCount);
 
     d()->fullyCreated = true;
 }
 
 bool ArgumentsObject::virtualDefineOwnProperty(Managed *m, PropertyKey id, const Property *desc, PropertyAttributes attrs)
 {
-    if (!id.isArrayIndex())
+    ArgumentsObject *args = static_cast<ArgumentsObject *>(m);
+    args->fullyCreate();
+    uint index = id.asArrayIndex();
+
+    if (!args->isMapped(index))
         return Object::virtualDefineOwnProperty(m, id, desc, attrs);
 
-    ArgumentsObject *a = static_cast<ArgumentsObject *>(m);
-    a->fullyCreate();
+    Scope scope(args);
+    PropertyAttributes cAttrs = attrs;
+    ScopedProperty cDesc(scope);
+    cDesc->copy(desc, attrs);
 
-    uint index = id.asArrayIndex();
-    Scope scope(m);
-    ScopedProperty map(scope);
-    PropertyAttributes mapAttrs;
-    uint numAccessors = qMin(a->d()->nFormals, a->context()->argc());
-    bool isMapped = false;
-    if (a->arrayData() && index < numAccessors &&
-        a->arrayData()->attributes(index).isAccessor() &&
-        a->arrayData()->get(index) == scope.engine->argumentsAccessors[index].getter()->asReturnedValue())
-        isMapped = true;
-
-    if (isMapped) {
-        Q_ASSERT(a->arrayData());
-        mapAttrs = a->arrayData()->attributes(index);
-        a->arrayData()->getProperty(index, map, &mapAttrs);
-        a->setArrayAttributes(index, Attr_Data);
-        PropertyIndex arrayIndex{ a->arrayData(), a->arrayData()->values.values + a->arrayData()->mappedIndex(index) };
-        arrayIndex.set(scope.engine, a->d()->mappedArguments->values[index]);
+    if (attrs.isData() && desc->value.isEmpty() && attrs.hasWritable() && !attrs.isWritable()) {
+        cDesc->value = args->context()->args()[index];
+        cAttrs.setType(PropertyAttributes::Data);
     }
 
-    bool result = Object::virtualDefineOwnProperty(m, id, desc, attrs);
-    if (!result)
+    bool allowed = Object::virtualDefineOwnProperty(m, id, cDesc, cAttrs);
+    if (!allowed)
         return false;
 
-    if (isMapped && attrs.isData()) {
-        Q_ASSERT(a->arrayData());
-        ScopedFunctionObject setter(scope, map->setter());
-        JSCallData jsCallData(scope, 1);
-        *jsCallData->thisObject = a->asReturnedValue();
-        jsCallData->args[0] = desc->value;
-        setter->call(jsCallData);
-
-        if (attrs.isWritable()) {
-            a->setArrayAttributes(index, mapAttrs);
-            a->arrayData()->setProperty(m->engine(), index, map);
-        }
+    if (attrs.isAccessor()) {
+        args->removeMapping(index);
+    } else {
+        if (!desc->value.isEmpty())
+            args->context()->setArg(index, desc->value);
+        if (attrs.hasWritable() && !attrs.isWritable())
+            args->removeMapping(index);
     }
-
-    return result;
+    return true;
 }
 
 ReturnedValue ArgumentsObject::virtualGet(const Managed *m, PropertyKey id, const Value *receiver, bool *hasProperty)
 {
     const ArgumentsObject *args = static_cast<const ArgumentsObject *>(m);
-    if (id.isArrayIndex() && !args->fullyCreated()) {
-        uint index = id.asArrayIndex();
-        if (index < static_cast<uint>(args->context()->argc())) {
-            if (hasProperty)
-                *hasProperty = true;
-            return args->context()->args()[index].asReturnedValue();
-        }
+    uint index = id.asArrayIndex();
+    if (index < args->d()->argCount && !args->d()->fullyCreated) {
+        if (hasProperty)
+            *hasProperty = true;
+        return args->context()->args()[index].asReturnedValue();
     }
-    return Object::virtualGet(m, id, receiver, hasProperty);
+
+    if (!args->isMapped(index))
+        return Object::virtualGet(m, id, receiver, hasProperty);
+    Q_ASSERT(index < static_cast<uint>(args->context()->function->formalParameterCount()));
+    if (hasProperty)
+        *hasProperty = true;
+    return args->context()->args()[index].asReturnedValue();
 }
 
 bool ArgumentsObject::virtualPut(Managed *m, PropertyKey id, const Value &value, Value *receiver)
 {
     ArgumentsObject *args = static_cast<ArgumentsObject *>(m);
-    if (id.isArrayIndex()) {
-        uint index = id.asArrayIndex();
-        if (!args->fullyCreated() && index >= static_cast<uint>(args->context()->argc()))
-            args->fullyCreate();
-
-        if (!args->fullyCreated()) {
-            args->context()->setArg(index, value);
-            return true;
-        }
+    uint index = id.asArrayIndex();
+
+    if (args == receiver && index < args->d()->argCount && !args->d()->fullyCreated) {
+        args->context()->setArg(index, value);
+        return true;
     }
+
+    bool isMapped = (args == receiver && args->isMapped(index));
+    if (isMapped)
+        args->context()->setArg(index, value);
+
     return Object::virtualPut(m, id, value, receiver);
 }
 
 bool ArgumentsObject::virtualDeleteProperty(Managed *m, PropertyKey id)
 {
     ArgumentsObject *args = static_cast<ArgumentsObject *>(m);
-    if (!args->fullyCreated())
-        args->fullyCreate();
-    return Object::virtualDeleteProperty(m, id);
+    args->fullyCreate();
+    bool result = Object::virtualDeleteProperty(m, id);
+    if (result)
+        args->removeMapping(id.asArrayIndex());
+    return result;
 }
 
 PropertyAttributes ArgumentsObject::virtualGetOwnProperty(Managed *m, PropertyKey id, Property *p)
 {
     const ArgumentsObject *args = static_cast<const ArgumentsObject *>(m);
-    if (!id.isArrayIndex() || args->fullyCreated())
-        return Object::virtualGetOwnProperty(m, id, p);
-
     uint index = id.asArrayIndex();
-    uint argCount = args->context()->argc();
-    if (index >= argCount)
-        return PropertyAttributes();
-    if (p)
+    if (index < args->d()->argCount && !args->d()->fullyCreated) {
         p->value = args->context()->args()[index];
-    return Attr_Data;
-}
-
-DEFINE_OBJECT_VTABLE(ArgumentsGetterFunction);
-
-ReturnedValue ArgumentsGetterFunction::virtualCall(const FunctionObject *getter, const Value *thisObject, const Value *, int)
-{
-    ExecutionEngine *v4 = getter->engine();
-    Scope scope(v4);
-    const ArgumentsGetterFunction *g = static_cast<const ArgumentsGetterFunction *>(getter);
-    Scoped<ArgumentsObject> o(scope, thisObject->as<ArgumentsObject>());
-    if (!o)
-        return v4->throwTypeError();
-
-    Q_ASSERT(g->index() < static_cast<unsigned>(o->context()->argc()));
-    return o->context()->args()[g->index()].asReturnedValue();
-}
+        return Attr_Data;
+    }
 
-DEFINE_OBJECT_VTABLE(ArgumentsSetterFunction);
+    PropertyAttributes attrs = Object::virtualGetOwnProperty(m, id, p);
+    if (attrs.isEmpty() || !args->isMapped(index))
+        return attrs;
 
-ReturnedValue ArgumentsSetterFunction::virtualCall(const FunctionObject *setter, const Value *thisObject, const Value *argv, int argc)
-{
-    ExecutionEngine *v4 = setter->engine();
-    Scope scope(v4);
-    const ArgumentsSetterFunction *s = static_cast<const ArgumentsSetterFunction *>(setter);
-    Scoped<ArgumentsObject> o(scope, thisObject->as<ArgumentsObject>());
-    if (!o)
-        return v4->throwTypeError();
-
-    Q_ASSERT(s->index() < static_cast<unsigned>(o->context()->argc()));
-    o->context()->setArg(s->index(), argc ? argv[0] : Primitive::undefinedValue());
-    return Encode::undefined();
+    Q_ASSERT(index < static_cast<uint>(args->context()->function->formalParameterCount()));
+    if (p)
+        p->value = args->context()->args()[index];
+    return attrs;
 }
 
 qint64 ArgumentsObject::virtualGetLength(const Managed *m)
diff --git a/src/qml/jsruntime/qv4argumentsobject_p.h b/src/qml/jsruntime/qv4argumentsobject_p.h
index 9ac9ac5d8b..85939071ed 100644
--- a/src/qml/jsruntime/qv4argumentsobject_p.h
+++ b/src/qml/jsruntime/qv4argumentsobject_p.h
@@ -59,27 +59,12 @@ namespace QV4 {
 
 namespace Heap {
 
-#define ArgumentsGetterFunctionMembers(class, Member) \
-    Member(class, NoMark, uint, index)
-
-DECLARE_HEAP_OBJECT(ArgumentsGetterFunction, FunctionObject) {
-    DECLARE_MARKOBJECTS(ArgumentsGetterFunction);
-    inline void init(QV4::ExecutionContext *scope, uint index);
-};
-
-#define ArgumentsSetterFunctionMembers(class, Member) \
-    Member(class, NoMark, uint, index)
-
-DECLARE_HEAP_OBJECT(ArgumentsSetterFunction, FunctionObject) {
-    DECLARE_MARKOBJECTS(ArgumentsSetterFunction);
-    inline void init(QV4::ExecutionContext *scope, uint index);
-};
-
 #define ArgumentsObjectMembers(class, Member) \
     Member(class, Pointer, CallContext *, context) \
     Member(class, Pointer, MemberData *, mappedArguments) \
     Member(class, NoMark, bool, fullyCreated) \
-    Member(class, NoMark, int, nFormals)
+    Member(class, NoMark, uint, argCount) \
+    Member(class, NoMark, quint64, mapped)
 
 DECLARE_HEAP_OBJECT(ArgumentsObject, Object) {
     DECLARE_MARKOBJECTS(ArgumentsObject);
@@ -104,37 +89,6 @@ DECLARE_HEAP_OBJECT(StrictArgumentsObject, Object) {
 
 }
 
-struct ArgumentsGetterFunction: FunctionObject
-{
-    V4_OBJECT2(ArgumentsGetterFunction, FunctionObject)
-
-    uint index() const { return d()->index; }
-    static ReturnedValue virtualCall(const FunctionObject *f, const Value *thisObject, const Value *argv, int argc);
-};
-
-inline void
-Heap::ArgumentsGetterFunction::init(QV4::ExecutionContext *scope, uint index)
-{
-    Heap::FunctionObject::init(scope);
-    this->index = index;
-}
-
-struct ArgumentsSetterFunction: FunctionObject
-{
-    V4_OBJECT2(ArgumentsSetterFunction, FunctionObject)
-
-    uint index() const { return d()->index; }
-    static ReturnedValue virtualCall(const FunctionObject *f, const Value *thisObject, const Value *argv, int argc);
-};
-
-inline void
-Heap::ArgumentsSetterFunction::init(QV4::ExecutionContext *scope, uint index)
-{
-    Heap::FunctionObject::init(scope);
-    this->index = index;
-}
-
-
 struct ArgumentsObject: Object {
     V4_OBJECT2(ArgumentsObject, Object)
     Q_MANAGED_TYPE(ArgumentsObject)
@@ -155,6 +109,17 @@ struct ArgumentsObject: Object {
 
     void fullyCreate();
 
+    // There's a slight hack here, as this limits the amount of mapped arguments to 64, but that should be
+    // more than enough for all practical uses of arguments
+    bool isMapped(uint arg) const {
+        return arg < 64 && (d()->mapped & (1ull << arg));
+    }
+
+    void removeMapping(uint arg) {
+        if (arg < 64)
+            (d()->mapped &= ~(1ull << arg));
+    }
+
 };
 
 struct StrictArgumentsObject : Object {
diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp
index f9ef6b45a1..a776e7d14a 100644
--- a/src/qml/jsruntime/qv4engine.cpp
+++ b/src/qml/jsruntime/qv4engine.cpp
@@ -148,8 +148,6 @@ ExecutionEngine::ExecutionEngine(QJSEngine *jsEngine)
     , globalCode(nullptr)
     , v8Engine(nullptr)
     , publicEngine(jsEngine)
-    , argumentsAccessors(nullptr)
-    , nArgumentsAccessors(0)
     , m_engineId(engineSerial.fetchAndAddOrdered(1))
     , regExpCache(nullptr)
     , m_multiplyWrappedQObjects(nullptr)
@@ -611,7 +609,6 @@ ExecutionEngine::~ExecutionEngine()
     delete jsStack;
     gcStack->deallocate();
     delete gcStack;
-    delete [] argumentsAccessors;
 }
 
 ExecutionContext *ExecutionEngine::currentContext() const
@@ -982,42 +979,8 @@ QUrl ExecutionEngine::resolvedUrl(const QString &file)
     return base.resolved(src);
 }
 
-void ExecutionEngine::requireArgumentsAccessors(int n)
-{
-    if (n <= nArgumentsAccessors)
-        return;
-
-    Scope scope(this);
-    ScopedFunctionObject get(scope);
-    ScopedFunctionObject set(scope);
-
-    if (n >= nArgumentsAccessors) {
-        Property *oldAccessors = argumentsAccessors;
-        int oldSize = nArgumentsAccessors;
-        nArgumentsAccessors = qMax(8, n);
-        argumentsAccessors = new Property[nArgumentsAccessors];
-        if (oldAccessors) {
-            memcpy(static_cast<void *>(argumentsAccessors), static_cast<const void *>(oldAccessors), oldSize*sizeof(Property));
-            delete [] oldAccessors;
-        }
-        ExecutionContext *global = rootContext();
-        for (int i = oldSize; i < nArgumentsAccessors; ++i) {
-            argumentsAccessors[i].value = ScopedValue(scope, memoryManager->allocate<ArgumentsGetterFunction>(global, i));
-            argumentsAccessors[i].set = ScopedValue(scope, memoryManager->allocate<ArgumentsSetterFunction>(global, i));
-        }
-    }
-}
-
 void ExecutionEngine::markObjects(MarkStack *markStack)
 {
-    for (int i = 0; i < nArgumentsAccessors; ++i) {
-        const Property &pd = argumentsAccessors[i];
-        if (Heap::FunctionObject *getter = pd.getter())
-            getter->mark(markStack);
-        if (Heap::FunctionObject *setter = pd.setter())
-            setter->mark(markStack);
-    }
-
     for (int i = 0; i < NClasses; ++i)
         if (classes[i])
             classes[i]->mark(markStack);
diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h
index fc0c93eaad..e1ea89c699 100644
--- a/src/qml/jsruntime/qv4engine_p.h
+++ b/src/qml/jsruntime/qv4engine_p.h
@@ -276,9 +276,6 @@ public:
     FunctionObject *getStackFunction() const { return reinterpret_cast<FunctionObject *>(jsObjects + GetStack_Function); }
     FunctionObject *thrower() const { return reinterpret_cast<FunctionObject *>(jsObjects + ThrowerObject); }
 
-    Property *argumentsAccessors;
-    int nArgumentsAccessors;
-
     enum JSStrings {
         String_Empty,
         String_undefined,
@@ -505,8 +502,6 @@ public:
     StackTrace stackTrace(int frameLimit = -1) const;
     QUrl resolvedUrl(const QString &file);
 
-    void requireArgumentsAccessors(int n);
-
     void markObjects(MarkStack *markStack);
 
     void initRootContext();
diff --git a/src/qml/jsruntime/qv4objectproto.cpp b/src/qml/jsruntime/qv4objectproto.cpp
index 65e9b836d1..47f101e4e4 100644
--- a/src/qml/jsruntime/qv4objectproto.cpp
+++ b/src/qml/jsruntime/qv4objectproto.cpp
@@ -756,8 +756,6 @@ void ObjectPrototype::toPropertyDescriptor(ExecutionEngine *engine, const Value
             return;
         }
         attrs->setWritable((tmp = o->get(engine->id_writable()))->toBoolean());
-        // writable forces it to be a data descriptor
-        desc->value = Primitive::undefinedValue();
     }
 
     if (o->hasProperty(engine->id_value()->toPropertyKey())) {
diff --git a/tests/auto/qml/ecmascripttests/TestExpectations b/tests/auto/qml/ecmascripttests/TestExpectations
index 6b220b963f..b1d7117325 100644
--- a/tests/auto/qml/ecmascripttests/TestExpectations
+++ b/tests/auto/qml/ecmascripttests/TestExpectations
@@ -1865,30 +1865,6 @@ built-ins/global/global-object.js fails
 built-ins/global/property-descriptor.js fails
 built-ins/isFinite/toprimitive-not-callable-throws.js fails
 built-ins/isNaN/toprimitive-not-callable-throws.js fails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-2.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-3.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-4.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-2.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-3.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-delete-4.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-1.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-3.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-4.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-nonwritable-5.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-2.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-3.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonconfigurable-strict-delete-4.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonwritable-nonconfigurable-1.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonwritable-nonconfigurable-3.js sloppyFails
-language/arguments-object/mapped/mapped-arguments-nonwritable-nonconfigurable-4.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-descriptors-set-value-by-arguments.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-descriptors-set-value-with-define-property.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-descriptors-with-param-assign.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-nonenumerable-nonwritable-descriptors-set-by-arguments.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-nonenumerable-nonwritable-descriptors-set-by-param.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-nonwritable-descriptors-set-by-arguments.js sloppyFails
-language/arguments-object/mapped/nonconfigurable-nonwritable-descriptors-set-by-param.js sloppyFails
-language/arguments-object/mapped/nonwritable-nonconfigurable-descriptors-set-by-arguments.js sloppyFails
 language/computed-property-names/class/static/generator-prototype.js fails
 language/computed-property-names/class/static/getter-prototype.js fails
 language/computed-property-names/class/static/method-prototype.js fails