Skip to content

Commit

Permalink
Bug 1916260 - Part 9: Resolve binding in destructuring before retriev…
Browse files Browse the repository at this point in the history 
…ing destructured element. r=arai

`BytecodeEmitter::emitDestructuringLHSRef` needs to call `NameOpEmitter::prepareForRhs`
to emit the necessary steps to resolve environment bindings. To avoid adding a new
`skipPrepareForRhs` method to `NameOpEmitter`, instead pass `DestructuringLHSRef&`
to `emitDestructuringLHSRef`, where `DestructuringLHSRef` holds the emitter for the
destructuring left-hand side reference.

Tests are in <tc39/test262#4216>.

Differential Revision: https://phabricator.services.mozilla.com/D220828
  • Loading branch information
@anba
anba committed Sep 4, 2024
1 parent 0c9e7e7 commit 640f230
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 204 deletions.
182 changes: 89 additions & 93 deletions js/src/frontend/BytecodeEmitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2595,23 +2595,63 @@ bool BytecodeEmitter::emitFunctionScript(FunctionNode* funNode) {
return fse.intoStencil();
}

class js::frontend::DestructuringLHSRef {
struct None {
size_t numReferenceSlots() const { return 0; }
};

mozilla::Variant<None, NameOpEmitter, PropOpEmitter, ElemOpEmitter,
PrivateOpEmitter>
emitter_ = AsVariant(None{});

public:
template <typename T>
void from(T&& emitter) {
emitter_.emplace<T>(std::forward<T>(emitter));
}

template <typename T>
T& emitter() {
return emitter_.as<T>();
}

/**
* Return the number of values pushed onto the stack.
*/
size_t numReferenceSlots() const {
return emitter_.match([](auto& e) { return e.numReferenceSlots(); });
}
};

bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target,
size_t* emitted) {
DestructuringFlavor flav,
DestructuringLHSRef& lref) {
#ifdef DEBUG
int depth = bytecodeSection().stackDepth();
#endif

switch (target->getKind()) {
case ParseNodeKind::Name:
case ParseNodeKind::ArrayExpr:
case ParseNodeKind::ObjectExpr:
// No need to recurse into ParseNodeKind::Array and ParseNodeKind::Object
// subpatterns here, since emitSetOrInitializeDestructuring does the
// recursion when setting or initializing the value. Getting reference
// doesn't recurse.
*emitted = 0;
// recursion when setting or initializing the value.
break;

case ParseNodeKind::Name: {
auto* name = &target->as<NameNode>();
NameOpEmitter noe(this, name->atom(),
flav == DestructuringFlavor::Assignment
? NameOpEmitter::Kind::SimpleAssignment
: NameOpEmitter::Kind::Initialize);
if (!noe.prepareForRhs()) {
return false;
}

lref.from(std::move(noe));
return true;
}

case ParseNodeKind::ArgumentsLength:
case ParseNodeKind::DotExpr: {
PropertyAccess* prop = &target->as<PropertyAccess>();
Expand Down Expand Up @@ -2642,8 +2682,7 @@ bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target,
return false;
}

// SUPERBASE was pushed onto THIS in poe.prepareForRhs above.
*emitted = 1 + isSuper;
lref.from(std::move(poe));
break;
}

Expand All @@ -2669,8 +2708,7 @@ bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target,
return false;
}

// SUPERBASE was pushed onto KEY in eoe.prepareForRhs above.
*emitted = 2 + isSuper;
lref.from(std::move(eoe));
break;
}

Expand All @@ -2686,7 +2724,8 @@ bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target,
// [stack] OBJ NAME
return false;
}
*emitted = xoe.numReferenceSlots();

lref.from(std::move(xoe));
break;
}

Expand All @@ -2701,13 +2740,14 @@ bool BytecodeEmitter::emitDestructuringLHSRef(ParseNode* target,
MOZ_CRASH("emitDestructuringLHSRef: bad lhs kind");
}

