Issue #1658 - Part 4: Fix optional chaining assertions and remove unreachable code

This includes the following:

Bug 1611777 - Part 1: Report syntax error for optional property access in self-hosting code. r=yulia!
Bug 1611777 - Part 2: Merge same blocks in emitDeleteOptionalChain(). r=yulia!
Bug 1611777 - Part 3: N/A
Bug 1611777 - Part 4: `super` can't occur on the left-hand side of an optional chain
Bug 1611777 - Part 5: Remove unnecessary super-handling in optional delete. r=yulia!
The child node of a DeleteOptionalChainExpr node can't be a super-property
accessor, so we can remove this code.

Bug 1611777 - Part 6: Crash for unexpected super-base in optional call. r=yulia!
Bug 1611777 - Part 7: Add missing entries to list of valid optional chain start expressions. r=yulia!
Bug 1611777 - Part 8: Add missing emitGet in emitOptionalElemExpression. r=yulia!
Aligns emitOptionalElemExpression() with emitOptionalDotExpression(), so it's
easier to compare both methods against each other.

Bug 1611777 - Part 9: Replace an if-statement with an assertion. r=yulia!
Bug 1611777 - Part 10: N/A
Bug 1611777 - Part 11: Support optional chaining in class heritage expression. r=yulia!
Bug 1611777 - Part 12: Use optionalExpr() for update expressions to match spec grammar. r=yulia!

Using optionalExpr matches the spec grammar more closely. This change also
modifies the reported error message. ++a?.b reported before this change
"unexpected token: '?.'", but now reports "invalid increment/decrement operand".

Bug 1611777 - Part 13: N/A
Bug 1611777 - Part 14: Simplify two lines in optionalExpr(). r=yulia!

We don't need to test for tt == TokenKind::Eof when we return for tt != TokenKind::OptionalChain anyway. Omit local variable for the result value and instead use a tail-call. This matches the local style in the parser more closely.

Bug 1611777 - Part 15: Support FunCall/FunApply optimisations for optional chaining. r=yulia!
Bug 1611777 - Part 16: Pass through ValueUsage in optional chains. r=yulia
This commit is contained in:
FranklinDM 2022-04-26 21:24:29 +08:00 • committed by roytam1
commit a350d51223
7 changed files with 127 additions and 151 deletions

View file

