From 5f7ecce23321f499b1b002c32a27c63815535baa Mon Sep 17 00:00:00 2001 From: Maximilian Goldstein Date: Fri, 9 Apr 2021 17:42:06 +0200 Subject: Implement optional chaining This change implements optional chaining (https://github.com/tc39/proposal-optional-chaining) by adding a new type of optional lookup with an offset to the end of a chain. If `undefined` or `null` is encountered during an access marked as optional, we jump to that end offset. Features: - Full support for all kinds of optional chain - With some codegen overhead but zero overhead during normal non-optional FieldMemberExpression resolution - Properly retains this contexts and does not need to resolve anything twice (this has been an issue previously) - No extra AST structures, just flags for existing ones [ChangeLog][QtQml] Added support for optional chaining (https://github.com/tc39/proposal-optional-chaining) Fixes: QTBUG-77926 Change-Id: I9a41cdc4ca272066c79c72b9b22206498a546843 Reviewed-by: Fabian Kosmale --- src/qml/compiler/qv4bytecodegenerator_p.h | 14 ++ src/qml/compiler/qv4codegen.cpp | 244 ++++++++++++++++++++++++++++-- src/qml/compiler/qv4codegen_p.h | 21 ++- src/qml/compiler/qv4instr_moth.cpp | 8 + src/qml/compiler/qv4instr_moth_p.h | 4 + 5 files changed, 275 insertions(+), 16 deletions(-) (limited to 'src/qml/compiler') diff --git a/src/qml/compiler/qv4bytecodegenerator_p.h b/src/qml/compiler/qv4bytecodegenerator_p.h index 5244c443c4..4cd3b37ad3 100644 --- a/src/qml/compiler/qv4bytecodegenerator_p.h +++ b/src/qml/compiler/qv4bytecodegenerator_p.h @@ -196,6 +196,20 @@ QT_WARNING_POP return addJumpInstruction(data); } + Q_REQUIRED_RESULT Jump jumpOptionalLookup(int index) + { + Instruction::GetOptionalLookup data{}; + data.index = index; + return addJumpInstruction(data); + } + + Q_REQUIRED_RESULT Jump jumpOptionalProperty(int name) + { + Instruction::LoadOptionalProperty data{}; + data.name = name; + return addJumpInstruction(data); + } + void jumpStrictEqual(const StackSlot &lhs, const Label &target) { Instruction::CmpStrictEqual cmp; diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index ab40f0ceba..d13ef1bc86 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -1253,11 +1253,30 @@ bool Codegen::visit(ArrayPattern *ast) bool Codegen::visit(ArrayMemberExpression *ast) { + auto label = traverseOptionalChain(ast); + auto targetLabel = label.has_value() ? label.value() : Moth::BytecodeGenerator::Label(); + if (hasError()) return false; + if (ast->isOptional) + Q_ASSERT(m_optionalChainLabels.contains(ast)); + + TailCallBlocker blockTailCalls(this); Reference base = expression(ast->base); + + auto writeSkip = [&]() { + auto acc = Reference::fromAccumulator(this).storeOnStack(); + base.loadInAccumulator(); + bytecodeGenerator->addInstruction(Instruction::CmpEqNull()); + auto jumpFalse = bytecodeGenerator->jumpFalse(); + bytecodeGenerator->addInstruction(Instruction::LoadUndefined()); + bytecodeGenerator->jump().link(m_optionalChainLabels.take(ast)); + jumpFalse.link(); + acc.loadInAccumulator(); + }; + if (hasError()) return false; if (base.isSuper()) { @@ -1272,17 +1291,31 @@ bool Codegen::visit(ArrayMemberExpression *ast) QString s = str->value.toString(); uint arrayIndex = stringToArrayIndex(s); if (arrayIndex == UINT_MAX) { - setExprResult(Reference::fromMember(base, str->value.toString())); + auto jumpLabel = ast->isOptional ? m_optionalChainLabels.take(ast) : Moth::BytecodeGenerator::Label(); + + setExprResult(Reference::fromMember(base, str->value.toString(), jumpLabel, targetLabel)); return false; } + + if (ast->isOptional) + writeSkip(); + Reference index = Reference::fromConst(this, QV4::Encode(arrayIndex)); - setExprResult(Reference::fromSubscript(base, index)); + setExprResult(Reference::fromSubscript(base, index, targetLabel)); return false; } + + + if (ast->isOptional) + writeSkip(); + Reference index = expression(ast->expression); + if (hasError()) return false; - setExprResult(Reference::fromSubscript(base, index)); + + setExprResult(Reference::fromSubscript(base, index, targetLabel)); + return false; } @@ -1917,6 +1950,8 @@ bool Codegen::visit(CallExpression *ast) if (hasError()) return false; + auto label = traverseOptionalChain(ast); + RegisterScope scope(this); TailCallBlocker blockTailCalls(this); @@ -1944,6 +1979,22 @@ bool Codegen::visit(CallExpression *ast) int thisObject = bytecodeGenerator->newRegister(); int functionObject = bytecodeGenerator->newRegister(); + if (ast->isOptional || (!base.optionalChainJumpLabel.isNull() && base.optionalChainJumpLabel->isValid())) { + if (ast->isOptional) + Q_ASSERT(m_optionalChainLabels.contains(ast)); + + auto jumpLabel = ast->isOptional ? m_optionalChainLabels.take(ast) : *base.optionalChainJumpLabel.get(); + + auto acc = Reference::fromAccumulator(this).storeOnStack(); + base.loadInAccumulator(); + bytecodeGenerator->addInstruction(Instruction::CmpEqNull()); + auto jumpFalse = bytecodeGenerator->jumpFalse(); + bytecodeGenerator->addInstruction(Instruction::LoadUndefined()); + bytecodeGenerator->jump().link(jumpLabel); + jumpFalse.link(); + acc.loadInAccumulator(); + } + auto calldata = pushArgs(ast->arguments); if (hasError()) return false; @@ -1977,15 +2028,28 @@ bool Codegen::visit(CallExpression *ast) } setExprResult(Reference::fromAccumulator(this)); + + if (label.has_value()) + label->link(); + return false; } - handleCall(base, calldata, functionObject, thisObject); + handleCall(base, calldata, functionObject, thisObject, ast->isOptional); + + if (label.has_value()) + label->link(); + return false; } -void Codegen::handleCall(Reference &base, Arguments calldata, int slotForFunction, int slotForThisObject) +void Codegen::endVisit(CallExpression *ast) +{ + m_seenOptionalChainNodes.remove(ast); +} + +void Codegen::handleCall(Reference &base, Arguments calldata, int slotForFunction, int slotForThisObject, bool optional) { //### Do we really need all these call instructions? can's we load the callee in a temp? if (base.type == Reference::Member) { @@ -2012,7 +2076,7 @@ void Codegen::handleCall(Reference &base, Arguments calldata, int slotForFunctio call.argv = calldata.argv; bytecodeGenerator->addInstruction(call); } else if (base.type == Reference::Name) { - if (base.name == QStringLiteral("eval")) { + if (base.name == QStringLiteral("eval") && !optional) { Instruction::CallPossiblyDirectEval call; call.argc = calldata.argc; call.argv = calldata.argv; @@ -2171,12 +2235,18 @@ bool Codegen::visit(DeleteExpression *ast) if (hasError()) return false; + auto label = traverseOptionalChain(ast); + RegisterScope scope(this); TailCallBlocker blockTailCalls(this); Reference expr = expression(ast->expression); if (hasError()) return false; + // If there is a label, there is a chain and that should only be possible with those two kinds of references + if (label.has_value()) + Q_ASSERT(expr.type == Reference::Member || expr.type == Reference::Subscript); + switch (expr.type) { case Reference::SuperProperty: // ### this should throw a reference error at runtime. @@ -2207,6 +2277,13 @@ bool Codegen::visit(DeleteExpression *ast) case Reference::Member: { //### maybe add a variant where the base can be in the accumulator? expr = expr.asLValue(); + + if (!expr.optionalChainJumpLabel.isNull() && expr.optionalChainJumpLabel->isValid()) { + expr.loadInAccumulator(); + bytecodeGenerator->addInstruction(Instruction::CmpEqNull()); + bytecodeGenerator->jumpTrue().link(*expr.optionalChainJumpLabel.get()); + } + Instruction::LoadRuntimeString instr; instr.stringId = expr.propertyNameIndex; bytecodeGenerator->addInstruction(instr); @@ -2217,16 +2294,41 @@ bool Codegen::visit(DeleteExpression *ast) del.index = index.stackSlot(); bytecodeGenerator->addInstruction(del); setExprResult(Reference::fromAccumulator(this)); + + if (label.has_value()) { + auto jump = bytecodeGenerator->jump(); + label->link(); + Instruction::LoadTrue loadTrue; + bytecodeGenerator->addInstruction(loadTrue); + jump.link(); + } + return false; } case Reference::Subscript: { //### maybe add a variant where the index can be in the accumulator? expr = expr.asLValue(); + + if (!expr.optionalChainJumpLabel.isNull() && expr.optionalChainJumpLabel->isValid()) { + expr.loadInAccumulator(); + bytecodeGenerator->addInstruction(Instruction::CmpEqNull()); + bytecodeGenerator->jumpTrue().link(*expr.optionalChainJumpLabel.get()); + } + Instruction::DeleteProperty del; del.base = expr.elementBase; del.index = expr.elementSubscript.stackSlot(); bytecodeGenerator->addInstruction(del); setExprResult(Reference::fromAccumulator(this)); + + if (label.has_value()) { + auto jump = bytecodeGenerator->jump(); + label->link(); + Instruction::LoadTrue loadTrue; + bytecodeGenerator->addInstruction(loadTrue); + jump.link(); + } + return false; } default: @@ -2237,6 +2339,10 @@ bool Codegen::visit(DeleteExpression *ast) return false; } +void Codegen::endVisit(DeleteExpression *ast) { + m_seenOptionalChainNodes.remove(ast); +} + bool Codegen::visit(FalseLiteral *) { if (hasError()) @@ -2255,11 +2361,81 @@ bool Codegen::visit(SuperLiteral *) return false; } +std::optional Codegen::traverseOptionalChain(Node *node) { + if (m_seenOptionalChainNodes.contains(node)) + return {}; + + auto label = bytecodeGenerator->newLabel(); + + auto isOptionalChainNode = [](const Node *node) { + return node->kind == Node::Kind_FieldMemberExpression || + node->kind == Node::Kind_CallExpression || + node->kind == Node::Kind_ArrayMemberExpression || + node->kind == Node::Kind_DeleteExpression; + }; + + bool labelUsed = false; + + while (isOptionalChainNode(node)) { + m_seenOptionalChainNodes.insert(node); + + switch (node->kind) { + case Node::Kind_FieldMemberExpression: { + auto *fme = AST::cast(node); + + if (fme->isOptional) { + m_optionalChainLabels.insert(fme, label); + labelUsed = true; + } + + node = fme->base; + break; + } + case Node::Kind_CallExpression: { + auto *ce = AST::cast(node); + + if (ce->isOptional) { + m_optionalChainLabels.insert(ce, label); + labelUsed = true; + } + + node = ce->base; + break; + } + case Node::Kind_ArrayMemberExpression: { + auto *ame = AST::cast(node); + + if (ame->isOptional) { + m_optionalChainLabels.insert(ame, label); + labelUsed = true; + } + + node = ame->base; + break; + } + case Node::Kind_DeleteExpression: { + auto *de = AST::cast(node); + node = de->expression; + break; + } + } + } + + if (!labelUsed) { + label.link(); // If we don't link the unused label here, we would hit an assert later. + return {}; + } + + return label; +} + bool Codegen::visit(FieldMemberExpression *ast) { if (hasError()) return false; + auto label = traverseOptionalChain(ast); + TailCallBlocker blockTailCalls(this); if (AST::IdentifierExpression *id = AST::cast(ast->base)) { if (id->name == QLatin1String("new")) { @@ -2270,16 +2446,28 @@ bool Codegen::visit(FieldMemberExpression *ast) Reference r = referenceForName(QStringLiteral("new.target"), false); r.isReadonly = true; setExprResult(r); + + if (label.has_value()) + label->link(); + return false; } Reference r = Reference::fromStackSlot(this, CallData::NewTarget); setExprResult(r); + + if (label.has_value()) + label->link(); + return false; } } Reference base = expression(ast->base); + + if (ast->isOptional) + Q_ASSERT(m_optionalChainLabels.contains(ast)); + if (hasError()) return false; if (base.isSuper()) { @@ -2288,12 +2476,25 @@ bool Codegen::visit(FieldMemberExpression *ast) bytecodeGenerator->addInstruction(load); Reference property = Reference::fromAccumulator(this).storeOnStack(); setExprResult(Reference::fromSuperProperty(property)); + + if (label.has_value()) + label->link(); + return false; } - setExprResult(Reference::fromMember(base, ast->name.toString())); + + setExprResult(Reference::fromMember(base, ast->name.toString(), + ast->isOptional ? m_optionalChainLabels.take(ast) : Moth::BytecodeGenerator::Label(), + label.has_value() ? label.value() : Moth::BytecodeGenerator::Label())); + return false; } +void Codegen::endVisit(FieldMemberExpression *ast) +{ + m_seenOptionalChainNodes.remove(ast); +} + bool Codegen::visit(TaggedTemplate *ast) { if (hasError()) @@ -4454,13 +4655,26 @@ QT_WARNING_POP propertyBase.loadInAccumulator(); tdzCheck(requiresTDZCheck); if (!disable_lookups && codegen->useFastLookups) { - Instruction::GetLookup load; - load.index = codegen->registerGetterLookup(propertyNameIndex); - codegen->bytecodeGenerator->addInstruction(load); + if (optionalChainJumpLabel->isValid()) { // If we got a valid jump label, this means it's an optional lookup + auto jump = codegen->bytecodeGenerator->jumpOptionalLookup(codegen->registerGetterLookup(propertyNameIndex)); + jump.link(*optionalChainJumpLabel.get()); + } else { + Instruction::GetLookup load; + load.index = codegen->registerGetterLookup(propertyNameIndex); + codegen->bytecodeGenerator->addInstruction(load); + } } else { - Instruction::LoadProperty load; - load.name = propertyNameIndex; - codegen->bytecodeGenerator->addInstruction(load); + if (optionalChainJumpLabel->isValid()) { + auto jump = codegen->bytecodeGenerator->jumpOptionalProperty(propertyNameIndex); + jump.link(*optionalChainJumpLabel.get()); + } else { + Instruction::LoadProperty load; + load.name = propertyNameIndex; + codegen->bytecodeGenerator->addInstruction(load); + } + } + if (optionalChainTargetLabel->isValid()) { + optionalChainTargetLabel->link(); } return; case Import: { @@ -4476,6 +4690,10 @@ QT_WARNING_POP Instruction::LoadElement load; load.base = elementBase; codegen->bytecodeGenerator->addInstruction(load); + + if (optionalChainTargetLabel->isValid()) { + optionalChainTargetLabel->link(); + } } return; case Invalid: break; diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h index e7d7a21294..e720cb2973 100644 --- a/src/qml/compiler/qv4codegen_p.h +++ b/src/qml/compiler/qv4codegen_p.h @@ -60,6 +60,8 @@ #include #include +#include + QT_BEGIN_NAMESPACE namespace QV4 { @@ -277,11 +279,15 @@ public: r.name = name; return r; } - static Reference fromMember(const Reference &baseRef, const QString &name) { + static Reference fromMember(const Reference &baseRef, const QString &name, + Moth::BytecodeGenerator::Label jumpLabel = Moth::BytecodeGenerator::Label(), + Moth::BytecodeGenerator::Label targetLabel = Moth::BytecodeGenerator::Label()) { Reference r(baseRef.codegen, Member); r.propertyBase = baseRef.asRValue(); r.propertyNameIndex = r.codegen->registerString(name); r.requiresTDZCheck = baseRef.requiresTDZCheck; + r.optionalChainJumpLabel.reset(new Moth::BytecodeGenerator::Label(jumpLabel)); + r.optionalChainTargetLabel.reset(new Moth::BytecodeGenerator::Label(targetLabel)); return r; } static Reference fromSuperProperty(const Reference &property) { @@ -291,13 +297,14 @@ public: r.subscriptRequiresTDZCheck = property.requiresTDZCheck; return r; } - static Reference fromSubscript(const Reference &baseRef, const Reference &subscript) { + static Reference fromSubscript(const Reference &baseRef, const Reference &subscript, Moth::BytecodeGenerator::Label targetLabel = Moth::BytecodeGenerator::Label()) { Q_ASSERT(baseRef.isStackSlot()); Reference r(baseRef.codegen, Subscript); r.elementBase = baseRef.stackSlot(); r.elementSubscript = subscript.asRValue(); r.requiresTDZCheck = baseRef.requiresTDZCheck; r.subscriptRequiresTDZCheck = subscript.requiresTDZCheck; + r.optionalChainTargetLabel.reset(new Moth::BytecodeGenerator::Label(targetLabel)); return r; } static Reference fromConst(Codegen *cg, QV4::ReturnedValue constant) { @@ -374,6 +381,8 @@ public: quint32 isVolatile:1; quint32 global:1; quint32 qmlGlobal:1; + QSharedPointer optionalChainJumpLabel; + QSharedPointer optionalChainTargetLabel; private: void storeAccumulator() const; @@ -598,11 +607,14 @@ protected: bool visit(QQmlJS::AST::ArrayMemberExpression *ast) override; bool visit(QQmlJS::AST::BinaryExpression *ast) override; bool visit(QQmlJS::AST::CallExpression *ast) override; + void endVisit(QQmlJS::AST::CallExpression *ast) override; bool visit(QQmlJS::AST::ConditionalExpression *ast) override; bool visit(QQmlJS::AST::DeleteExpression *ast) override; + void endVisit(QQmlJS::AST::DeleteExpression *ast) override; bool visit(QQmlJS::AST::FalseLiteral *ast) override; bool visit(QQmlJS::AST::SuperLiteral *ast) override; bool visit(QQmlJS::AST::FieldMemberExpression *ast) override; + void endVisit(QQmlJS::AST::FieldMemberExpression *ast) override; bool visit(QQmlJS::AST::TaggedTemplate *ast) override; bool visit(QQmlJS::AST::FunctionExpression *ast) override; bool visit(QQmlJS::AST::IdentifierExpression *ast) override; @@ -686,7 +698,7 @@ public: Reference jumpBinop(QSOperator::Op oper, Reference &left, Reference &right); struct Arguments { int argc; int argv; bool hasSpread; }; Arguments pushArgs(QQmlJS::AST::ArgumentList *args); - void handleCall(Reference &base, Arguments calldata, int slotForFunction, int slotForThisObject); + void handleCall(Reference &base, Arguments calldata, int slotForFunction, int slotForThisObject, bool optional = false); Arguments pushTemplateArgs(QQmlJS::AST::TemplateLiteral *args); bool handleTaggedTemplate(Reference base, QQmlJS::AST::TaggedTemplate *ast); @@ -776,6 +788,8 @@ protected: bool functionEndsWithReturn = false; bool _tailCallsAreAllowed = true; QSet m_globalNames; + QSet m_seenOptionalChainNodes; + QHash m_optionalChainLabels; ControlFlow *controlFlow = nullptr; @@ -812,6 +826,7 @@ private: void handleConstruct(const Reference &base, QQmlJS::AST::ArgumentList *args); void throwError(ErrorType errorType, const QQmlJS::SourceLocation &loc, const QString &detail); + std::optional traverseOptionalChain(QQmlJS::AST::Node *node); }; } diff --git a/src/qml/compiler/qv4instr_moth.cpp b/src/qml/compiler/qv4instr_moth.cpp index 640a908dd3..c791790cba 100644 --- a/src/qml/compiler/qv4instr_moth.cpp +++ b/src/qml/compiler/qv4instr_moth.cpp @@ -280,10 +280,18 @@ void dumpBytecode(const char *code, int len, int nLocals, int nFormals, int /*st d << "acc[" << name << "]"; MOTH_END_INSTR(LoadProperty) + MOTH_BEGIN_INSTR(LoadOptionalProperty) + d << "acc[" << name << "], jump(" << ABSOLUTE_OFFSET() << ")"; + MOTH_END_INSTR(LoadOptionalProperty) + MOTH_BEGIN_INSTR(GetLookup) d << "acc(" << index << ")"; MOTH_END_INSTR(GetLookup) + MOTH_BEGIN_INSTR(GetOptionalLookup) + d << "acc(" << index << "), jump(" << ABSOLUTE_OFFSET() << ")"; + MOTH_END_INSTR(GetOptionalLookup) + MOTH_BEGIN_INSTR(StoreProperty) d << dumpRegister(base, nFormals) << "[" << name<< "]"; MOTH_END_INSTR(StoreProperty) diff --git a/src/qml/compiler/qv4instr_moth_p.h b/src/qml/compiler/qv4instr_moth_p.h index 254e1c46e9..69e4eb26f3 100644 --- a/src/qml/compiler/qv4instr_moth_p.h +++ b/src/qml/compiler/qv4instr_moth_p.h @@ -89,7 +89,9 @@ QT_BEGIN_NAMESPACE #define INSTR_StoreNameSloppy(op) INSTRUCTION(op, StoreNameSloppy, 1, name) #define INSTR_StoreNameStrict(op) INSTRUCTION(op, StoreNameStrict, 1, name) #define INSTR_LoadProperty(op) INSTRUCTION(op, LoadProperty, 1, name) +#define INSTR_LoadOptionalProperty(op) INSTRUCTION(op, LoadOptionalProperty, 2, name, offset) #define INSTR_GetLookup(op) INSTRUCTION(op, GetLookup, 1, index) +#define INSTR_GetOptionalLookup(op) INSTRUCTION(op, GetOptionalLookup, 2, index, offset) #define INSTR_LoadIdObject(op) INSTRUCTION(op, LoadIdObject, 2, index, base) #define INSTR_Yield(op) INSTRUCTION(op, Yield, 0) #define INSTR_YieldStar(op) INSTRUCTION(op, YieldStar, 0) @@ -229,7 +231,9 @@ QT_BEGIN_NAMESPACE F(LoadElement) \ F(StoreElement) \ F(LoadProperty) \ + F(LoadOptionalProperty) \ F(GetLookup) \ + F(GetOptionalLookup) \ F(StoreProperty) \ F(SetLookup) \ F(LoadSuperProperty) \ -- cgit v1.2.3