MOZ_ASSERT(bytecodeSection().stackDepth() == depth + int(*emitted));
MOZ_ASSERT(bytecodeSection().stackDepth() ==
depth + int(lref.numReferenceSlots()));

return true;
}

bool BytecodeEmitter::emitSetOrInitializeDestructuring(
ParseNode* target, DestructuringFlavor flav) {
ParseNode* target, DestructuringFlavor flav, DestructuringLHSRef& lref) {
// Now emit the lvalue opcode sequence. If the lvalue is a nested
// destructuring initialiser-form, call ourselves to handle it, then pop
// the matched value. Otherwise emit an lvalue bytecode sequence followed
Expand All @@ -2724,51 +2764,14 @@ bool BytecodeEmitter::emitSetOrInitializeDestructuring(
break;

case ParseNodeKind::Name: {
auto name = target->as<NameNode>().name();
NameLocation loc = lookupName(name);
NameOpEmitter::Kind kind;
switch (flav) {
case DestructuringFlavor::Declaration:
kind = NameOpEmitter::Kind::Initialize;
break;
// The environment is already pushed by emitDestructuringLHSRef.
// [stack] ENV? VAL
auto& noe = lref.emitter<NameOpEmitter>();

case DestructuringFlavor::Assignment:
kind = NameOpEmitter::Kind::SimpleAssignment;
break;
}

NameOpEmitter noe(this, name, loc, kind);
if (!noe.prepareForRhs()) {
// [stack] V ENV?
return false;
}
if (noe.emittedBindOp()) {
// This is like ordinary assignment, but with one difference.
//
// In `a = b`, we first determine a binding for `a` (using
// JSOp::BindName or JSOp::BindGName), then we evaluate `b`, then
// a JSOp::SetName instruction.
//
// In `[a] = [b]`, per spec, `b` is evaluated first, then we
// determine a binding for `a`. Then we need to do assignment--
// but the operands are on the stack in the wrong order for
// JSOp::SetProp, so we have to add a JSOp::Swap.
//
// In the cases where we are emitting a name op, emit a swap
// because of this.
if (!emit1(JSOp::Swap)) {
// [stack] ENV V
return false;
}
} else {
// In cases of emitting a frame slot or environment slot,
// nothing needs be done.
}
if (!noe.emitAssignment()) {
// [stack] V
// [stack] VAL
return false;
}

break;
}

Expand All @@ -2779,16 +2782,11 @@ bool BytecodeEmitter::emitSetOrInitializeDestructuring(
// [stack] THIS SUPERBASE VAL
// [stack] # otherwise
// [stack] OBJ VAL
PropertyAccess* prop = &target->as<PropertyAccess>();
bool isSuper = prop->isSuper();
PropOpEmitter poe(this, PropOpEmitter::Kind::SimpleAssignment,
isSuper ? PropOpEmitter::ObjKind::Super
: PropOpEmitter::ObjKind::Other);
if (!poe.skipObjAndRhs()) {
return false;
}
// [stack] # VAL
auto& poe = lref.emitter<PropOpEmitter>();
auto* prop = &target->as<PropertyAccess>();

if (!poe.emitAssignment(prop->key().atom())) {
// [stack] # VAL
return false;
}
break;
Expand All @@ -2800,15 +2798,8 @@ bool BytecodeEmitter::emitSetOrInitializeDestructuring(
// [stack] THIS KEY SUPERBASE VAL
// [stack] # otherwise
// [stack] OBJ KEY VAL
PropertyByValue* elem = &target->as<PropertyByValue>();
bool isSuper = elem->isSuper();
MOZ_ASSERT(!elem->key().isKind(ParseNodeKind::PrivateName));
ElemOpEmitter eoe(this, ElemOpEmitter::Kind::SimpleAssignment,
isSuper ? ElemOpEmitter::ObjKind::Super
: ElemOpEmitter::ObjKind::Other);
if (!eoe.skipObjAndKeyAndRhs()) {
return false;
}
auto& eoe = lref.emitter<ElemOpEmitter>();

if (!eoe.emitAssignment()) {
// [stack] VAL
return false;
Expand All @@ -2819,12 +2810,8 @@ bool BytecodeEmitter::emitSetOrInitializeDestructuring(
case ParseNodeKind::PrivateMemberExpr: {
// The reference is already pushed by emitDestructuringLHSRef.
// [stack] OBJ NAME VAL
PrivateMemberAccess* privateExpr = &target->as<PrivateMemberAccess>();
PrivateOpEmitter xoe(this, PrivateOpEmitter::Kind::SimpleAssignment,
privateExpr->privateName().name());
if (!xoe.skipReference()) {
return false;
}
auto& xoe = lref.emitter<PrivateOpEmitter>();

if (!xoe.emitAssignment()) {
// [stack] VAL
return false;
Expand Down Expand Up @@ -3375,6 +3362,7 @@ bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern,
idx += 1;
continue;
}
MOZ_ASSERT(member->isKind(ParseNodeKind::Name));

if (!emit1(JSOp::Dup)) {
// [stack] LENGTH OBJ OBJ
Expand Down Expand Up @@ -3432,7 +3420,13 @@ bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern,
return false;
}

if (!emitSetOrInitializeDestructuring(member, flav)) {
DestructuringLHSRef lref;
if (!emitDestructuringLHSRef(member, flav, lref)) {
// [stack] LENGTH OBJ
return false;
}

if (!emitSetOrInitializeDestructuring(member, flav, lref)) {
// [stack] LENGTH OBJ
return false;
}
Expand Down Expand Up @@ -3519,21 +3513,22 @@ bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern,
pndefault = subpattern->as<AssignmentNode>().right();
}

// Number of stack slots emitted for the LHS reference.
size_t emitted = 0;

// Spec requires LHS reference to be evaluated first.
DestructuringLHSRef lref;
bool isElision = lhsPattern->isKind(ParseNodeKind::Elision);
if (!isElision) {
auto emitLHSRef = [lhsPattern, &emitted](BytecodeEmitter* bce) {
return bce->emitDestructuringLHSRef(lhsPattern, &emitted);
auto emitLHSRef = [lhsPattern, flav, &lref](BytecodeEmitter* bce) {
return bce->emitDestructuringLHSRef(lhsPattern, flav, lref);
// [stack] ... OBJ NEXT ITER DONE LREF*
};
if (!wrapWithDestructuringTryNote(tryNoteDepth, emitLHSRef)) {
return false;
}
}

// Number of stack slots emitted for the LHS reference.
size_t emitted = lref.numReferenceSlots();

// Pick the DONE value to the top of the stack.
if (emitted) {
if (!emitPickN(emitted)) {
Expand Down Expand Up @@ -3616,8 +3611,8 @@ bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern,
return false;
}

auto emitAssignment = [lhsPattern, flav](BytecodeEmitter* bce) {
return bce->emitSetOrInitializeDestructuring(lhsPattern, flav);
auto emitAssignment = [lhsPattern, flav, &lref](BytecodeEmitter* bce) {
return bce->emitSetOrInitializeDestructuring(lhsPattern, flav, lref);
// [stack] ... OBJ NEXT ITER TRUE
};
if (!wrapWithDestructuringTryNote(tryNoteDepth, emitAssignment)) {
Expand Down Expand Up @@ -3741,8 +3736,8 @@ bool BytecodeEmitter::emitDestructuringOpsArray(ListNode* pattern,
}

if (!isElision) {
auto emitAssignment = [lhsPattern, flav](BytecodeEmitter* bce) {
return bce->emitSetOrInitializeDestructuring(lhsPattern, flav);
auto emitAssignment = [lhsPattern, flav, &lref](BytecodeEmitter* bce) {
return bce->emitSetOrInitializeDestructuring(lhsPattern, flav, lref);
// [stack] ... OBJ NEXT ITER DONE
};

Expand Down Expand Up @@ -3863,15 +3858,16 @@ bool BytecodeEmitter::emitDestructuringOpsObject(ListNode* pattern,
pndefault = subpattern->as<AssignmentNode>().right();
}

// Number of stack slots emitted for the LHS reference.
size_t emitted = 0;

// Spec requires LHS reference to be evaluated first.
if (!emitDestructuringLHSRef(lhs, &emitted)) {
DestructuringLHSRef lref;
if (!emitDestructuringLHSRef(lhs, flav, lref)) {
// [stack] ... SET? RHS KEY? LREF*
return false;
}

// Number of stack slots emitted for the LHS reference.
size_t emitted = lref.numReferenceSlots();

// Duplicate the value being destructured to use as a reference base.
if (!emitDupAt(emitted + hasKeyOnStack)) {
// [stack] ... SET? RHS KEY? LREF* RHS
Expand Down Expand Up @@ -3911,7 +3907,7 @@ bool BytecodeEmitter::emitDestructuringOpsObject(ListNode* pattern,
}

// Destructure TARGET per this member's lhs.
if (!emitSetOrInitializeDestructuring(lhs, flav)) {
if (!emitSetOrInitializeDestructuring(lhs, flav, lref)) {
// [stack] ... RHS
return false;
}
Expand Down Expand Up @@ -3998,7 +3994,7 @@ bool BytecodeEmitter::emitDestructuringOpsObject(ListNode* pattern,
}

// Destructure PROP per this member's lhs.
if (!emitSetOrInitializeDestructuring(lhs, flav)) {
if (!emitSetOrInitializeDestructuring(lhs, flav, lref)) {
// [stack] ... SET? RHS
return false;
}
Expand Down
9 changes: 5 additions & 4 deletions js/src/frontend/BytecodeEmitter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ namespace frontend {
class BytecodeOffset;
class CallOrNewEmitter;
class ClassEmitter;
class DestructuringLHSRef;
class ElemOpEmitter;
class EmitterScope;
class ErrorReporter;
Expand Down Expand Up @@ -787,16 +788,16 @@ struct MOZ_STACK_CLASS BytecodeEmitter {
// If the lhs expression is object property |OBJ.prop|, it emits |OBJ|.
// If it's object element |OBJ[ELEM]|, it emits |OBJ| and |ELEM|.
// If there's nothing to evaluate for the reference, it emits nothing.
// |emitted| parameter receives the number of values pushed onto the stack.
[[nodiscard]] bool emitDestructuringLHSRef(ParseNode* target,
size_t* emitted);
DestructuringFlavor flav,
DestructuringLHSRef& lref);

// emitSetOrInitializeDestructuring assumes the lhs expression's reference
// and the to-be-destructured value has been pushed on the stack. It emits
// code to destructure a single lhs expression (either a name or a compound
// []/{} expression).
[[nodiscard]] bool emitSetOrInitializeDestructuring(ParseNode* target,
DestructuringFlavor flav);
[[nodiscard]] bool emitSetOrInitializeDestructuring(
ParseNode* target, DestructuringFlavor flav, DestructuringLHSRef& lref);

// emitDestructuringObjRestExclusionSet emits the property exclusion set
// for the rest-property in an object pattern.
Expand Down
10 changes: 0 additions & 10 deletions js/src/frontend/ElemOpEmitter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,16 +138,6 @@ bool ElemOpEmitter::prepareForRhs() {
return true;
}

bool ElemOpEmitter::skipObjAndKeyAndRhs() {
MOZ_ASSERT(state_ == State::Start);
MOZ_ASSERT(isSimpleAssignment() || isPropInit());

#ifdef DEBUG
state_ = State::Rhs;
#endif
return true;
}

bool ElemOpEmitter::emitDelete() {
MOZ_ASSERT(state_ == State::Key);
MOZ_ASSERT(isDelete());
Expand Down
Loading

0 comments on commit 640f230

Please sign in to comment.