Cleanup argument handling in contexts

Fix the compiler to already deal with duplicated argument names.
Doing this at runtime was not ideal.

Remove the callData member from the context. Instead use the fact that
the arguments already followed the locals in the context. Don't copy the
thisObject over into the CallContext anymore, it's never used from there
anyway.

Fix the ordering of names in the internalclass, so that arguments don't
require special handling anymore when looking them up by name.

Adjust all places that used callData, and related methods.

Change-Id: I0bc45e1be3f1fcd38dc3b4f04e91edaf7b9ed103
Reviewed-by: Erik Verbruggen <erik.verbruggen@qt.io>

7 files changed, 67 insertions, 104 deletions

diff --git a/src/qml/compiler/qv4compilerscanfunctions.cpp b/src/qml/compiler/qv4compilerscanfunctions.cpp
index b59beb0315..8c41d89bd4 100644
--- a/src/qml/compiler/qv4compilerscanfunctions.cpp
+++ b/src/qml/compiler/qv4compilerscanfunctions.cpp
@@ -408,11 +408,20 @@ void ScanFunctions::enterFunction(Node *ast, const QString &name, FormalParamete
 
     for (FormalParameterList *it = formals; it; it = it->next) {
         QString arg = it->name.toString();
-        if (_context->isStrict) {
-            if (_context->arguments.contains(arg)) {
+        int duplicateIndex = _context->arguments.indexOf(arg);
+        if (duplicateIndex != -1) {
+            if (_context->isStrict) {
                 _cg->throwSyntaxError(it->identifierToken, QStringLiteral("Duplicate parameter name '%1' is not allowed in strict mode").arg(arg));
                 return;
+            } else {
+                // change the name of the earlier argument to enforce the specified lookup semantics
+                QString modified = arg;
+                while (_context->arguments.contains(modified))
+                    modified += QString(0xfffe);
+                _context->arguments[duplicateIndex] = modified;
             }
+        }
+        if (_context->isStrict) {
             if (arg == QLatin1String("eval") || arg == QLatin1String("arguments")) {
                 _cg->throwSyntaxError(it->identifierToken, QStringLiteral("'%1' cannot be used as parameter name in strict mode").arg(arg));
                 return;
diff --git a/src/qml/jsruntime/qv4argumentsobject.cpp b/src/qml/jsruntime/qv4argumentsobject.cpp
index 0d2d5fb072..c91af88c30 100644
--- a/src/qml/jsruntime/qv4argumentsobject.cpp
+++ b/src/qml/jsruntime/qv4argumentsobject.cpp
@@ -75,7 +75,7 @@ void Heap::ArgumentsObject::init(QV4::CallContext *context, bool strict)
         args->setProperty(CalleePropertyIndex, context->d()->function);
     }
     Q_ASSERT(LengthPropertyIndex == args->internalClass()->find(v4->id_length()));
-    args->setProperty(LengthPropertyIndex, Primitive::fromInt32(context->d()->callData->argc));
+    args->setProperty(LengthPropertyIndex, Primitive::fromInt32(context->argc()));
 }
 
 void ArgumentsObject::fullyCreate()
@@ -85,7 +85,7 @@ void ArgumentsObject::fullyCreate()
 
     Scope scope(engine());
 
-    uint argCount = context()->callData->argc;
+    uint argCount = context()->argc();
     uint numAccessors = qMin(context()->formalParameterCount(), argCount);
     ArrayData::realloc(this, Heap::ArrayData::Sparse, argCount, true);
     scope.engine->requireArgumentsAccessors(numAccessors);
@@ -94,11 +94,11 @@ void ArgumentsObject::fullyCreate()
     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()->callData->args[i]);
+            d()->mappedArguments->values.set(scope.engine, i, context()->args()[i]);
             arraySet(i, scope.engine->argumentsAccessors + i, Attr_Accessor);
         }
     }
-    arrayPut(numAccessors, context()->callData->args + numAccessors, argCount - numAccessors);
+    arrayPut(numAccessors, context()->args() + numAccessors, argCount - numAccessors);
     for (uint i = numAccessors; i < argCount; ++i)
         setArrayAttributes(i, Attr_Data);
 
@@ -112,7 +112,7 @@ bool ArgumentsObject::defineOwnProperty(ExecutionEngine *engine, uint index, con
     Scope scope(engine);
     ScopedProperty map(scope);
     PropertyAttributes mapAttrs;
-    uint numAccessors = qMin(context()->formalParameterCount(), static_cast<uint>(context()->callData->argc));
+    uint numAccessors = qMin(context()->formalParameterCount(), static_cast<uint>(context()->argc()));
     bool isMapped = false;
     if (arrayData() && index < numAccessors &&
         arrayData()->attributes(index).isAccessor() &&
@@ -158,10 +158,10 @@ ReturnedValue ArgumentsObject::getIndexed(const Managed *m, uint index, bool *ha
     if (args->fullyCreated())
         return Object::getIndexed(m, index, hasProperty);
 
-    if (index < static_cast<uint>(args->context()->callData->argc)) {
+    if (index < static_cast<uint>(args->context()->argc())) {
         if (hasProperty)
             *hasProperty = true;
-        return args->context()->callData->args[index].asReturnedValue();
+        return args->context()->args()[index].asReturnedValue();
     }
     if (hasProperty)
         *hasProperty = false;
@@ -171,13 +171,13 @@ ReturnedValue ArgumentsObject::getIndexed(const Managed *m, uint index, bool *ha
 bool ArgumentsObject::putIndexed(Managed *m, uint index, const Value &value)
 {
     ArgumentsObject *args = static_cast<ArgumentsObject *>(m);
-    if (!args->fullyCreated() && index >= static_cast<uint>(args->context()->callData->argc))
+    if (!args->fullyCreated() && index >= static_cast<uint>(args->context()->argc()))
         args->fullyCreate();
 
     if (args->fullyCreated())
         return Object::putIndexed(m, index, value);
 
-    args->context()->callData->args[index] = value;
+    args->context()->setArg(index, value);
     return true;
 }
 
@@ -195,8 +195,8 @@ PropertyAttributes ArgumentsObject::queryIndexed(const Managed *m, uint index)
     if (args->fullyCreated())
         return Object::queryIndexed(m, index);
 
-    uint numAccessors = qMin((int)args->context()->formalParameterCount(), args->context()->callData->argc);
-    uint argCount = args->context()->callData->argc;
+    uint numAccessors = qMin((int)args->context()->formalParameterCount(), args->context()->argc());
+    uint argCount = args->context()->argc();
     if (index >= argCount)
         return PropertyAttributes();
     if (index >= numAccessors)
@@ -215,8 +215,8 @@ ReturnedValue ArgumentsGetterFunction::call(const Managed *getter, CallData *cal
     if (!o)
         return v4->throwTypeError();
 
-    Q_ASSERT(g->index() < static_cast<unsigned>(o->context()->callData->argc));
-    return o->context()->callData->args[g->index()].asReturnedValue();
+    Q_ASSERT(g->index() < static_cast<unsigned>(o->context()->argc()));
+    return o->context()->args()[g->index()].asReturnedValue();
 }
 
 DEFINE_OBJECT_VTABLE(ArgumentsSetterFunction);
@@ -230,8 +230,8 @@ ReturnedValue ArgumentsSetterFunction::call(const Managed *setter, CallData *cal
     if (!o)
         return v4->throwTypeError();
 
-    Q_ASSERT(s->index() < static_cast<unsigned>(o->context()->callData->argc));
-    o->context()->callData->args[s->index()] = callData->argc ? callData->args[0].asReturnedValue() : Encode::undefined();
+    Q_ASSERT(s->index() < static_cast<unsigned>(o->context()->argc()));
+    o->context()->setArg(s->index(), (callData->argc ? callData->args[0] : Primitive::undefinedValue()));
     return Encode::undefined();
 }
 
diff --git a/src/qml/jsruntime/qv4context.cpp b/src/qml/jsruntime/qv4context.cpp
index fbcfcc3460..eaf54282e7 100644
--- a/src/qml/jsruntime/qv4context.cpp
+++ b/src/qml/jsruntime/qv4context.cpp
@@ -58,7 +58,7 @@ DEFINE_MANAGED_VTABLE(CatchContext);
 Heap::CallContext *ExecutionContext::newCallContext(Heap::ExecutionContext *outer, Function *function, CallData *callData, const FunctionObject *f)
 {
     uint nFormals = qMax(static_cast<uint>(callData->argc), function->nFormals);
-    uint localsAndFormals = function->compiledFunction->nLocals + sizeof(CallData)/sizeof(Value) - 1 + nFormals;
+    uint localsAndFormals = function->compiledFunction->nLocals + nFormals;
     size_t requiredMemory = sizeof(CallContext::Data) - sizeof(Value) + sizeof(Value) * (localsAndFormals);
 
     ExecutionEngine *v4 = outer->internalClass->engine;
@@ -83,8 +83,8 @@ Heap::CallContext *ExecutionContext::newCallContext(Heap::ExecutionContext *oute
         std::fill(c->locals.values, c->locals.values + nLocals, Primitive::undefinedValue());
 #endif
 
-    c->callData = reinterpret_cast<CallData *>(c->locals.values + nLocals);
-    ::memcpy(c->callData, callData, sizeof(CallData) - sizeof(Value) + nFormals * sizeof(Value));
+    ::memcpy(c->locals.values + nLocals, &callData->args[0], nFormals * sizeof(Value));
+    c->nArgs = callData->argc;
 
     return c;
 }
@@ -161,28 +161,6 @@ void Heap::CatchContext::init(ExecutionContext *outerContext, String *exceptionV
     this->exceptionValue.set(internalClass->engine, exceptionValue);
 }
 
-Identifier * const *CallContext::formals() const
-{
-    return d()->v4Function ? d()->internalClass->nameMap.constData() : 0;
-}
-
-unsigned int CallContext::formalCount() const
-{
-    return d()->v4Function ? d()->v4Function->nFormals : 0;
-}
-
-Identifier * const *CallContext::variables() const
-{
-    return d()->v4Function ? d()->internalClass->nameMap.constData() + d()->v4Function->nFormals : 0;
-}
-
-unsigned int CallContext::variableCount() const
-{
-    return d()->v4Function ? d()->v4Function->compiledFunction->nLocals : 0;
-}
-
-
-
 bool ExecutionContext::deleteProperty(String *name)
 {
     name->makeIdentifier();
@@ -257,13 +235,7 @@ ExecutionContext::Error ExecutionContext::setProperty(String *name, const Value
             if (c->v4Function) {
                 uint index = c->internalClass->find(id);
                 if (index < UINT_MAX) {
-                    if (index < c->v4Function->nFormals) {
-                        c->callData->args[c->v4Function->nFormals - index - 1] = value;
-                    } else {
-                        Q_ASSERT(c->type == Heap::ExecutionContext::Type_CallContext);
-                        index -= c->v4Function->nFormals;
-                        static_cast<Heap::CallContext *>(c)->locals.set(v4, index, value);
-                    }
+                    static_cast<Heap::CallContext *>(c)->locals.set(v4, index, value);
                     return NoError;
                 }
             }
@@ -314,12 +286,8 @@ ReturnedValue ExecutionContext::getProperty(String *name)
             Identifier *id = name->identifier();
 
             uint index = c->internalClass->find(id);
-            if (index < UINT_MAX) {
-                if (index < c->v4Function->nFormals)
-                    return c->callData->args[c->v4Function->nFormals - index - 1].asReturnedValue();
-                Q_ASSERT(c->type == Heap::ExecutionContext::Type_CallContext);
-                return c->locals[index - c->v4Function->nFormals].asReturnedValue();
-            }
+            if (index < UINT_MAX)
+                return c->locals[index].asReturnedValue();
             if (c->v4Function->isNamedExpression()) {
                 Scope scope(this);
                 if (c->function && name->equals(ScopedString(scope, c->v4Function->name())))
@@ -365,11 +333,8 @@ ReturnedValue ExecutionContext::getPropertyAndBase(String *name, Value *base)
             Identifier *id = name->identifier();
 
             uint index = c->internalClass->find(id);
-            if (index < UINT_MAX) {
-                if (index < c->v4Function->nFormals)
-                    return c->callData->args[c->v4Function->nFormals - index - 1].asReturnedValue();
-                return c->locals[index - c->v4Function->nFormals].asReturnedValue();
-            }
+            if (index < UINT_MAX)
+                return c->locals[index].asReturnedValue();
             if (c->v4Function->isNamedExpression()) {
                 Scope scope(this);
                 if (c->function && name->equals(ScopedString(scope, c->v4Function->name())))
@@ -421,3 +386,9 @@ Function *ExecutionContext::getFunction() const
 
     return 0;
 }
+
+
+void Heap::CallContext::setArg(uint index, Value v)
+{
+    locals.set(internalClass->engine, locals.size + index, v);
+}
diff --git a/src/qml/jsruntime/qv4context_p.h b/src/qml/jsruntime/qv4context_p.h
index dfe0645f9d..57ba29c228 100644
--- a/src/qml/jsruntime/qv4context_p.h
+++ b/src/qml/jsruntime/qv4context_p.h
@@ -125,11 +125,10 @@ DECLARE_HEAP_OBJECT(ExecutionContext, Base) {
         type = t;
     }
 
-    quint8 type;
+    quint32 type : 8;
+    quint32 nArgs : 24;
 #if QT_POINTER_SIZE == 8
-    quint8 padding_[7];
-#else
-    quint8 padding_[3];
+    quint8 padding_[4];
 #endif
 };
 V4_ASSERT_IS_TRIVIAL(ExecutionContext)
@@ -141,7 +140,6 @@ Q_STATIC_ASSERT(offsetof(ExecutionContextData, activation) == offsetof(Execution
 Q_STATIC_ASSERT(offsetof(ExecutionContextData, v4Function) == offsetof(ExecutionContextData, activation) + QT_POINTER_SIZE);
 
 #define CallContextMembers(class, Member) \
-    Member(class, NoMark, CallData *, callData)  \
     Member(class, Pointer, FunctionObject *, function) \
     Member(class, ValueArray, ValueArray, locals)
 
@@ -153,13 +151,19 @@ DECLARE_HEAP_OBJECT(CallContext, ExecutionContext) {
         ExecutionContext::init(Type_CallContext);
     }
 
-    inline unsigned int formalParameterCount() const;
+    int argc() const {
+        return static_cast<int>(nArgs);
+    }
+    const Value *args() const {
+        return &locals[locals.size];
+    }
+    void setArg(uint index, Value v);
 
+    inline unsigned int formalParameterCount() const;
 };
 V4_ASSERT_IS_TRIVIAL(CallContext)
 Q_STATIC_ASSERT(std::is_standard_layout<CallContextData>::value);
-Q_STATIC_ASSERT(offsetof(CallContextData, callData) == 0);
-Q_STATIC_ASSERT(offsetof(CallContextData, function) == offsetof(CallContextData, callData) + QT_POINTER_SIZE);
+Q_STATIC_ASSERT(offsetof(CallContextData, function) == 0);
 Q_STATIC_ASSERT(offsetof(CallContextData, locals) == offsetof(CallContextData, function) + QT_POINTER_SIZE);
 //### The following size check fails on Win8. With the ValueArray at the end of the
 // CallContextMembers, it doesn't look very useful.
@@ -222,23 +226,11 @@ struct Q_QML_EXPORT CallContext : public ExecutionContext
     V4_MANAGED(CallContext, ExecutionContext)
     V4_INTERNALCLASS(CallContext)
 
-    // formals are in reverse order
-    Identifier * const *formals() const;
-    unsigned int formalCount() const;
-    Identifier * const *variables() const;
-    unsigned int variableCount() const;
-
-    Value &thisObject() const {
-        return d()->callData->thisObject;
-    }
     int argc() const {
-        return d()->callData->argc;
+        return d()->argc();
     }
     const Value *args() const {
-        return d()->callData->args;
-    }
-    ReturnedValue argument(int i) const {
-        return d()->callData->argument(i);
+        return d()->args();
     }
 };
 
diff --git a/src/qml/jsruntime/qv4function.cpp b/src/qml/jsruntime/qv4function.cpp
index 6a07c49478..c6533054ce 100644
--- a/src/qml/jsruntime/qv4function.cpp
+++ b/src/qml/jsruntime/qv4function.cpp
@@ -60,29 +60,19 @@ Function::Function(ExecutionEngine *engine, CompiledData::CompilationUnit *unit,
     Q_UNUSED(engine);
 
     internalClass = engine->internalClasses[EngineBase::Class_CallContext];
-    const CompiledData::LEUInt32 *formalsIndices = compiledFunction->formalsTable();
-    // iterate backwards, so we get the right ordering for duplicate names
-    Scope scope(engine);
-    ScopedString arg(scope);
-    for (int i = static_cast<int>(compiledFunction->nFormals - 1); i >= 0; --i) {
-        arg = compilationUnit->runtimeStrings[formalsIndices[i]];
-        while (1) {
-            InternalClass *newClass = internalClass->addMember(arg, Attr_NotConfigurable);
-            if (newClass != internalClass) {
-                internalClass = newClass;
-                break;
-            }
-            // duplicate arguments, need some trick to store them
-            MemoryManager *mm = engine->memoryManager;
-            arg = mm->alloc<String>(arg->d(), engine->newString(QString(0xfffe)));
-        }
-    }
-    nFormals = compiledFunction->nFormals;
 
+    // first locals
     const CompiledData::LEUInt32 *localsIndices = compiledFunction->localsTable();
     for (quint32 i = 0; i < compiledFunction->nLocals; ++i)
         internalClass = internalClass->addMember(compilationUnit->runtimeStrings[localsIndices[i]]->identifier, Attr_NotConfigurable);
 
+    const CompiledData::LEUInt32 *formalsIndices = compiledFunction->formalsTable();
+    for (quint32 i = 0; i < compiledFunction->nFormals; ++i)
+        internalClass = internalClass->addMember(compilationUnit->runtimeStrings[formalsIndices[i]]->identifier, Attr_NotConfigurable);
+
+    nFormals = compiledFunction->nFormals;
+
+
     canUseSimpleCall = compiledFunction->flags & CompiledData::Function::CanUseSimpleCall;
 }
 
diff --git a/src/qml/jsruntime/qv4functionobject.cpp b/src/qml/jsruntime/qv4functionobject.cpp
index 27810dc836..a47ea43965 100644
--- a/src/qml/jsruntime/qv4functionobject.cpp
+++ b/src/qml/jsruntime/qv4functionobject.cpp
@@ -290,8 +290,8 @@ ReturnedValue FunctionPrototype::method_apply(const BuiltinFunction *b, CallData
     if (len) {
         if (ArgumentsObject::isNonStrictArgumentsObject(arr) && !arr->cast<ArgumentsObject>()->fullyCreated()) {
             QV4::ArgumentsObject *a = arr->cast<ArgumentsObject>();
-            int l = qMin(len, (uint)a->d()->context->callData->argc);
-            memcpy(cData->args, a->d()->context->callData->args, l*sizeof(Value));
+            int l = qMin(len, (uint)a->d()->context->argc());
+            memcpy(cData->args, a->d()->context->args(), l*sizeof(Value));
             for (quint32 i = l; i < len; ++i)
                 cData->args[i] = Primitive::undefinedValue();
         } else if (arr->arrayType() == Heap::ArrayData::Simple && !arr->protoHasArray()) {
diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp
index 1189f0167f..20b38823f9 100644
--- a/src/qml/jsruntime/qv4vme_moth.cpp
+++ b/src/qml/jsruntime/qv4vme_moth.cpp
@@ -397,17 +397,18 @@ static inline ReturnedValue loadScopedArg(CppStackFrame &frame, int index, int s
     auto ctxt = getScope(frame.jsFrame, scope);
     Q_ASSERT(ctxt->type == QV4::Heap::ExecutionContext::Type_CallContext);
     auto cc = static_cast<Heap::CallContext *>(ctxt);
-    return cc->callData->args[index].asReturnedValue();
+    return cc->args()[index].asReturnedValue();
 }
 
 static inline void storeScopedArg(ExecutionEngine *engine, CppStackFrame &frame, int index, int scope,
                                   const QV4::Value &value)
 {
+    Q_UNUSED(engine);
     auto ctxt = getScope(frame.jsFrame, scope);
     Q_ASSERT(ctxt->type == QV4::Heap::ExecutionContext::Type_CallContext);
     auto cc = static_cast<Heap::CallContext *>(ctxt);
 
-    QV4::WriteBarrier::write(engine, cc, cc->callData->args + index, value);
+    cc->setArg(index, value);
 }
 
 static inline const QV4::Value &constant(Function *function, int index)