From 849ab4417c4f1a9f971f1f3d9fc072c8700cd71d Mon Sep 17 00:00:00 2001 From: Martok Date: Sun, 9 Apr 2023 04:42:04 +0200 Subject: [PATCH] Issue #2142 - Use JSOP_INITPROP for field initializers We don't want to call setters for field initialization. Based-on: m-c 1535471, 1547035 --- js/src/builtin/ReflectParse.cpp | 2 +- js/src/frontend/BytecodeEmitter.cpp | 50 +++++++++++++++++++++-------- js/src/frontend/BytecodeEmitter.h | 3 +- js/src/frontend/ElemOpEmitter.cpp | 19 ++++++----- js/src/frontend/ElemOpEmitter.h | 6 ++++ js/src/frontend/FoldConstants.cpp | 2 ++ js/src/frontend/ParseNode.cpp | 1 + js/src/frontend/ParseNode.h | 8 +++-- js/src/frontend/Parser.cpp | 12 +++---- js/src/frontend/PropOpEmitter.cpp | 20 ++++++------ js/src/frontend/PropOpEmitter.h | 6 ++++ 11 files changed, 89 insertions(+), 40 deletions(-) diff --git a/js/src/builtin/ReflectParse.cpp b/js/src/builtin/ReflectParse.cpp index 2902d2b724..242a4b8b0f 100644 --- a/js/src/builtin/ReflectParse.cpp +++ b/js/src/builtin/ReflectParse.cpp @@ -2770,7 +2770,7 @@ ASTSerializer::classField(ClassField* classField, MutableHandleValue dst) ->head()->as() .scopeBody()->as() .head()->as() - .kid()->as() + .kid()->as() .right(); // RawUndefinedExpr is the node we use for "there is no initializer". If one // writes, literally, `x = undefined;`, it will not be a RawUndefinedExpr diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 02400ce2a3..5fb9ec7809 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -1214,6 +1214,10 @@ BytecodeEmitter::checkSideEffects(ParseNode* pn, bool* answer) return true; // Binary cases with obvious side effects. + case PNK_INITPROP: + *answer = true; + return true; + case PNK_ASSIGN: case PNK_ADDASSIGN: case PNK_SUBASSIGN: @@ -3697,9 +3701,14 @@ EmitAssignmentRhs(BytecodeEmitter* bce, ParseNode* rhs, uint8_t offset) } bool -BytecodeEmitter::emitAssignment(ParseNode* lhs, JSOp compoundOp, ParseNode* rhs) +BytecodeEmitter::emitAssignmentOrInit(ParseNodeKind kind, JSOp compoundOp, + ParseNode* lhs, ParseNode* rhs) { bool isCompound = compoundOp != JSOP_NOP; + bool isInit = kind == PNK_INITPROP; + + MOZ_ASSERT_IF(isInit, lhs->isKind(PNK_DOT) || + lhs->isKind(PNK_ELEM)); // Name assignments are handled separately because choosing ops and when // to emit BINDNAME is involved and should avoid duplication. @@ -3754,7 +3763,8 @@ BytecodeEmitter::emitAssignment(ParseNode* lhs, JSOp compoundOp, ParseNode* rhs) poe.emplace(this, isCompound ? PropOpEmitter::Kind::CompoundAssignment - : PropOpEmitter::Kind::SimpleAssignment, + : isInit ? PropOpEmitter::Kind::PropInit + : PropOpEmitter::Kind::SimpleAssignment, isSuper ? PropOpEmitter::ObjKind::Super : PropOpEmitter::ObjKind::Other); @@ -3781,7 +3791,8 @@ BytecodeEmitter::emitAssignment(ParseNode* lhs, JSOp compoundOp, ParseNode* rhs) eoe.emplace(this, isCompound ? ElemOpEmitter::Kind::CompoundAssignment - : ElemOpEmitter::Kind::SimpleAssignment, + : isInit ? ElemOpEmitter::Kind::PropInit + : ElemOpEmitter::Kind::SimpleAssignment, isSuper ? ElemOpEmitter::ObjKind::Super : ElemOpEmitter::ObjKind::Other); @@ -4714,7 +4725,7 @@ BytecodeEmitter::emitInitializeForInOrOfTarget(TernaryNode* forHead) // initialization is just assigning the iteration value to a target // expression. if (!parser->handler.isDeclarationList(target)) - return emitAssignment(target, JSOP_NOP, nullptr); // ... ITERVAL + return emitAssignmentOrInit(PNK_ASSIGN, JSOP_NOP, target, nullptr); // ... ITERVAL // Otherwise, per-loop initialization is (possibly) declaration // initialization. If the declaration is a lexical declaration, it must be @@ -4728,8 +4739,19 @@ BytecodeEmitter::emitInitializeForInOrOfTarget(TernaryNode* forHead) MOZ_ASSERT(target->isForLoopDeclaration()); target = parser->handler.singleBindingFromDeclaration(&target->as()); + NameNode* nameNode = nullptr; if (target->isKind(PNK_NAME)) { - NameOpEmitter noe(this, target->name(), NameOpEmitter::Kind::Initialize); + nameNode = &target->as(); + } else if (target->isKind(PNK_ASSIGN) || + target->isKind(PNK_INITPROP)) { + BinaryNode* assignNode = &target->as(); + if (assignNode->left()->is()) { + nameNode = &assignNode->left()->as(); + } + } + + if (nameNode) { + NameOpEmitter noe(this, nameNode->name(), NameOpEmitter::Kind::Initialize); if (!noe.prepareForRhs()) { return false; } @@ -4755,7 +4777,7 @@ BytecodeEmitter::emitInitializeForInOrOfTarget(TernaryNode* forHead) return true; } - MOZ_ASSERT(!target->isKind(PNK_ASSIGN), + MOZ_ASSERT(!target->isKind(PNK_ASSIGN) && !target->isKind(PNK_INITPROP), "for-in/of loop destructuring declarations can't have initializers"); MOZ_ASSERT(target->isKind(PNK_ARRAY) || target->isKind(PNK_OBJECT)); @@ -5396,7 +5418,7 @@ BytecodeEmitter::emitComprehensionForOf(ForNode* forNode) // Notice: Comprehension for-of doesn't perform IteratorClose, since it's // not in the spec. - if (!emitAssignment(loopVariableName, JSOP_NOP, nullptr)) // ITER RESULT VALUE + if (!emitAssignmentOrInit(PNK_ASSIGN, JSOP_NOP, loopVariableName, nullptr)) // ITER RESULT VALUE return false; // Remove VALUE from the stack to release it. @@ -5533,7 +5555,7 @@ BytecodeEmitter::emitComprehensionForIn(ForNode* forNode) // Emit code to assign the enumeration value to the left hand side, but // also leave it on the stack. - if (!emitAssignment(forHead->kid2(), JSOP_NOP, nullptr)) + if (!emitAssignmentOrInit(PNK_ASSIGN, JSOP_NOP, forHead->kid2(), nullptr)) return false; /* The stack should be balanced around the assignment opcode sequence. */ @@ -8384,9 +8406,9 @@ BytecodeEmitter::emitFunctionFormalParameters(ListNode* paramsBody) for (ParseNode* arg = paramsBody->head(); arg != funBody; arg = arg->pn_next) { ParseNode* bindingElement = arg; ParseNode* initializer = nullptr; - if (arg->isKind(PNK_ASSIGN)) { - bindingElement = arg->as().left(); - initializer = arg->as().right(); + if (arg->isKind(PNK_ASSIGN) || arg->isKind(PNK_INITPROP)) { + bindingElement = arg->as().left(); + initializer = arg->as().right(); } bool hasInitializer = !!initializer; bool isRest = hasRest && arg->pn_next == funBody; @@ -8830,6 +8852,7 @@ BytecodeEmitter::emitTree(ParseNode* pn, ValueUsage valueUsage /* = ValueUsage:: return false; break; + case PNK_INITPROP: case PNK_ASSIGN: case PNK_ADDASSIGN: case PNK_SUBASSIGN: @@ -8843,8 +8866,9 @@ BytecodeEmitter::emitTree(ParseNode* pn, ValueUsage valueUsage /* = ValueUsage:: case PNK_DIVASSIGN: case PNK_MODASSIGN: case PNK_POWASSIGN: { - AssignmentNode* assignNode = &pn->as(); - if (!emitAssignment(assignNode->left(), assignNode->getOp(), assignNode->right())) + BinaryNode* assignNode = &pn->as(); + if (!emitAssignmentOrInit(assignNode->getKind(), assignNode->getOp(), + assignNode->left(), assignNode->right())) return false; break; } diff --git a/js/src/frontend/BytecodeEmitter.h b/js/src/frontend/BytecodeEmitter.h index 8a196a4799..88594778b6 100644 --- a/js/src/frontend/BytecodeEmitter.h +++ b/js/src/frontend/BytecodeEmitter.h @@ -696,7 +696,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter MOZ_MUST_USE bool emitCallSiteObject(CallSiteNode* callSiteObj); MOZ_MUST_USE bool emitTemplateString(ListNode* templateString); - MOZ_MUST_USE bool emitAssignment(ParseNode* lhs, JSOp compoundOp, ParseNode* rhs); + MOZ_MUST_USE bool emitAssignmentOrInit(ParseNodeKind kind, JSOp compoundOp, + ParseNode* lhs, ParseNode* rhs); MOZ_MUST_USE bool emitReturn(UnaryNode* returnNode); MOZ_MUST_USE bool emitStatement(UnaryNode* exprStmt); diff --git a/js/src/frontend/ElemOpEmitter.cpp b/js/src/frontend/ElemOpEmitter.cpp index 2644072337..4bd8684ab2 100644 --- a/js/src/frontend/ElemOpEmitter.cpp +++ b/js/src/frontend/ElemOpEmitter.cpp @@ -132,11 +132,11 @@ ElemOpEmitter::emitGet() bool ElemOpEmitter::prepareForRhs() { - MOZ_ASSERT(isSimpleAssignment() || isCompoundAssignment()); - MOZ_ASSERT_IF(isSimpleAssignment(), state_ == State::Key); + MOZ_ASSERT(isSimpleAssignment() || isPropInit()|| isCompoundAssignment()); + MOZ_ASSERT_IF(isSimpleAssignment() || isPropInit(), state_ == State::Key); MOZ_ASSERT_IF(isCompoundAssignment(), state_ == State::Get); - if (isSimpleAssignment()) { + if (isSimpleAssignment() || isPropInit()) { // For CompoundAssignment, SUPERBASE is already emitted by emitGet. if (isSuper()) { if (!bce_->emit1(JSOP_SUPERBASE)) { // THIS KEY SUPERBASE @@ -155,7 +155,7 @@ bool ElemOpEmitter::skipObjAndKeyAndRhs() { MOZ_ASSERT(state_ == State::Start); - MOZ_ASSERT(isSimpleAssignment()); + MOZ_ASSERT(isSimpleAssignment() || isPropInit()); #ifdef DEBUG state_ = State::Rhs; @@ -203,12 +203,15 @@ ElemOpEmitter::emitDelete() bool ElemOpEmitter::emitAssignment() { - MOZ_ASSERT(isSimpleAssignment() || isCompoundAssignment()); + MOZ_ASSERT(isSimpleAssignment() || isPropInit() || isCompoundAssignment()); MOZ_ASSERT(state_ == State::Rhs); - JSOp setOp = isSuper() - ? bce_->sc->strict() ? JSOP_STRICTSETELEM_SUPER : JSOP_SETELEM_SUPER - : bce_->sc->strict() ? JSOP_STRICTSETELEM : JSOP_SETELEM; + MOZ_ASSERT_IF(isPropInit(), !isSuper()); + + JSOp setOp = isPropInit() ? JSOP_INITELEM + : isSuper() + ? bce_->sc->strict() ? JSOP_STRICTSETELEM_SUPER : JSOP_SETELEM_SUPER + : bce_->sc->strict() ? JSOP_STRICTSETELEM : JSOP_SETELEM; if (!bce_->emitElemOpBase(setOp)) { // ELEM return false; } diff --git a/js/src/frontend/ElemOpEmitter.h b/js/src/frontend/ElemOpEmitter.h index 48b3eaa43c..5eccb920e7 100644 --- a/js/src/frontend/ElemOpEmitter.h +++ b/js/src/frontend/ElemOpEmitter.h @@ -129,6 +129,7 @@ class MOZ_STACK_CLASS ElemOpEmitter PostDecrement, PreDecrement, SimpleAssignment, + PropInit, CompoundAssignment }; enum class ObjKind { @@ -176,6 +177,7 @@ class MOZ_STACK_CLASS ElemOpEmitter // | +--------+ | // | +-------------------+ // | [SimpleAssignment] | + // | [PropInit] | // | prepareForRhs v +-----+ // +--------------------->+-------------->+->| Rhs |-+ // | ^ +-----+ | @@ -225,6 +227,10 @@ class MOZ_STACK_CLASS ElemOpEmitter return kind_ == Kind::SimpleAssignment; } + MOZ_MUST_USE bool isPropInit() const { + return kind_ == Kind::PropInit; + } + MOZ_MUST_USE bool isDelete() const { return kind_ == Kind::Delete; } diff --git a/js/src/frontend/FoldConstants.cpp b/js/src/frontend/FoldConstants.cpp index eda40a0836..bb8f0d11b6 100644 --- a/js/src/frontend/FoldConstants.cpp +++ b/js/src/frontend/FoldConstants.cpp @@ -351,6 +351,7 @@ ContainsHoistedDeclaration(ExclusiveContext* cx, ParseNode* node, bool* result) case PNK_DIV: case PNK_MOD: case PNK_POW: + case PNK_INITPROP: case PNK_ASSIGN: case PNK_ADDASSIGN: case PNK_SUBASSIGN: @@ -1877,6 +1878,7 @@ Fold(ExclusiveContext* cx, ParseNode** pnp, Parser& parser, bo case PNK_SWITCH: case PNK_COLON: + case PNK_INITPROP: case PNK_ASSIGN: case PNK_ADDASSIGN: case PNK_SUBASSIGN: diff --git a/js/src/frontend/ParseNode.cpp b/js/src/frontend/ParseNode.cpp index 00bbd7afee..de3ea60931 100644 --- a/js/src/frontend/ParseNode.cpp +++ b/js/src/frontend/ParseNode.cpp @@ -257,6 +257,7 @@ PushNodeChildren(ParseNode* pn, NodeStack* stack) // Binary nodes with two non-null children. // All assignment and compound assignment nodes qualify. + case PNK_INITPROP: case PNK_ASSIGN: case PNK_ADDASSIGN: case PNK_SUBASSIGN: diff --git a/js/src/frontend/ParseNode.h b/js/src/frontend/ParseNode.h index da722a51e7..5919cfa735 100644 --- a/js/src/frontend/ParseNode.h +++ b/js/src/frontend/ParseNode.h @@ -125,6 +125,7 @@ class ObjectBox; F(SUPERBASE) \ F(SUPERCALL) \ F(SETTHIS) \ + F(INITPROP) \ F(IMPORT_META) \ F(CALL_IMPORT) \ \ @@ -170,7 +171,7 @@ class ObjectBox; F(POW) \ \ /* Assignment operators (= += -= etc.). */ \ - /* ParseNode::isAssignment assumes all these are consecutive. */ \ + /* AssignmentNode::test assumes all these are consecutive. */ \ F(ASSIGN) \ F(ADDASSIGN) \ F(SUBASSIGN) \ @@ -385,7 +386,10 @@ IsTypeofKind(ParseNodeKind kind) * PNK_COMMA (ListNode) * head: list of N comma-separated exprs * count: N >= 2 - * PNK_ASSIGN (BinaryNode) + * PNK_INITPROP (BinaryNode) + * left: target of assignment, base-class setter will not be invoked + * right: value to assign + * PNK_ASSIGN (AssignmentNode) * left: target of assignment * right: value to assign * PNK_ADDASSIGN, PNK_SUBASSIGN, PNK_BITORASSIGN, PNK_BITXORASSIGN, diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index d184ae956a..bd7dc40bf7 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -8048,18 +8048,18 @@ Parser::fieldInitializerOpt(YieldHandling yieldHandling, bool hasH return null(); } - // Synthesize an assignment expression for the property. - AssignmentNodeType initializerAssignment = handler.newAssignment(PNK_ASSIGN, - propAssignFieldAccess, initializerExpr, - JSOP_NOP); - if (!initializerAssignment) + // Synthesize a property init. + AssignmentNodeType initializerPropInit = handler.newAssignment(PNK_INITPROP, + propAssignFieldAccess, initializerExpr, + JSOP_NOP); + if (!initializerPropInit) return null(); bool canSkipLazyClosedOverBindings = handler.canSkipLazyClosedOverBindings(); if (!declareFunctionThis(canSkipLazyClosedOverBindings)) return null(); - UnaryNodeType exprStatement = handler.newExprStatement(initializerAssignment, wholeInitializerPos.end); + UnaryNodeType exprStatement = handler.newExprStatement(initializerPropInit, wholeInitializerPos.end); if (!exprStatement) return null(); diff --git a/js/src/frontend/PropOpEmitter.cpp b/js/src/frontend/PropOpEmitter.cpp index fd00024e7c..7be5f17230 100644 --- a/js/src/frontend/PropOpEmitter.cpp +++ b/js/src/frontend/PropOpEmitter.cpp @@ -110,11 +110,11 @@ PropOpEmitter::emitGet(JSAtom* prop) bool PropOpEmitter::prepareForRhs() { - MOZ_ASSERT(isSimpleAssignment() || isCompoundAssignment()); - MOZ_ASSERT_IF(isSimpleAssignment(), state_ == State::Obj); + MOZ_ASSERT(isSimpleAssignment() || isPropInit() || isCompoundAssignment()); + MOZ_ASSERT_IF(isSimpleAssignment() || isPropInit(), state_ == State::Obj); MOZ_ASSERT_IF(isCompoundAssignment(), state_ == State::Get); - if (isSimpleAssignment()) { + if (isSimpleAssignment() || isPropInit()) { // For CompoundAssignment, SUPERBASE is already emitted by emitGet. if (isSuper()) { if (!bce_->emit1(JSOP_SUPERBASE)) { // THIS SUPERBASE @@ -133,7 +133,7 @@ bool PropOpEmitter::skipObjAndRhs() { MOZ_ASSERT(state_ == State::Start); - MOZ_ASSERT(isSimpleAssignment()); + MOZ_ASSERT(isSimpleAssignment() || isPropInit()); #ifdef DEBUG state_ = State::Rhs; @@ -182,18 +182,20 @@ PropOpEmitter::emitDelete(JSAtom* prop) bool PropOpEmitter::emitAssignment(JSAtom* prop) { - MOZ_ASSERT(isSimpleAssignment() || isCompoundAssignment()); + MOZ_ASSERT(isSimpleAssignment() || isPropInit() || isCompoundAssignment()); MOZ_ASSERT(state_ == State::Rhs); - if (isSimpleAssignment()) { + if (isSimpleAssignment() || isPropInit()) { if (!prepareAtomIndex(prop)) { return false; } } - JSOp setOp = isSuper() - ? bce_->sc->strict() ? JSOP_STRICTSETPROP_SUPER : JSOP_SETPROP_SUPER - : bce_->sc->strict() ? JSOP_STRICTSETPROP : JSOP_SETPROP; + MOZ_ASSERT_IF(isPropInit(), !isSuper()); + JSOp setOp = isPropInit() ? JSOP_INITPROP + : isSuper() + ? bce_->sc->strict() ? JSOP_STRICTSETPROP_SUPER : JSOP_SETPROP_SUPER + : bce_->sc->strict() ? JSOP_STRICTSETPROP : JSOP_SETPROP; if (!bce_->emitAtomOp(propAtomIndex_, setOp)) { // VAL return false; } diff --git a/js/src/frontend/PropOpEmitter.h b/js/src/frontend/PropOpEmitter.h index 2c5a26ec45..2e6d6e5089 100644 --- a/js/src/frontend/PropOpEmitter.h +++ b/js/src/frontend/PropOpEmitter.h @@ -115,6 +115,7 @@ class MOZ_STACK_CLASS PropOpEmitter PostDecrement, PreDecrement, SimpleAssignment, + PropInit, CompoundAssignment }; enum class ObjKind { @@ -167,6 +168,7 @@ class MOZ_STACK_CLASS PropOpEmitter // | +--------+ | // | | // | [SimpleAssignment] | + // | [PropInit] | // | prepareForRhs | +-----+ // +--------------------->+-------------->+->| Rhs |-+ // | ^ +-----+ | @@ -217,6 +219,10 @@ class MOZ_STACK_CLASS PropOpEmitter return kind_ == Kind::SimpleAssignment; } + MOZ_MUST_USE bool isPropInit() const { + return kind_ == Kind::PropInit; + } + MOZ_MUST_USE bool isDelete() const { return kind_ == Kind::Delete; }