Reimplemented nullish coalesce operator

mozilla · Oct 23, 2024 · 9301e8c · 9301e8c
1 parent 83bd883
commit 9301e8c
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,18 +42,34 @@ 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"
                         + "var eval1 = f() ?? 42; \n"
                         + "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
@@ -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);
+    }
 }