From 9301e8c32c8ea31a46dd961923faf0b60fbad796 Mon Sep 17 00:00:00 2001 From: "andrea.bergia" Date: Fri, 11 Oct 2024 14:26:32 +0200 Subject: [PATCH] Reimplemented nullish coalesce operator --- .../org/mozilla/javascript/CodeGenerator.java | 19 ++++++++++++ .../org/mozilla/javascript/IRFactory.java | 30 +------------------ .../java/org/mozilla/javascript/Icode.java | 6 ++-- .../org/mozilla/javascript/Interpreter.java | 12 ++++++++ .../tests/NullishCoalescingOpTest.java | 27 +++++++++++++---- .../tests/OptionalChainingOperatorTest.java | 6 ++++ 6 files changed, 64 insertions(+), 36 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java b/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java index d09770cc11..99d62483b5 100644 --- a/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java +++ b/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java @@ -1115,6 +1115,25 @@ private void visitExpression(Node node, int contextFlags) { visitTemplateLiteral(node); break; + case Token.NULLISH_COALESCING: + { + visitExpression(child, 0); + child = child.getNext(); + + addIcode(Icode_DUP); + stackChange(1); + int end = iCodeTop; + addGotoOp(Icode.Icode_IF_NOT_NULL_UNDEF); + stackChange(-1); + + addIcode(Icode_POP); + visitExpression(child, 0); + stackChange(-1); + + resolveForwardGoto(end); + break; + } + default: throw badTree(node); } diff --git a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java index 26b3bdcf9b..073efaa9d5 100644 --- a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java +++ b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java @@ -807,35 +807,7 @@ private Node transformIf(IfStatement n) { private Node transformInfix(InfixExpression node) { Node left = transform(node.getLeft()); Node right = transform(node.getRight()); - return (node.getType() == Token.NULLISH_COALESCING) - ? transformNullishCoalescing(left, right, node) - : createBinary(node.getType(), left, right); - } - - private Node transformNullishCoalescing(Node left, Node right, Node parent) { - String tempName = parser.currentScriptOrFn.getNextTempName(); - - Node nullNode = new Node(Token.NULL); - Node undefinedNode = new Name(0, "undefined"); - - Node conditional = - new Node( - Token.OR, - new Node(Token.SHEQ, nullNode, parser.createName(tempName)), - new Node(Token.SHEQ, undefinedNode, parser.createName(tempName))); - - Node hookNode = - new Node( - Token.HOOK, - /* left= */ conditional, - /* mid= */ right, - /* right= */ parser.createName(tempName)); - - parser.defineSymbol(Token.LP, tempName, true); - return createBinary( - Token.COMMA, - createAssignment(Token.ASSIGN, parser.createName(tempName), left), - hookNode); + return createBinary(node.getType(), left, right); } private Node transformLabeledStatement(LabeledStatement ls) { diff --git a/rhino/src/main/java/org/mozilla/javascript/Icode.java b/rhino/src/main/java/org/mozilla/javascript/Icode.java index 242b8b5d6b..b5a288cf32 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Icode.java +++ b/rhino/src/main/java/org/mozilla/javascript/Icode.java @@ -145,9 +145,9 @@ abstract class Icode { // Jump if stack head is null or undefined Icode_IF_NULL_UNDEF = Icode_LITERAL_KEY_SET - 1, - + Icode_IF_NOT_NULL_UNDEF = Icode_IF_NULL_UNDEF - 1, // Last icode - MIN_ICODE = Icode_IF_NULL_UNDEF; + MIN_ICODE = Icode_IF_NOT_NULL_UNDEF; static String bytecodeName(int bytecode) { if (!validBytecode(bytecode)) { @@ -315,6 +315,8 @@ static String bytecodeName(int bytecode) { return "LITERAL_KEY_SET"; case Icode_IF_NULL_UNDEF: return "IF_NULL_UNDEF"; + case Icode_IF_NOT_NULL_UNDEF: + return "IF_NOT_NULL_UNDEF"; } // icode without name diff --git a/rhino/src/main/java/org/mozilla/javascript/Interpreter.java b/rhino/src/main/java/org/mozilla/javascript/Interpreter.java index 7265325bdd..d1bc7ca67f 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Interpreter.java +++ b/rhino/src/main/java/org/mozilla/javascript/Interpreter.java @@ -552,6 +552,7 @@ static void dumpICode(InterpreterData idata) { case Token.IFNE: case Icode_IFEQ_POP: case Icode_IF_NULL_UNDEF: + case Icode_IF_NOT_NULL_UNDEF: case Icode_LEAVEDQ: { int newPC = pc + getShort(iCode, pc) - 1; @@ -823,6 +824,7 @@ private static int bytecodeSpan(int bytecode) { case Token.IFNE: case Icode_IFEQ_POP: case Icode_IF_NULL_UNDEF: + case Icode_IF_NOT_NULL_UNDEF: case Icode_LEAVEDQ: // target pc offset return 1 + 2; @@ -1446,6 +1448,16 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl } break jumplessRun; } + case Icode_IF_NOT_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: diff --git a/tests/src/test/java/org/mozilla/javascript/tests/NullishCoalescingOpTest.java b/tests/src/test/java/org/mozilla/javascript/tests/NullishCoalescingOpTest.java index e2999167a3..18825dd68e 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/NullishCoalescingOpTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/NullishCoalescingOpTest.java @@ -8,7 +8,7 @@ public class NullishCoalescingOpTest { @Test - public void testNullishCoalescingOperatorRequiresEs6() { + public void testNullishCoalescingOperatorRequiresES6() { Utils.runWithAllOptimizationLevels( cx -> { Scriptable scope = cx.initStandardObjects(); @@ -20,13 +20,14 @@ public void testNullishCoalescingOperatorRequiresEs6() { } @Test - public void testNullishColascingBasic() { + public void testNullishCoalescingBasic() { + Utils.assertWithAllOptimizationLevelsES6("val", "'val' ?? 'default string'"); Utils.assertWithAllOptimizationLevelsES6("default string", "null ?? 'default string'"); Utils.assertWithAllOptimizationLevelsES6("default string", "undefined ?? 'default string'"); } @Test - public void testNullishColascingShortCircuit() { + public void testNullishCoalescingShortCircuit() { String script = "0 || 0 ?? true"; Utils.assertEvaluatorExceptionES6("Syntax Error: Unexpected token. (test#1)", script); @@ -41,13 +42,13 @@ public void testNullishColascingShortCircuit() { } @Test - public void testNullishColascingPrecedence() { + public void testNullishCoalescingPrecedence() { Utils.assertWithAllOptimizationLevelsES6( "yes", "3 == 3 ? 'yes' ?? 'default string' : 'no'"); } @Test - public void testNullishColascingEvalOnce() { + public void testNullishCoalescingEvalOnce() { String script = "var runs = 0; \n" + "function f() { runs++; return 3; } \n" @@ -55,4 +56,20 @@ public void testNullishColascingEvalOnce() { + "runs"; Utils.assertWithAllOptimizationLevelsES6(1, script); } + + @Test + public void testNullishCoalescingDoesNotEvaluateRightHandSideIfNotNecessary() { + String script = + "var runs = 0; \n" + + "function f() { runs++; return 3; } \n" + + "var eval1 = 42 ?? f(); \n" + + "runs"; + Utils.assertWithAllOptimizationLevelsES6(0, script); + } + + @Test + public void testNullishCoalescingDoesNotLeakVariables() { + String script = "$0 = false; true ?? true; $0"; + Utils.assertWithAllOptimizationLevelsES6(false, script); + } } diff --git a/tests/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTest.java b/tests/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTest.java index e6088d40d9..3052a84410 100644 --- a/tests/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTest.java +++ b/tests/src/test/java/org/mozilla/javascript/tests/OptionalChainingOperatorTest.java @@ -89,4 +89,10 @@ public void leftHandSideIsEvaluatedOnlyOnce() { + "f()?.length;\n" + "counter\n"); } + + @Test + public void doesNotLeakVariables() { + String script = "$0 = false; o = {}; o?.x; $0"; + Utils.assertWithAllOptimizationLevelsES6(false, script); + } }