@ -3043,7 +3043,10 @@ ASTSerializer::expression(ParseNode* pn, MutableHandleValue dst)
RootedValue propname(cx);
RootedAtom pnAtom(cx, pn->pn_atom);
if (pn->as<PropertyAccessBase>().isSuper()) {
bool isSuper = pn->is<PropertyAccess>() &&
pn->as<PropertyAccess>().isSuper();
if (isSuper) {
if (!builder.super(&pn->pn_expr->pn_pos, &expr))
return false;
} else {
@ -3066,7 +3069,10 @@ ASTSerializer::expression(ParseNode* pn, MutableHandleValue dst)
RootedValue left(cx), right(cx);
if (pn->as<PropertyByValueBase>().isSuper()) {
bool isSuper = pn->is<PropertyByValue>() &&
pn->as<PropertyByValue>().isSuper();
if (isSuper) {
if (!builder.super(&pn->pn_left->pn_pos, &left))
return false;
} else {

View file

@ -9396,19 +9396,9 @@ BytecodeEmitter::emitDeleteOptionalChain(ParseNode* deleteNode)
ParseNode* kid = deleteNode->pn_kid;
switch (kid->getKind()) {
case PNK_ELEM: {
PropertyByValue* elemExpr = &kid->as<PropertyByValue>();
if (!emitDeleteElementInOptChain(elemExpr, oe)) {
// [stack] # If shortcircuit
// [stack] UNDEFINED-OR-NULL
// [stack] # otherwise
// [stack] TRUE
return false;
}
break;
}
case PNK_ELEM:
case PNK_OPTELEM: {
OptionalPropertyByValue* elemExpr = &kid->as<OptionalPropertyByValue>();
PropertyByValueBase* elemExpr = &kid->as<PropertyByValueBase>();
if (!emitDeleteElementInOptChain(elemExpr, oe)) {
// [stack] # If shortcircuit
// [stack] UNDEFINED-OR-NULL
@ -9418,19 +9408,9 @@ BytecodeEmitter::emitDeleteOptionalChain(ParseNode* deleteNode)
}
break;
}
case PNK_DOT: {
PropertyAccess* propExpr = &kid->as<PropertyAccess>();
if (!emitDeletePropertyInOptChain(propExpr, oe)) {
// [stack] # If shortcircuit
// [stack] UNDEFINED-OR-NULL
// [stack] # otherwise
// [stack] TRUE
return false;
}
break;
}
case PNK_DOT:
case PNK_OPTDOT: {
OptionalPropertyAccess* propExpr = &kid->as<OptionalPropertyAccess>();
PropertyAccessBase* propExpr = &kid->as<PropertyAccessBase>();
if (!emitDeletePropertyInOptChain(propExpr, oe)) {
// [stack] # If shortcircuit
// [stack] UNDEFINED-OR-NULL
@ -9460,54 +9440,26 @@ BytecodeEmitter::emitDeletePropertyInOptChain(
PropertyAccessBase* propExpr,
OptionalEmitter& oe)
{
if (propExpr->isSuper()) {
// The expression |delete super.foo;| has to evaluate |super.foo|,
// which could throw if |this| hasn't yet been set by a |super(...)|
// call or the super-base is not an object, before throwing a
// ReferenceError for attempting to delete a super-reference.
ParseNode* base = &propExpr->expression();
if (!emitGetThisForSuperBase(base)) {
// [stack] THIS
return false;
}
} else {
if (!emitOptionalTree(&propExpr->expression(), oe)) {
MOZ_ASSERT_IF(propExpr->is<PropertyAccess>(),
!propExpr->as<PropertyAccess>().isSuper());
if (!emitOptionalTree(&propExpr->expression(), oe)) {
// [stack] OBJ
return false;
}
if (propExpr->isKind(PNK_OPTDOT)) {
if (!oe.emitJumpShortCircuit()) {
// [stack] # if Jump
// [stack] UNDEFINED-OR-NULL
// [stack] # otherwise
// [stack] OBJ
return false;
}
if (propExpr->isKind(PNK_OPTDOT)) {
if (!oe.emitJumpShortCircuit()) {
// [stack] # if Jump
// [stack] UNDEFINED-OR-NULL
// [stack] # otherwise
// [stack] OBJ
return false;
}
}
}
if (propExpr->isSuper()) {
// Still have to calculate the base, even though we are are going
// to throw unconditionally, as calculating the base could also
// throw.
if (!emit1(JSOP_SUPERBASE)) {
return false;
}
if (!emitUint16Operand(JSOP_THROWMSG, JSMSG_CANT_DELETE_SUPER)) {
return false;
}
// Another wrinkle: Balance the stack from the emitter's point of view.
// Execution will not reach here, as the last bytecode threw.
if (!emit1(JSOP_POP)) {
return false;
}
} else {
JSOp delOp = sc->strict() ? JSOP_STRICTDELPROP : JSOP_DELPROP;
if (!emitAtomOp(propExpr, delOp)) {
return false;
}
JSOp delOp = sc->strict() ? JSOP_STRICTDELPROP : JSOP_DELPROP;
if (!emitAtomOp(propExpr, delOp)) {
return false;
}
return true;
@ -9518,6 +9470,9 @@ BytecodeEmitter::emitDeleteElementInOptChain(
PropertyByValueBase* elemExpr,
OptionalEmitter& oe)
{
MOZ_ASSERT_IF(elemExpr->is<PropertyByValue>(),
!elemExpr->as<PropertyByValue>().isSuper());
if (!emitOptionalTree(elemExpr->pn_left, oe)) {
// [stack] OBJ
return false;
@ -9538,19 +9493,6 @@ BytecodeEmitter::emitDeleteElementInOptChain(
return false;
}
if (elemExpr->isSuper()) {
if (!emit1(JSOP_SUPERBASE)) {
return false;
}
if (!emitUint16Operand(JSOP_THROWMSG, JSMSG_CANT_DELETE_SUPER)) {
return false;
}
// Another wrinkle: Balance the stack from the emitter's point of view.
// Execution will not reach here, as the last bytecode threw.
return emit1(JSOP_POP);
}
JSOp delOp = sc->strict() ? JSOP_STRICTDELELEM : JSOP_DELELEM;
return emitElemOpBase(delOp);
}
@ -9858,18 +9800,11 @@ BytecodeEmitter::emitOptionalCalleeAndThis(
isCall = false;
break;
}
case PNK_SUPERBASE: {
MOZ_ASSERT(callNode->isKind(PNK_SUPERCALL));
MOZ_ASSERT(parser->handler.isSuperBase(calleeNode));
if (!emit1(JSOP_SUPERFUN)) {
return false;
}
break;
}
case PNK_OPTCHAIN: {
return emitCalleeAndThisForOptionalChain(calleeNode, callNode, isCall);
}
default: {
MOZ_RELEASE_ASSERT(calleeNode->getKind() != PNK_SUPERBASE);
if (!emitOptionalTree(calleeNode, oe)) {
return false;
}
@ -9899,10 +9834,10 @@ BytecodeEmitter::emitOptionalCalleeAndThis(
bool
BytecodeEmitter::emitOptionalCall(
ParseNode* callNode,
OptionalEmitter& oe)
OptionalEmitter& oe,
ValueUsage valueUsage)
{
bool isCall = true;
ValueUsage valueUsage = ValueUsage::WantValue;
ParseNode* calleeNode = callNode->pn_head;
if (!emitOptionalCalleeAndThis(callNode, calleeNode, isCall, oe)) {
@ -11509,7 +11444,7 @@ BytecodeEmitter::emitTree(ParseNode* pn, ValueUsage valueUsage /* = ValueUsage::
break;
case PNK_OPTCHAIN:
if (!emitOptionalChain(pn)) {
if (!emitOptionalChain(pn, valueUsage)) {
return false;
}
break;
@ -11713,7 +11648,8 @@ BytecodeEmitter::emitTreeInBranch(ParseNode* pn,
bool
BytecodeEmitter::emitOptionalTree(
ParseNode* pn,
OptionalEmitter& oe)
OptionalEmitter& oe,
ValueUsage valueUsage /* = ValueUsage::WantValue */)
{
JS_CHECK_RECURSION(cx, return false);
@ -11749,7 +11685,7 @@ BytecodeEmitter::emitOptionalTree(
}
case PNK_CALL:
case PNK_OPTCALL: {
if (!emitOptionalCall(pn, oe)) {
if (!emitOptionalCall(pn, oe, valueUsage)) {
return false;
}
break;
@ -11759,28 +11695,38 @@ BytecodeEmitter::emitOptionalTree(
// For example, a taggedTemplateExpr node might occur if we have
// `test`?.b, with `test` as the taggedTemplateExpr ParseNode.
default: {
MOZ_ASSERT(
(kind == PNK_ARRAY ||
kind == PNK_OBJECT ||
kind == PNK_TRUE ||
kind == PNK_FALSE ||
kind == PNK_STRING ||
kind == PNK_NUMBER ||
kind == PNK_RAW_UNDEFINED ||
kind == PNK_NULL ||
kind == PNK_NAME ||
kind == PNK_FUNCTION ||
kind == PNK_THIS ||
kind == PNK_TAGGED_TEMPLATE ||
kind == PNK_TEMPLATE_STRING ||
kind == PNK_AWAIT ||
kind == PNK_REGEXP ||
kind == PNK_CLASS ||
kind == PNK_COMMA ||
kind == PNK_NEW ||
kind == PNK_SETTHIS ||
kind == PNK_NEWTARGET),
"Unknown ParseNodeKind for OptionalChain");
#ifdef DEBUG
// https://tc39.es/ecma262/#sec-primary-expression
bool isPrimaryExpression =
kind == PNK_THIS ||
kind == PNK_NAME ||
kind == PNK_NULL ||
kind == PNK_TRUE ||
kind == PNK_FALSE ||
kind == PNK_NUMBER ||
kind == PNK_STRING ||
kind == PNK_ARRAY ||
kind == PNK_OBJECT ||
kind == PNK_FUNCTION ||
kind == PNK_CLASS ||
kind == PNK_REGEXP ||
kind == PNK_TEMPLATE_STRING ||
kind == PNK_RAW_UNDEFINED ||
pn->isInParens();
// https://tc39.es/ecma262/#sec-left-hand-side-expressions
bool isMemberExpression = isPrimaryExpression ||
kind == PNK_TAGGED_TEMPLATE ||
kind == PNK_NEW ||
kind == PNK_NEWTARGET;
//kind == ParseNodeKind::ImportMetaExpr;
bool isCallExpression = kind == PNK_SETTHIS;
//kind == ParseNodeKind::CallImportExpr;
MOZ_ASSERT(isMemberExpression || isCallExpression,
"Unknown ParseNodeKind for OptionalChain");
#endif
return emitTree(pn);
}
}
@ -11824,13 +11770,15 @@ BytecodeEmitter::emitCalleeAndThisForOptionalChain(
}
bool
BytecodeEmitter::emitOptionalChain(ParseNode* optionalChain)
BytecodeEmitter::emitOptionalChain(
ParseNode* optionalChain,
ValueUsage valueUsage)
{
ParseNode* expression = optionalChain->pn_kid;
OptionalEmitter oe(this, stackDepth);
if (!emitOptionalTree(expression, oe)) {
if (!emitOptionalTree(expression, oe, valueUsage)) {
// [stack] VAL
return false;
}
@ -11853,12 +11801,13 @@ BytecodeEmitter::emitOptionalDotExpression(
ParseNode* calleeNode,
bool isCall)
{
bool isSuper = prop->isSuper();
bool isSuper = prop->is<PropertyAccess>() &&
prop->as<PropertyAccess>().isSuper();
ParseNode* base = &prop->expression();
if (isSuper) {
if (!emitGetThisForSuperBase(base)) {
// [stack] THIS
// [stack] OBJ
return false;
}
} else {
@ -11869,6 +11818,7 @@ BytecodeEmitter::emitOptionalDotExpression(
}
if (prop->isKind(PNK_OPTDOT)) {
MOZ_ASSERT(!isSuper);
if (!oe.emitJumpShortCircuit()) {
// [stack] # if Jump
// [stack] UNDEFINED-OR-NULL
@ -11915,20 +11865,23 @@ BytecodeEmitter::emitOptionalElemExpression(
ParseNode* calleeNode,
bool isCall)
{
if (elem->isSuper()) {
if (!emitSuperElemOp(calleeNode, JSOP_GETELEM_SUPER, isCall)) {
bool isSuper = elem->is<PropertyByValue>() &&
elem->as<PropertyByValue>().isSuper();
if (isSuper) {
if (!emitGetThisForSuperBase(calleeNode)) {
// [stack] OBJ
return false;
}
} else {
if (!emitOptionalTree(calleeNode->pn_left, oe)) {
// [stack] OBJ
return false;
}
return true;
}
if (!emitOptionalTree(calleeNode->pn_left, oe)) {
// [stack] OBJ
return false;
}
if (elem->isKind(PNK_OPTELEM)) {
MOZ_ASSERT(!isSuper);
if (!oe.emitJumpShortCircuit()) {
// [stack] # if Jump
// [stack] UNDEFINED-OR-NULL

View file

@ -459,7 +459,9 @@ struct MOZ_STACK_CLASS BytecodeEmitter
EmitLineNumberNote emitLineNote = EMIT_LINENOTE);
// Emit code for the optional tree rooted at pn.
MOZ_MUST_USE bool emitOptionalTree(ParseNode* pn, OptionalEmitter& oe);
MOZ_MUST_USE bool emitOptionalTree(ParseNode* pn,
OptionalEmitter& oe,
ValueUsage valueUsage = ValueUsage::WantValue);
// Emit code for the tree rooted at pn with its own TDZ cache.
MOZ_MUST_USE bool emitTreeInBranch(ParseNode* pn,
@ -786,7 +788,8 @@ struct MOZ_STACK_CLASS BytecodeEmitter
MOZ_MUST_USE bool emitDeleteExpression(ParseNode* pn);
// Optional methods which emit Optional Jump Target
MOZ_MUST_USE bool emitOptionalChain(ParseNode* optionalChain);
MOZ_MUST_USE bool emitOptionalChain(ParseNode* optionalChain,
ValueUsage valueUsage);
MOZ_MUST_USE bool emitCalleeAndThisForOptionalChain(ParseNode* optionalChain,
ParseNode* callNode,
bool isCall);
@ -802,7 +805,9 @@ struct MOZ_STACK_CLASS BytecodeEmitter
OptionalEmitter& oe,
ParseNode* calleeNode,
bool isCall);
MOZ_MUST_USE bool emitOptionalCall(ParseNode* callNode, OptionalEmitter& oe);
MOZ_MUST_USE bool emitOptionalCall(ParseNode* callNode,
OptionalEmitter& oe,
ValueUsage valueUsage);
MOZ_MUST_USE bool emitDeletePropertyInOptChain(PropertyAccessBase* propExpr,
OptionalEmitter& oe);
MOZ_MUST_USE bool emitDeleteElementInOptChain(PropertyByValueBase* elemExpr,

View file

@ -74,6 +74,10 @@ class FullParseHandler
return node->isKind(PNK_DOT) || node->isKind(PNK_ELEM);
}
bool isOptionalPropertyAccess(ParseNode* node) {
return node->isKind(PNK_OPTDOT) || node->isKind(PNK_OPTELEM);
}
bool isFunctionCall(ParseNode* node) {
// Note: super() is a special form, *not* a function call.
return node->isKind(PNK_CALL);
@ -949,7 +953,9 @@ class FullParseHandler
return pn->isKind(PNK_CALL);
}
PropertyName* maybeDottedProperty(ParseNode* pn) {
return pn->is<PropertyAccess>() ? &pn->as<PropertyAccess>().name() : nullptr;
return pn->is<PropertyAccessBase>() ?
&pn->as<PropertyAccessBase>().name() :
nullptr;
}
JSAtom* isStringExprStatement(ParseNode* pn, TokenPos* pos) {
if (JSAtom* atom = pn->isStringExprStatement()) {

View file

@ -1237,11 +1237,6 @@ class PropertyAccessBase : public ParseNode
PropertyName& name() const {
return *pn_u.name.atom->asPropertyName();
}
bool isSuper() const {
// PNK_SUPERBASE cannot result from any expression syntax.
return expression().isKind(PNK_SUPERBASE);
}
};
class PropertyAccess : public PropertyAccessBase
@ -1260,6 +1255,11 @@ class PropertyAccess : public PropertyAccessBase
MOZ_ASSERT_IF(match, node.isArity(PN_NAME));
return match;
}
bool isSuper() const {
// PNK_SUPERBASE cannot result from any expression syntax.
return expression().isKind(PNK_SUPERBASE);
}
};
class OptionalPropertyAccess : public PropertyAccessBase
@ -1297,10 +1297,6 @@ class PropertyByValueBase : public ParseNode
MOZ_ASSERT_IF(match, node.isArity(PN_BINARY));
return match;
}
bool isSuper() const {
return pn_left->isKind(PNK_SUPERBASE);
}
};
class PropertyByValue : public PropertyByValueBase {
@ -1314,6 +1310,10 @@ class PropertyByValue : public PropertyByValueBase {
MOZ_ASSERT_IF(match, node.isArity(PN_BINARY));
return match;
}
bool isSuper() const {
return pn_left->isKind(PNK_SUPERBASE);
}
};
class OptionalPropertyByValue : public PropertyByValueBase {

View file

@ -7154,7 +7154,7 @@ Parser<ParseHandler>::classDefinition(YieldHandling yieldHandling,
if (hasHeritage) {
if (!tokenStream.getToken(&tt))
return null();
classHeritage = memberExpr(yieldHandling, TripledotProhibited, tt);
classHeritage = optionalExpr(yieldHandling, TripledotProhibited, tt);
if (!classHeritage)
return null();
}
@ -8386,7 +8386,7 @@ Parser<ParseHandler>::optionalExpr(
return null();
}
if (tt == TOK_EOF || tt != TOK_OPTCHAIN) {
if (tt != TOK_OPTCHAIN) {
return lhs;
}
@ -8456,9 +8456,8 @@ Parser<ParseHandler>::optionalExpr(
break;
}
if (nextMember) {
lhs = nextMember;
}
MOZ_ASSERT(nextMember);
lhs = nextMember;
}
return handler.newOptionalChain(begin, lhs);
@ -8515,7 +8514,7 @@ Parser<ParseHandler>::unaryExpr(YieldHandling yieldHandling, TripledotHandling t
return null();
uint32_t operandOffset = pos().begin;
Node operand = memberExpr(yieldHandling, TripledotProhibited, tt2);
Node operand = optionalExpr(yieldHandling, TripledotProhibited, tt2);
if (!operand || !checkIncDecOperand(operand, operandOffset))
return null();
@ -9157,6 +9156,7 @@ Parser<ParseHandler>::memberPropertyAccess(
return null();
}
if (optionalKind == OptionalKind::Optional) {
MOZ_ASSERT(!handler.isSuperBase(lhs));
return handler.newOptionalPropertyAccess(lhs, field, pos().end);
}
return handler.newPropertyAccess(lhs, field, pos().end);
@ -9182,6 +9182,7 @@ Parser<ParseHandler>::memberElemAccess(
}
if (optionalKind == OptionalKind::Optional) {
MOZ_ASSERT(!handler.isSuperBase(lhs));
return handler.newOptionalPropertyByValue(lhs, propExpr, pos().end);
}
return handler.newPropertyByValue(lhs, propExpr, pos().end);
@ -9194,7 +9195,8 @@ Parser<ParseHandler>::memberCall(
PossibleError* possibleError /* = nullptr */,
OptionalKind optionalKind /* = OptionalKind::NonOptional */)
{
if (options().selfHostingMode && handler.isPropertyAccess(lhs)) {
if (options().selfHostingMode && (handler.isPropertyAccess(lhs) ||
handler.isOptionalPropertyAccess(lhs))) {
error(JSMSG_SELFHOSTED_METHOD_CALL);
return null();
}

View file

@ -149,6 +149,10 @@ class SyntaxParseHandler
return node == NodeDottedProperty || node == NodeElement;
}
bool isOptionalPropertyAccess(Node node) {
return node == NodeOptionalDottedProperty || node == NodeOptionalElement;
}
bool isFunctionCall(Node node) {
// Note: super() is a special form, *not* a function call.
return node == NodeFunctionCall;
@ -603,7 +607,7 @@ class SyntaxParseHandler
// |this|. It's not really eligible for the funapply/funcall
// optimizations as they're currently implemented (assuming a single
// value is used for both retrieval and |this|).
if (node != NodeDottedProperty)
if (node != NodeDottedProperty && node != NodeOptionalDottedProperty)
return nullptr;
return lastAtom->asPropertyName();
}