Skip to content

Commit

Permalink
Reimplementing optional chaining using simpler bytecode
Browse files Browse the repository at this point in the history
Made special ref work with optional chaining

Made optional chaining work with expressions, `a?.[b]`

Removed useless `++maxStack`

Add new token in `AstNode`

Fixed bug in interpreter code generation - was jumping a bit too far
  • Loading branch information
andreabergia authored and gbrail committed Oct 23, 2024
1 parent 6bddf63 commit 83bd883
Showing 14 changed files with 441 additions and 51 deletions.
92 changes: 89 additions & 3 deletions rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java
Original file line number Diff line number Diff line change
@@ -689,7 +689,27 @@ private void visitExpression(Node node, int contextFlags) {
case Token.GETPROPNOWARN:
visitExpression(child, 0);
child = child.getNext();
addStringOp(type, child.getString());
if (node.getIntProp(Node.OPTIONAL_CHAINING, 0) == 1) {
// Jump if null or undefined
addIcode(Icode_DUP);
stackChange(1);
int putUndefinedLabel = iCodeTop;
addGotoOp(Icode.Icode_IF_NULL_UNDEF);
stackChange(-1);

// Access property
addStringOp(type, child.getString());
int afterLabel = iCodeTop;
addGotoOp(Token.GOTO);

// Put undefined
resolveForwardGoto(putUndefinedLabel);
addIcode(Icode_POP);
addStringOp(Token.NAME, "undefined");
resolveForwardGoto(afterLabel);
} else {
addStringOp(type, child.getString());
}
break;

case Token.DELPROP:
@@ -707,6 +727,30 @@ private void visitExpression(Node node, int contextFlags) {
break;

case Token.GETELEM:
visitExpression(child, 0);
child = child.getNext();
if (node.getIntProp(Node.OPTIONAL_CHAINING, 0) == 1) {
addIcode(Icode_DUP);
stackChange(1);
int putUndefinedLabel = iCodeTop;
addGotoOp(Icode.Icode_IF_NULL_UNDEF);
stackChange(-1);

// Infix op
finishGetElemGeneration(child);
int afterLabel = iCodeTop;
addGotoOp(Token.GOTO);

// Put undefined
resolveForwardGoto(putUndefinedLabel);
addIcode(Icode_POP);
addStringOp(Token.NAME, "undefined");
resolveForwardGoto(afterLabel);
} else {
finishGetElemGeneration(child);
}
break;

case Token.BITAND:
case Token.BITOR:
case Token.BITXOR:
@@ -754,7 +798,23 @@ private void visitExpression(Node node, int contextFlags) {
case Token.GET_REF:
case Token.DEL_REF:
visitExpression(child, 0);
addToken(type);
if (node.getIntProp(Node.OPTIONAL_CHAINING, 0) == 1) {
// On the stack we'll have either the Ref or undefined
addIcode(Icode_DUP);
stackChange(1);

// If it's null or undefined, just jump ahead
int afterLabel = iCodeTop;
addGotoOp(Icode.Icode_IF_NULL_UNDEF);
stackChange(-1);

// Otherwise do the GET_REF
addToken(type);

resolveForwardGoto(afterLabel);
} else {
addToken(type);
}
break;

case Token.SETPROP:
@@ -962,7 +1022,27 @@ private void visitExpression(Node node, int contextFlags) {

case Token.REF_SPECIAL:
visitExpression(child, 0);
addStringOp(type, (String) node.getProp(Node.NAME_PROP));
if (node.getIntProp(Node.OPTIONAL_CHAINING, 0) == 1) {
// Jump if null or undefined
addIcode(Icode_DUP);
stackChange(1);
int putUndefinedLabel = iCodeTop;
addGotoOp(Icode.Icode_IF_NULL_UNDEF);
stackChange(-1);

// Access property
addStringOp(type, (String) node.getProp(Node.NAME_PROP));
int afterLabel = iCodeTop;
addGotoOp(Token.GOTO);

// Put undefined
resolveForwardGoto(putUndefinedLabel);
addIcode(Icode_POP);
addStringOp(Token.NAME, "undefined");
resolveForwardGoto(afterLabel);
} else {
addStringOp(type, (String) node.getProp(Node.NAME_PROP));
}
break;

case Token.REF_MEMBER:
@@ -1043,6 +1123,12 @@ private void visitExpression(Node node, int contextFlags) {
}
}

private void finishGetElemGeneration(Node child) {
visitExpression(child, 0);
addToken(Token.GETELEM);
stackChange(-1);
}

private void generateCallFunAndThis(Node left) {
// Generate code to place on stack function and thisObj
int type = left.getType();
34 changes: 27 additions & 7 deletions rhino/src/main/java/org/mozilla/javascript/IRFactory.java
Original file line number Diff line number Diff line change
@@ -163,6 +163,12 @@ private Node transform(AstNode node) {
return transformElementGet((ElementGet) node);
case Token.GETPROP:
return transformPropertyGet((PropertyGet) node);
case Token.QUESTION_DOT:
if (node instanceof ElementGet) {
return transformElementGet((ElementGet) node);
} else {
return transformPropertyGet((PropertyGet) node);
}
case Token.HOOK:
return transformCondExpr((ConditionalExpression) node);
case Token.IF:
@@ -347,7 +353,8 @@ private Node arrayCompTransformHelper(ArrayComprehension node, String arrayName)
Node call =
createCallOrNew(
Token.CALL,
createPropertyGet(parser.createName(arrayName), null, "push", 0));
createPropertyGet(
parser.createName(arrayName), null, "push", 0, node.type));

Node body = new Node(Token.EXPR_VOID, call, lineno);

@@ -518,7 +525,11 @@ private Node transformElementGet(ElementGet node) {
// iff elem is string that can not be number
Node target = transform(node.getTarget());
Node element = transform(node.getElement());
return new Node(Token.GETELEM, target, element);
Node getElem = new Node(Token.GETELEM, target, element);
if (node.type == Token.QUESTION_DOT) {
getElem.putIntProp(Node.OPTIONAL_CHAINING, 1);
}
return getElem;
}

private Node transformExprStmt(ExpressionStatement node) {
@@ -938,7 +949,7 @@ private Node transformComputedPropertyKey(ComputedPropertyKey node) {
private Node transformPropertyGet(PropertyGet node) {
Node target = transform(node.getTarget());
String name = node.getProperty().getIdentifier();
return createPropertyGet(target, null, name, 0);
return createPropertyGet(target, null, name, 0, node.type);
}

private Node transformTemplateLiteral(TemplateLiteral node) {
@@ -1262,7 +1273,7 @@ private Node transformXmlRef(Node pn, XmlRef node, int memberTypeFlags) {
String ns = namespace != null ? namespace.getIdentifier() : null;
if (node instanceof XmlPropRef) {
String name = ((XmlPropRef) node).getPropName().getIdentifier();
return createPropertyGet(pn, ns, name, memberTypeFlags);
return createPropertyGet(pn, ns, name, memberTypeFlags, node.type);
}
Node expr = transform(((XmlElemRef) node).getExpression());
return createElementGet(pn, ns, expr, memberTypeFlags);
@@ -1897,7 +1908,7 @@ private static Node createIncDec(int nodeType, boolean post, Node child) {
}

private Node createPropertyGet(
Node target, String namespace, String name, int memberTypeFlags) {
Node target, String namespace, String name, int memberTypeFlags, int type) {
if (namespace == null && memberTypeFlags == 0) {
if (target == null) {
return parser.createName(name);
@@ -1906,10 +1917,19 @@ private Node createPropertyGet(
if (ScriptRuntime.isSpecialProperty(name)) {
Node ref = new Node(Token.REF_SPECIAL, target);
ref.putProp(Node.NAME_PROP, name);
return new Node(Token.GET_REF, ref);
Node getRef = new Node(Token.GET_REF, ref);
if (type == Token.QUESTION_DOT) {
ref.putIntProp(Node.OPTIONAL_CHAINING, 1);
getRef.putIntProp(Node.OPTIONAL_CHAINING, 1);
}
return getRef;
}

return new Node(Token.GETPROP, target, Node.newString(name));
Node node = new Node(Token.GETPROP, target, Node.newString(name));
if (type == Token.QUESTION_DOT) {
node.putIntProp(Node.OPTIONAL_CHAINING, 1);
}
return node;
}
Node elem = Node.newString(name);
memberTypeFlags |= Node.PROPERTY_FLAG;
8 changes: 7 additions & 1 deletion rhino/src/main/java/org/mozilla/javascript/Icode.java
Original file line number Diff line number Diff line change
@@ -142,8 +142,12 @@ abstract class Icode {
Icode_TEMPLATE_LITERAL_CALLSITE = Icode_REG_BIGINT4 - 1,
Icode_LITERAL_KEYS = Icode_TEMPLATE_LITERAL_CALLSITE - 1,
Icode_LITERAL_KEY_SET = Icode_LITERAL_KEYS - 1,

// Jump if stack head is null or undefined
Icode_IF_NULL_UNDEF = Icode_LITERAL_KEY_SET - 1,

// Last icode
MIN_ICODE = Icode_LITERAL_KEY_SET;
MIN_ICODE = Icode_IF_NULL_UNDEF;

static String bytecodeName(int bytecode) {
if (!validBytecode(bytecode)) {
@@ -309,6 +313,8 @@ static String bytecodeName(int bytecode) {
return "LITERAL_KEYS";
case Icode_LITERAL_KEY_SET:
return "LITERAL_KEY_SET";
case Icode_IF_NULL_UNDEF:
return "IF_NULL_UNDEF";
}

// icode without name
12 changes: 12 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/Interpreter.java
Original file line number Diff line number Diff line change
@@ -551,6 +551,7 @@ static void dumpICode(InterpreterData idata) {
case Token.IFEQ:
case Token.IFNE:
case Icode_IFEQ_POP:
case Icode_IF_NULL_UNDEF:
case Icode_LEAVEDQ:
{
int newPC = pc + getShort(iCode, pc) - 1;
@@ -821,6 +822,7 @@ private static int bytecodeSpan(int bytecode) {
case Token.IFEQ:
case Token.IFNE:
case Icode_IFEQ_POP:
case Icode_IF_NULL_UNDEF:
case Icode_LEAVEDQ:
// target pc offset
return 1 + 2;
@@ -1434,6 +1436,16 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl
}
stack[stackTop--] = null;
break jumplessRun;
case Icode_IF_NULL_UNDEF:
{
Object val = frame.stack[stackTop];
--stackTop;
if (val != null && !Undefined.isUndefined(val)) {
frame.pc += 2;
continue Loop;
}
break jumplessRun;
}
case Token.GOTO:
break jumplessRun;
case Icode_GOSUB:
5 changes: 4 additions & 1 deletion rhino/src/main/java/org/mozilla/javascript/Node.java
Original file line number Diff line number Diff line change
@@ -66,7 +66,8 @@ public class Node implements Iterable<Node> {
TEMPLATE_LITERAL_PROP = 27,
TRAILING_COMMA = 28,
OBJECT_LITERAL_DESTRUCTURING = 29,
LAST_PROP = 29;
OPTIONAL_CHAINING = 30,
LAST_PROP = OPTIONAL_CHAINING;

// values of ISNUMBER_PROP to specify
// which of the children are Number types
@@ -434,6 +435,8 @@ private static final String propToString(int propType) {
return "template_literal";
case TRAILING_COMMA:
return "trailing comma";
case OPTIONAL_CHAINING:
return "optional_chaining";

default:
Kit.codeBug();
54 changes: 41 additions & 13 deletions rhino/src/main/java/org/mozilla/javascript/Parser.java
Original file line number Diff line number Diff line change
@@ -2439,6 +2439,8 @@ private AstNode assignExpr() throws IOException {

markDestructuring(pn);
int opPos = ts.tokenBeg;
if (isNotValidSimpleAssignmentTarget(pn))
reportError("msg.syntax.invalid.assignment.lhs");

pn = new Assignment(tt, pn, assignExpr(), opPos);

@@ -2461,6 +2463,12 @@ private AstNode assignExpr() throws IOException {
return pn;
}

private static boolean isNotValidSimpleAssignmentTarget(AstNode pn) {
if (pn.getType() == Token.GETPROP)
return isNotValidSimpleAssignmentTarget(((PropertyGet) pn).getLeft());
return pn.getType() == Token.QUESTION_DOT;
}

private AstNode condExpr() throws IOException {
AstNode pn = nullishCoalescingExpr();
if (matchToken(Token.HOOK, true)) {
@@ -2887,6 +2895,7 @@ private AstNode memberExprTail(boolean allowCallSyntax, AstNode pn) throws IOExc
int tt = peekToken();
switch (tt) {
case Token.DOT:
case Token.QUESTION_DOT:
case Token.DOTDOT:
lineno = ts.lineno;
pn = propertyAccess(tt, pn);
@@ -2916,20 +2925,8 @@ private AstNode memberExprTail(boolean allowCallSyntax, AstNode pn) throws IOExc

case Token.LB:
consumeToken();
int lb = ts.tokenBeg, rb = -1;
lineno = ts.lineno;
AstNode expr = expr(false);
end = getNodeEnd(expr);
if (mustMatchToken(Token.RB, "msg.no.bracket.index", true)) {
rb = ts.tokenBeg;
end = ts.tokenEnd;
}
ElementGet g = new ElementGet(pos, end - pos);
g.setTarget(pn);
g.setElement(expr);
g.setParens(lb, rb);
g.setLineno(lineno);
pn = g;
pn = makeElemGet(pn, ts.tokenBeg, lineno);
break;

case Token.LP:
@@ -3048,6 +3045,19 @@ private AstNode propertyAccess(int tt, AstNode pn) throws IOException {
ref = propertyName(-1, memberTypeFlags);
break;
}

case Token.LB:
if (tt == Token.QUESTION_DOT) {
// a ?.[ expr ]
consumeToken();
ElementGet g = makeElemGet(pn, ts.tokenBeg, ts.lineno);
g.setType(Token.QUESTION_DOT);
return g;
} else {
reportError("msg.no.name.after.dot");
return makeErrorNode();
}

default:
if (compilerEnv.isReservedKeywordAsIdentifier()) {
// allow keywords as property names, e.g. ({if: 1})
@@ -3065,6 +3075,7 @@ private AstNode propertyAccess(int tt, AstNode pn) throws IOException {
boolean xml = ref instanceof XmlRef;
InfixExpression result = xml ? new XmlMemberGet() : new PropertyGet();
if (xml && tt == Token.DOT) result.setType(Token.DOT);
if (!xml && tt == Token.QUESTION_DOT) result.setType(Token.QUESTION_DOT);
int pos = pn.getPosition();
result.setPosition(pos);
result.setLength(getNodeEnd(ref) - pos);
@@ -3075,6 +3086,23 @@ private AstNode propertyAccess(int tt, AstNode pn) throws IOException {
return result;
}

private ElementGet makeElemGet(AstNode pn, int lb, int lineno) throws IOException {
int pos = pn.getPosition();
AstNode expr = expr(false);
int end = getNodeEnd(expr);
int rb = -1;
if (mustMatchToken(Token.RB, "msg.no.bracket.index", true)) {
rb = ts.tokenBeg;
end = ts.tokenEnd;
}
ElementGet g = new ElementGet(pos, end - pos);
g.setTarget(pn);
g.setElement(expr);
g.setParens(lb, rb);
g.setLineno(lineno);
return g;
}

/**
* Xml attribute expression:
*
Loading

0 comments on commit 83bd883

Please sign in to comment.