From bdb8cf49daf4a1c7dfb9bd9caf14e19e3aa66293 Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Thu, 31 May 2018 14:22:35 +0200 Subject: Fix creation of object literals Our method to create object literals wasn't compliant with the ES7 spec, as we would in some cases re-order the properties. This violated the spec which required properties to be created in order, so that for-of would also iterate over them in creation order. As a nice side effect, this simplifies the code and gets a couple of test cases using computed property names to pass. Task-number: QTBUG-62512 Change-Id: I6dfe004357c5d46a0890027f4fd9e2d1e1a2a17a Reviewed-by: Simon Hausmann --- src/qml/compiler/qv4codegen.cpp | 178 ++++++++++++++---------------------- src/qml/compiler/qv4compiler.cpp | 24 +---- src/qml/compiler/qv4compiler_p.h | 3 +- src/qml/compiler/qv4instr_moth.cpp | 7 +- src/qml/compiler/qv4instr_moth_p.h | 3 +- src/qml/jit/qv4baselinejit.cpp | 12 +-- src/qml/jit/qv4baselinejit_p.h | 4 +- src/qml/jsruntime/qv4global_p.h | 6 ++ src/qml/jsruntime/qv4runtime.cpp | 54 +++++------ src/qml/jsruntime/qv4runtimeapi_p.h | 2 +- src/qml/jsruntime/qv4value_p.h | 1 - src/qml/jsruntime/qv4vme_moth.cpp | 2 +- 12 files changed, 117 insertions(+), 179 deletions(-) (limited to 'src') diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index 8d0d0695f5..9d15e2ef6b 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -1944,138 +1944,94 @@ bool Codegen::visit(ObjectPattern *ast) RegisterScope scope(this); - for (PatternPropertyList *it = ast->properties; it; it = it->next) { + QStringList members; + + int argc = 0; + int args = 0; + auto push = [this, &args, &argc](const Reference &arg) { + int temp = bytecodeGenerator->newRegister(); + if (argc == 0) + args = temp; + (void) arg.storeOnStack(temp); + ++argc; + }; + + PatternPropertyList *it = ast->properties; + for (; it; it = it->next) { PatternProperty *p = it->property; AST::ComputedPropertyName *cname = AST::cast(p->name); - if (cname) { - Reference name = expression(cname->expression); - if (hasError) - return false; - name = name.storeOnStack(); - computedProperties.append({name, ObjectPropertyValue()}); - } + if (cname || p->type == PatternProperty::Getter || p->type == PatternProperty::Setter) + break; QString name = p->name->asString(); - ObjectPropertyValue &v = cname ? computedProperties.last().second : valueMap[name]; - if (p->type == PatternProperty::Literal) { + uint arrayIndex = QV4::String::toArrayIndex(name); + if (arrayIndex != UINT_MAX) + break; + if (members.contains(name)) + break; + members.append(name); + + { + RegisterScope innerScope(this); Reference value = expression(p->initializer); if (hasError) return false; - - v.rvalue = value.storeOnStack(); - v.getter = v.setter = -1; - } else if (p->type == PatternProperty::Getter || p->type == PatternProperty::Setter) { - FunctionExpression *f = AST::cast(p->initializer); - Q_ASSERT(f); - const int function = defineFunction(name, f, f->formals, f->body); - v.rvalue = Reference(); - if (p->type == PatternProperty::Getter) - v.getter = function; - else - v.setter = function; - } else { - Q_UNREACHABLE(); + value.loadInAccumulator(); } + push(Reference::fromAccumulator(this)); } - QVector nonArrayKey, arrayKeyWithValue, arrayKeyWithGetterSetter; - bool needSparseArray = false; // set to true if any array index is bigger than 16 - - for (QMap::iterator it = valueMap.begin(), eit = valueMap.end(); - it != eit; ++it) { - QString name = it.key(); - uint keyAsIndex = QV4::String::toArrayIndex(name); - if (keyAsIndex != std::numeric_limits::max()) { - it->keyAsIndex = keyAsIndex; - if (keyAsIndex > 16) - needSparseArray = true; - if (it->hasSetter() || it->hasGetter()) - arrayKeyWithGetterSetter.append(name); - else - arrayKeyWithValue.append(name); - } else { - nonArrayKey.append(name); - } - } + int classId = jsUnitGenerator->registerJSClass(members); - int args = -1; - auto push = [this, &args](const Reference &arg) { - int temp = bytecodeGenerator->newRegister(); - if (args == -1) - args = temp; - (void) arg.storeOnStack(temp); - }; + // handle complex property setters + for (; it; it = it->next) { + PatternProperty *p = it->property; + AST::ComputedPropertyName *cname = AST::cast(p->name); + ObjectLiteralArgument argType = ObjectLiteralArgument::Value; + if (p->type == PatternProperty::Getter) + argType = ObjectLiteralArgument::Getter; + else if (p->type == PatternProperty::Setter) + argType = ObjectLiteralArgument::Setter; - QVector members; + Reference::fromConst(this, Encode(int(argType))).loadInAccumulator(); + push(Reference::fromAccumulator(this)); - Reference acc = Reference::fromAccumulator(this); - // generate the key/value pairs - for (const QString &key : qAsConst(nonArrayKey)) { - const ObjectPropertyValue &prop = valueMap[key]; - - if (prop.hasGetter() || prop.hasSetter()) { - Q_ASSERT(!prop.rvalue.isValid()); - loadClosure(prop.getter); - push(acc); - loadClosure(prop.setter); - push(acc); - members.append({ key, true }); + if (cname) { + RegisterScope innerScope(this); + Reference name = expression(cname->expression); + if (hasError) + return false; + name.loadInAccumulator(); } else { - Q_ASSERT(prop.rvalue.isValid()); - push(prop.rvalue); - members.append({ key, false }); + QString name = p->name->asString(); +#if 0 + uint arrayIndex = QV4::String::toArrayIndex(name); + if (arrayIndex != UINT_MAX) { + Reference::fromConst(this, Encode(arrayIndex)).loadInAccumulator(); + } else +#endif + { + Instruction::LoadRuntimeString instr; + instr.stringId = registerString(name); + bytecodeGenerator->addInstruction(instr); + } } + push(Reference::fromAccumulator(this)); + { + RegisterScope innerScope(this); + Reference value = expression(p->initializer); + if (hasError) + return false; + value.loadInAccumulator(); + } + push(Reference::fromAccumulator(this)); } - // generate array entries with values - for (const QString &key : qAsConst(arrayKeyWithValue)) { - const ObjectPropertyValue &prop = valueMap[key]; - Q_ASSERT(!prop.hasGetter() && !prop.hasSetter()); - push(Reference::fromConst(this, Encode(prop.keyAsIndex))); - push(prop.rvalue); - } - - // generate array entries with both a value and a setter - for (const QString &key : qAsConst(arrayKeyWithGetterSetter)) { - const ObjectPropertyValue &prop = valueMap[key]; - Q_ASSERT(!prop.rvalue.isValid()); - push(Reference::fromConst(this, Encode(prop.keyAsIndex))); - loadClosure(prop.getter); - push(acc); - loadClosure(prop.setter); - push(acc); - } - - int classId = jsUnitGenerator->registerJSClass(members); - - uint arrayGetterSetterCountAndFlags = arrayKeyWithGetterSetter.size(); - arrayGetterSetterCountAndFlags |= needSparseArray << 30; - - if (args == -1) - args = 0; - Instruction::DefineObjectLiteral call; call.internalClassId = classId; - call.arrayValueCount = arrayKeyWithValue.size(); - call.arrayGetterSetterCountAndFlags = arrayGetterSetterCountAndFlags; + call.argc = argc; call.args = Moth::StackSlot::createRegister(args); bytecodeGenerator->addInstruction(call); Reference result = Reference::fromAccumulator(this); - - if (!computedProperties.isEmpty()) { - result = result.storeOnStack(); - for (const auto &c : qAsConst(computedProperties)) { - // ### if RHS is an anonymous FunctionExpression, we need to set it's name to the computed name - Reference element = Reference::fromSubscript(result, c.first); - if (c.second.getter >= 0 || c.second.setter >= 0) { - throwSyntaxError(ast->firstSourceLocation(), QLatin1String("getter/setter with computed property names unimplemented.")); - return false;// ### - } else { - c.second.rvalue.loadInAccumulator(); - element.storeConsumeAccumulator(); - } - } - } - _expr.setResult(result); return false; } diff --git a/src/qml/compiler/qv4compiler.cpp b/src/qml/compiler/qv4compiler.cpp index 9b60ad14e6..d5a02a3ac8 100644 --- a/src/qml/compiler/qv4compiler.cpp +++ b/src/qml/compiler/qv4compiler.cpp @@ -188,7 +188,7 @@ QV4::ReturnedValue QV4::Compiler::JSUnitGenerator::constant(int idx) return constants.at(idx); } -int QV4::Compiler::JSUnitGenerator::registerJSClass(const QVector &members) +int QV4::Compiler::JSUnitGenerator::registerJSClass(const QStringList &members) { // ### re-use existing class definitions. @@ -202,31 +202,15 @@ int QV4::Compiler::JSUnitGenerator::registerJSClass(const QVector &m jsClass->nMembers = members.size(); CompiledData::JSClassMember *member = reinterpret_cast(jsClass + 1); - for (const MemberInfo &memberInfo : members) { - member->nameOffset = registerString(memberInfo.name); - member->isAccessor = memberInfo.isAccessor; + for (const auto &name : members) { + member->nameOffset = registerString(name); + member->isAccessor = false; ++member; } return jsClassOffsets.size() - 1; } -int QV4::Compiler::JSUnitGenerator::registerJSClass(int count, CompiledData::JSClassMember *members) -{ - const int size = CompiledData::JSClass::calculateSize(count); - jsClassOffsets.append(jsClassData.size()); - const int oldSize = jsClassData.size(); - jsClassData.resize(jsClassData.size() + size); - memset(jsClassData.data() + oldSize, 0, size); - - CompiledData::JSClass *jsClass = reinterpret_cast(jsClassData.data() + oldSize); - jsClass->nMembers = count; - CompiledData::JSClassMember *jsClassMembers = reinterpret_cast(jsClass + 1); - memcpy(jsClassMembers, members, sizeof(CompiledData::JSClassMember)*count); - - return jsClassOffsets.size() - 1; -} - QV4::CompiledData::Unit *QV4::Compiler::JSUnitGenerator::generateUnit(GeneratorOption option) { registerString(module->fileName); diff --git a/src/qml/compiler/qv4compiler_p.h b/src/qml/compiler/qv4compiler_p.h index bd330d3353..c40d49213a 100644 --- a/src/qml/compiler/qv4compiler_p.h +++ b/src/qml/compiler/qv4compiler_p.h @@ -116,8 +116,7 @@ struct Q_QML_PRIVATE_EXPORT JSUnitGenerator { int registerConstant(ReturnedValue v); ReturnedValue constant(int idx); - int registerJSClass(const QVector &members); - int registerJSClass(int count, CompiledData::JSClassMember *members); + int registerJSClass(const QStringList &members); enum GeneratorOption { GenerateWithStringTable, diff --git a/src/qml/compiler/qv4instr_moth.cpp b/src/qml/compiler/qv4instr_moth.cpp index 95bc459df0..c50735b61e 100644 --- a/src/qml/compiler/qv4instr_moth.cpp +++ b/src/qml/compiler/qv4instr_moth.cpp @@ -470,10 +470,9 @@ void dumpBytecode(const char *code, int len, int nLocals, int nFormals, int /*st MOTH_END_INSTR(DefineArray) MOTH_BEGIN_INSTR(DefineObjectLiteral) - d << dumpRegister(args, nFormals) - << ", " << internalClassId - << ", " << arrayValueCount - << ", " << arrayGetterSetterCountAndFlags; + d << internalClassId + << ", " << argc + << ", " << dumpRegister(args, nFormals); MOTH_END_INSTR(DefineObjectLiteral) MOTH_BEGIN_INSTR(CreateMappedArgumentsObject) diff --git a/src/qml/compiler/qv4instr_moth_p.h b/src/qml/compiler/qv4instr_moth_p.h index a783f9352c..2223400787 100644 --- a/src/qml/compiler/qv4instr_moth_p.h +++ b/src/qml/compiler/qv4instr_moth_p.h @@ -134,8 +134,7 @@ QT_BEGIN_NAMESPACE #define INSTR_TypeofValue(op) INSTRUCTION(op, TypeofValue, 0) #define INSTR_DeclareVar(op) INSTRUCTION(op, DeclareVar, 2, varName, isDeletable) #define INSTR_DefineArray(op) INSTRUCTION(op, DefineArray, 2, argc, args) -// arrayGetterSetterCountAndFlags contains 30 bits for count, 1 bit for needsSparseArray boolean -#define INSTR_DefineObjectLiteral(op) INSTRUCTION(op, DefineObjectLiteral, 4, internalClassId, arrayValueCount, arrayGetterSetterCountAndFlags, args) +#define INSTR_DefineObjectLiteral(op) INSTRUCTION(op, DefineObjectLiteral, 3, internalClassId, argc, args) #define INSTR_CreateMappedArgumentsObject(op) INSTRUCTION(op, CreateMappedArgumentsObject, 0) #define INSTR_CreateUnmappedArgumentsObject(op) INSTRUCTION(op, CreateUnmappedArgumentsObject, 0) #define INSTR_CreateRestParameter(op) INSTRUCTION(op, CreateRestParameter, 1, argIndex) diff --git a/src/qml/jit/qv4baselinejit.cpp b/src/qml/jit/qv4baselinejit.cpp index cd07ac4d58..d8f0976774 100644 --- a/src/qml/jit/qv4baselinejit.cpp +++ b/src/qml/jit/qv4baselinejit.cpp @@ -804,14 +804,12 @@ void BaselineJIT::generate_DefineArray(int argc, int args) JIT_GENERATE_RUNTIME_CALL(Runtime::method_arrayLiteral, Assembler::ResultInAccumulator); } -void BaselineJIT::generate_DefineObjectLiteral(int internalClassId, int arrayValueCount, - int arrayGetterSetterCountAndFlags, int args) +void BaselineJIT::generate_DefineObjectLiteral(int internalClassId, int argc, int args) { - as->prepareCallWithArgCount(5); - as->passInt32AsArg(arrayGetterSetterCountAndFlags, 4); - as->passInt32AsArg(arrayValueCount, 3); - as->passInt32AsArg(internalClassId, 2); - as->passRegAsArg(args, 1); + as->prepareCallWithArgCount(4); + as->passRegAsArg(args, 3); + as->passInt32AsArg(argc, 2); + as->passInt32AsArg(internalClassId, 1); as->passEngineAsArg(0); JIT_GENERATE_RUNTIME_CALL(Runtime::method_objectLiteral, Assembler::ResultInAccumulator); } diff --git a/src/qml/jit/qv4baselinejit_p.h b/src/qml/jit/qv4baselinejit_p.h index 12a3f27e1b..c83b985bd6 100644 --- a/src/qml/jit/qv4baselinejit_p.h +++ b/src/qml/jit/qv4baselinejit_p.h @@ -151,9 +151,7 @@ public: void generate_TypeofValue() override; void generate_DeclareVar(int varName, int isDeletable) override; void generate_DefineArray(int argc, int args) override; - void generate_DefineObjectLiteral(int internalClassId, int arrayValueCount, - int arrayGetterSetterCountAndFlags, - int args) override; + void generate_DefineObjectLiteral(int internalClassId, int argc, int args) override; void generate_CreateMappedArgumentsObject() override; void generate_CreateUnmappedArgumentsObject() override; void generate_CreateRestParameter(int argIndex) override; diff --git a/src/qml/jsruntime/qv4global_p.h b/src/qml/jsruntime/qv4global_p.h index 87eec81afb..607f8b4d28 100644 --- a/src/qml/jsruntime/qv4global_p.h +++ b/src/qml/jsruntime/qv4global_p.h @@ -372,6 +372,12 @@ struct Q_QML_EXPORT StackFrame { }; typedef QVector StackTrace; +enum class ObjectLiteralArgument { + Value, + Getter, + Setter +}; + } Q_DECLARE_TYPEINFO(QV4::PropertyAttributes, Q_PRIMITIVE_TYPE); diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp index b712b40897..5eec51c1e4 100644 --- a/src/qml/jsruntime/qv4runtime.cpp +++ b/src/qml/jsruntime/qv4runtime.cpp @@ -1382,45 +1382,45 @@ ReturnedValue Runtime::method_arrayLiteral(ExecutionEngine *engine, Value *value return engine->newArrayObject(values, length)->asReturnedValue(); } -ReturnedValue Runtime::method_objectLiteral(ExecutionEngine *engine, const QV4::Value *args, int classId, int arrayValueCount, int arrayGetterSetterCountAndFlags) +ReturnedValue Runtime::method_objectLiteral(ExecutionEngine *engine, int classId, int argc, const QV4::Value *args) { Scope scope(engine); Scoped klass(scope, engine->currentStackFrame->v4Function->compilationUnit->runtimeClasses[classId]); ScopedObject o(scope, engine->newObject(klass->d())); - { - bool needSparseArray = arrayGetterSetterCountAndFlags >> 30; - if (needSparseArray) - o->initSparseArray(); - } + Q_ASSERT(uint(argc) >= klass->d()->size); for (uint i = 0; i < klass->d()->size; ++i) o->setProperty(i, *args++); - if (arrayValueCount > 0) { - ScopedValue entry(scope); - for (int i = 0; i < arrayValueCount; ++i) { - uint idx = args->toUInt32(); - ++args; - entry = *args++; - o->arraySet(idx, entry); - } - } + Q_ASSERT((argc - klass->d()->size) % 3 == 0); + int additionalArgs = (argc - int(klass->d()->size))/3; - uint arrayGetterSetterCount = arrayGetterSetterCountAndFlags & ((1 << 30) - 1); - if (arrayGetterSetterCount > 0) { - ScopedProperty pd(scope); - for (uint i = 0; i < arrayGetterSetterCount; ++i) { - uint idx = args->toUInt32(); - ++args; - pd->value = *args; - ++args; - pd->set = *args; - ++args; - o->arraySet(idx, pd, Attr_Accessor); + if (!additionalArgs) + return o->asReturnedValue(); + + Scoped name(scope); + ScopedProperty pd(scope); + for (int i = 0; i < additionalArgs; ++i) { + Q_ASSERT(args->isInteger()); + ObjectLiteralArgument arg = ObjectLiteralArgument(args->integerValue()); + name = args[1].toPropertyKey(engine); + if (engine->hasException) + return Encode::undefined(); + Q_ASSERT(arg == ObjectLiteralArgument::Value || args[2].isFunctionObject()); + if (arg == ObjectLiteralArgument::Value || arg == ObjectLiteralArgument::Getter) { + pd->value = args[2]; + pd->set = Primitive::emptyValue(); + } else { + pd->value = Primitive::emptyValue(); + pd->set = args[2]; } - } + bool ok = o->__defineOwnProperty__(scope.engine, name, pd, (arg == ObjectLiteralArgument::Value ? Attr_Data : Attr_Accessor)); + if (!ok) + return engine->throwTypeError(); + args += 3; + } return o.asReturnedValue(); } diff --git a/src/qml/jsruntime/qv4runtimeapi_p.h b/src/qml/jsruntime/qv4runtimeapi_p.h index 694cd3ea78..66d3bb2a81 100644 --- a/src/qml/jsruntime/qv4runtimeapi_p.h +++ b/src/qml/jsruntime/qv4runtimeapi_p.h @@ -142,7 +142,7 @@ struct ExceptionCheck { \ /* literals */ \ F(ReturnedValue, arrayLiteral, (ExecutionEngine *engine, Value *values, uint length)) \ - F(ReturnedValue, objectLiteral, (ExecutionEngine *engine, const Value *args, int classId, int arrayValueCount, int arrayGetterSetterCountAndFlags)) \ + F(ReturnedValue, objectLiteral, (ExecutionEngine *engine, int classId, int argc, const Value *args)) \ \ /* for-in, for-of and array destructuring */ \ F(ReturnedValue, getIterator, (ExecutionEngine *engine, const Value &in, int iterator)) \ diff --git a/src/qml/jsruntime/qv4value_p.h b/src/qml/jsruntime/qv4value_p.h index 59a8adca8f..16bbf241ff 100644 --- a/src/qml/jsruntime/qv4value_p.h +++ b/src/qml/jsruntime/qv4value_p.h @@ -919,7 +919,6 @@ struct ValueArray { // have wrong offsets between host and target. Q_STATIC_ASSERT(offsetof(ValueArray<0>, values) == 8); - } QT_END_NAMESPACE diff --git a/src/qml/jsruntime/qv4vme_moth.cpp b/src/qml/jsruntime/qv4vme_moth.cpp index 052d6f773b..d5e3ef28bd 100644 --- a/src/qml/jsruntime/qv4vme_moth.cpp +++ b/src/qml/jsruntime/qv4vme_moth.cpp @@ -1034,7 +1034,7 @@ QV4::ReturnedValue VME::interpret(CppStackFrame &frame, const char *code) MOTH_BEGIN_INSTR(DefineObjectLiteral) QV4::Value *arguments = stack + args; - acc = Runtime::method_objectLiteral(engine, arguments, internalClassId, arrayValueCount, arrayGetterSetterCountAndFlags); + acc = Runtime::method_objectLiteral(engine, internalClassId, argc, arguments); MOTH_END_INSTR(DefineObjectLiteral) MOTH_BEGIN_INSTR(CreateMappedArgumentsObject) -- cgit v1.2.3