Skip to content

Commit

Permalink
Handle super.x++ and similar
Browse files Browse the repository at this point in the history
  • Loading branch information
@andreabergia
andreabergia committed Nov 27, 2024
1 parent b45336f commit 37c3998
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 17 deletions.
79 changes: 79 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1266,6 +1266,12 @@ private CompleteOptionalCallJump completeOptionalCallJump() {
private void visitIncDec(Node node, Node child) {
int incrDecrMask = node.getExistingIntProp(Node.INCRDECR_PROP);
int childType = child.getType();

if (child.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) {
visitSuperIncDec(node, child, childType, incrDecrMask);
return;
}

switch (childType) {
case Token.GETVAR:
{
Expand Down Expand Up @@ -1319,6 +1325,79 @@ private void visitIncDec(Node node, Node child) {
}
}

// Handles super.x++ and variants thereof. We don't want to create new icode in the interpreter
// for this edge case, so we will transform this into something like super.x = super.x + 1
private void visitSuperIncDec(Node node, Node child, int childType, int incrDecrMask) {
Node object = child.getFirstChild();

// Push the old value on the stack
visitExpression(object, 0); // stack: [super]
switch (childType) {
case Token.GETPROP:
addStringOp(Token.GETPROP_SUPER, object.getNext().getString()); // stack: [p]
break;

case Token.GETELEM:
{
Node index = object.getNext();
visitExpression(index, 0); // stack: [super, elem]
addToken(Token.GETELEM_SUPER); // stack: [p]
stackChange(-1);
break;
}

default:
throw badTree(node);
}

// If it's a postfix expression, we copy the old value
// If it's postfix, we only need the _new_ value on the stack
if ((incrDecrMask & Node.POST_FLAG) != 0) {
addIcode(Icode_DUP); // stack: postfix [p, p], prefix: [p]
stackChange(+1);
}

// We need, in order, super and then the new value
addToken(Token.SUPER); // stack: postfix [p, p, super], prefix: [p, super]
stackChange(+1);
addIcode(Icode_SWAP); // stack: postfix [p, super, p], prefix: [super, p]

// Increment or decrement the new value
addIcode(Icode_ONE); // stack: prefix [p, super, p, 1], postfix: [super, p, 1]
stackChange(+1);
if ((incrDecrMask & Node.DECR_FLAG) == 0) {
addToken(Token.ADD); // stack: prefix [p, super, p+1], postfix: [super, p+1]
} else {
addToken(Token.SUB); // stack: prefix [p, super, p-1], postfix: [super, p-1]
}
stackChange(-1);

// Assign the new value to the property
switch (childType) {
case Token.GETPROP:
addStringOp(Token.SETPROP_SUPER, object.getNext().getString());
// stack: prefix [p, p+-1], postfix: [p+-1]
stackChange(-1);
break;

case Token.GETELEM:
{
Node index = object.getNext();
visitExpression(index, 0);
// stack: prefix [p, super, p+-1, elem], postfix: [super, p+-1, elem]
addToken(Token.SETELEM_SUPER); // stack: prefix [p, p+-1], postfix: [p+-1]
stackChange(-2);
break;
}
}

// If it was a postfix, just drop the new value
if ((incrDecrMask & Node.POST_FLAG) != 0) {
addIcode(Icode_POP); // stack: [p]
stackChange(-1);
}
}

private void visitLiteral(Node node, Node child) {
int type = node.getType();
if (type == Token.ARRAYLIT) {
Expand Down
84 changes: 84 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3564,6 +3564,12 @@ private void addInstructionCount(int count) {
private void visitIncDec(Node node) {
int incrDecrMask = node.getExistingIntProp(Node.INCRDECR_PROP);
Node child = node.getFirstChild();

if (child.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) {
visitSuperIncDec(node, child, incrDecrMask);
return;
}

switch (child.getType()) {
case Token.GETVAR:
if (!hasVarsInRegs) Kit.codeBug();
Expand Down Expand Up @@ -3728,6 +3734,84 @@ private void visitIncDec(Node node) {
}
}

// Handles super.x++ and variants thereof. We don't want to create new icode in the interpreter
// for this edge case, so we will transform this into something like super.x = super.x + 1
private void visitSuperIncDec(Node node, Node child, int incrDecrMask) {
Node object = child.getFirstChild();

// Push the old value on the stack
generateExpression(object, node); // [super]
cfw.add(ByteCode.DUP); // [super, super]
switch (child.getType()) {
case Token.GETPROP:
cfw.addALoad(contextLocal);
cfw.addALoad(variableObjectLocal);
cfw.addALoad(thisObjLocal);
cfw.addLoadConstant(0); // no_warn flag
addDynamicInvoke(
"PROP:GETSUPER:" + child.getFirstChild().getNext().getString(),
Signatures.PROP_GET_SUPER);
break;

case Token.GETELEM:
generateExpression(object.getNext(), node);
cfw.addALoad(contextLocal);
cfw.addALoad(variableObjectLocal);
cfw.addALoad(thisObjLocal);
addDynamicInvoke("PROP:GETELEMENTSUPER", Signatures.PROP_GET_ELEMENT_SUPER);
break;

default:
Codegen.badTree();
}

// stack: [super, p]
if ((incrDecrMask & Node.POST_FLAG) != 0) {
// For postfix we want a copy of the old value on the stack
cfw.add(ByteCode.SWAP); // [p, super]
cfw.add(ByteCode.DUP2); // [p, super, p, super]
cfw.add(ByteCode.POP); // [p, super, p]
}

// Increment or decrement the value
addObjectToDouble(); // Unbox
cfw.addPush(1.0);
if ((incrDecrMask & Node.DECR_FLAG) == 0) {
cfw.add(ByteCode.DADD);
} else {
cfw.add(ByteCode.DSUB);
}
addDoubleWrap(); // Box back

// Assign the new value to the property
// Now stack is for prefix: [super, p+-1] and for postfix: [p, super, p+-1]
switch (child.getType()) {
case Token.GETPROP:
cfw.addALoad(contextLocal);
cfw.addALoad(variableObjectLocal);
cfw.addALoad(thisObjLocal);
addDynamicInvoke(
"PROP:SETSUPER:" + child.getFirstChild().getNext().getString(),
Signatures.PROP_SET_SUPER);
break;

case Token.GETELEM:
generateExpression(object.getNext(), node); // [..., super, p+-1, elem]
cfw.add(ByteCode.SWAP); // [..., super, elem, p+-1]
cfw.addALoad(contextLocal);
cfw.addALoad(variableObjectLocal);
cfw.addALoad(thisObjLocal);
addDynamicInvoke("PROP:SETELEMENTSUPER", Signatures.PROP_SET_ELEMENT_SUPER);
break;
}
// Now stack is for prefix: [p+-1] and for postfix: [p, p+-1]

// If it was a postfix, just drop the new value
if ((incrDecrMask & Node.POST_FLAG) != 0) {
cfw.add(ByteCode.POP);
}
}

private static boolean isArithmeticNode(Node node) {
int type = node.getType();
return (type == Token.SUB)
Expand Down
4 changes: 4 additions & 0 deletions rhino/src/main/java/org/mozilla/javascript/optimizer/Optimizer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ private int rewriteForNumberVariables(Node n, int desired) {
case Token.DEC:
{
Node child = n.getFirstChild();
if (child.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) {
// Don't optimize super.prop++ and related
return NoType;
}
int type = rewriteForNumberVariables(child, NumberType);
if (child.getType() == Token.GETVAR) {
if (type == NumberType && !convertParameter(child)) {
Expand Down
34 changes: 17 additions & 17 deletions rhino/src/main/java/org/mozilla/javascript/optimizer/Signatures.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,11 @@ interface Signatures {
* string, number, or symbol. Falls back to ScriptRuntime.getSuperElem.
*/
String PROP_GET_ELEMENT_SUPER =
"(Ljava/lang/Object;"
+ "Ljava/lang/Object;"
+ "Lorg/mozilla/javascript/Context;"
+ "Lorg/mozilla/javascript/Scriptable;"
+ "Ljava/lang/Object;"
"(Ljava/lang/Object;" // super
+ "Ljava/lang/Object;" // elem
+ "Lorg/mozilla/javascript/Context;" // cx
+ "Lorg/mozilla/javascript/Scriptable;" // scope
+ "Ljava/lang/Object;" // this
+ ")Ljava/lang/Object;";

/**
Expand All @@ -110,11 +110,11 @@ interface Signatures {
*/
String PROP_SET_SUPER =
"("
+ "Ljava/lang/Object;"
+ "Ljava/lang/Object;"
+ "Lorg/mozilla/javascript/Context;"
+ "Lorg/mozilla/javascript/Scriptable;"
+ "Ljava/lang/Object;"
+ "Ljava/lang/Object;" // superObj
+ "Ljava/lang/Object;" // value
+ "Lorg/mozilla/javascript/Context;" // cx
+ "Lorg/mozilla/javascript/Scriptable;" // scope
+ "Ljava/lang/Object;" // thisObj
+ ")Ljava/lang/Object;";

/**
Expand Down Expand Up @@ -155,16 +155,16 @@ interface Signatures {
+ ")Ljava/lang/Object;";

/**
* PROP:SETELEMENT_SUPER: Set a property on super based on an identifier. Falls back to
* PROP:SETELEMENTSUPER: Set a property on super based on an identifier. Falls back to
* ScriptRuntime.setSuperElem.
*/
String PROP_SET_ELEMENT_SUPER =
"(Ljava/lang/Object;"
+ "Ljava/lang/Object;"
+ "Ljava/lang/Object;"
+ "Lorg/mozilla/javascript/Context;"
+ "Lorg/mozilla/javascript/Scriptable;"
+ "Ljava/lang/Object;"
"(Ljava/lang/Object;" // super
+ "Ljava/lang/Object;" // elem
+ "Ljava/lang/Object;" // value
+ "Lorg/mozilla/javascript/Context;" // cx
+ "Lorg/mozilla/javascript/Scriptable;" // scope
+ "Ljava/lang/Object;" // this
+ ")Ljava/lang/Object;";

/**
Expand Down
61 changes: 61 additions & 0 deletions rhino/src/test/java/org/mozilla/javascript/SuperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,7 @@ void missingPropertyThisSealedIsErrorInStrictMode() {

@Test
void modifyOperatorByName() {
// Equivalent to `super.x = super.x + 1`, so reads from super, writes in this
String script =
""
+ "var proto = { x: 'proto' };"
Expand All @@ -748,6 +749,66 @@ void modifyOperatorByName() {
+ "object.x + ':' + proto.x";
Utils.assertWithAllOptimizationLevelsES6("proto1:proto", script);
}

@Test
void memberIncrementPostfix() {
String script =
""
+ "var proto = { x: 1 };"
+ "var object = {\n"
+ " x: 42,\n"
+ " f() { return super.x++; }\n"
+ "};\n"
+ "Object.setPrototypeOf(object, proto);\n"
+ "var f = object.f();"
+ "f + ':' + object.x + ':' + proto.x";
Utils.assertWithAllOptimizationLevelsES6("1:2:1", script);
}

@Test
void memberIncrementPrefix() {
String script =
""
+ "var proto = { x: 1 };"
+ "var object = {\n"
+ " x: 42,\n"
+ " f() { return ++super.x; }\n"
+ "};\n"
+ "Object.setPrototypeOf(object, proto);\n"
+ "var f = object.f();"
+ "f + ':' + object.x + ':' + proto.x";
Utils.assertWithAllOptimizationLevelsES6("2:2:1", script);
}

@Test
void elementDecrementPostfix() {
String script =
""
+ "var proto = { 0: 1 };"
+ "var object = {\n"
+ " 0: 42,\n"
+ " f() { return super[0]--; }\n"
+ "};\n"
+ "Object.setPrototypeOf(object, proto);\n"
+ "var f = object.f();"
+ "f + ':' + object[0] + ':' + proto[0]";
Utils.assertWithAllOptimizationLevelsES6("1:0:1", script);
}

@Test
void elementDecrementPrefix() {
String script =
""
+ "var proto = { 0: 1 };"
+ "var object = {\n"
+ " 0: 42,\n"
+ " f() { return --super[0]; }\n"
+ "};\n"
+ "Object.setPrototypeOf(object, proto);\n"
+ "var f = object.f();"
+ "f + ':' + object[0] + ':' + proto[0]";
Utils.assertWithAllOptimizationLevelsES6("0:0:1", script);
}
}

@Nested
Expand Down

0 comments on commit 37c3998

Please sign in to comment.