[GR-24939] Do not unwrap OptionalChainNodes that are not part of a larger optional chain.

PullRequest: js/1591

Author: iamstolis

graal-js/src/com.oracle.truffle.js.parser/src/com/oracle/truffle/js/parser/GraalJSTranslator.java

@@ -2162,7 +2162,14 @@ private JavaScriptNode enterDeleteProperty(UnaryNode deleteNode) {
     }
 
     private JavaScriptNode filterOptionalChainTarget(JavaScriptNode target, boolean optional) {
-        JavaScriptNode innerAccess = target instanceof OptionalChainNode ? ((OptionalChainNode) target).getAccessNode() : target;
+        JavaScriptNode innerAccess;
+        if (target instanceof OptionalChainNode) {
+            innerAccess = ((OptionalChainNode) target).getAccessNode();
+        } else if (target instanceof OptionalChainNode.OptionalTargetableNode) {
+            innerAccess = ((OptionalChainNode.OptionalTargetableNode) target).getDelegateNode();
+        } else {
+            innerAccess = target;
+        }
         if (optional) {
             innerAccess = factory.createOptionalChainShortCircuit(innerAccess);
         }
@@ -2193,7 +2200,9 @@ private JavaScriptNode enterNewNode(UnaryNode unaryNode) {
     public JavaScriptNode enterCallNode(CallNode callNode) {
         JavaScriptNode function = transform(callNode.getFunction());
         JavaScriptNode[] args = transformArgs(callNode.getArgs());
-        function = filterOptionalChainTarget(function, callNode.isOptional());
+        if (callNode.isOptionalChain()) {
+            function = filterOptionalChainTarget(function, callNode.isOptional());
+        }
         JavaScriptNode call;
         if (callNode.isEval() && args.length >= 1) {
             call = createCallEvalNode(function, args);

graal-js/src/com.oracle.truffle.js.test/js/GR-24939.js

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2020, 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * Licensed under the Universal Permissive License v 1.0 as shown at http://oss.oracle.com/licenses/upl.
+ */
+
+load('assert.js');
+
+var argumentsEvaluated = false;
+var f = function () {
+    argumentsEvaluated = true;
+};
+assertThrows(function () {
+    (undefined?.foo)(f());
+}, TypeError);
+assertSame(true, argumentsEvaluated);
+
+true;

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/NodeFactory.java

@@ -585,7 +585,7 @@ public JavaScriptNode createTryFinally(JavaScriptNode tryNode, JavaScriptNode fi
 
     public JavaScriptNode createFunctionCall(@SuppressWarnings("unused") JSContext context, JavaScriptNode expression, JavaScriptNode[] arguments) {
         if (expression instanceof PropertyNode || expression instanceof ReadElementNode || expression instanceof WithVarWrapperNode || expression instanceof PrivateFieldGetNode ||
-                        expression instanceof OptionalChainNode.ShortCircuitTargetableNode) {
+                        expression instanceof OptionalChainNode.ShortCircuitTargetableNode || expression instanceof OptionalChainNode.OptionalTargetableNode) {
             assert !(expression instanceof PropertyNode) || ((PropertyNode) expression).isMethod();
             return JSFunctionCallNode.createInvoke((JSTargetableNode) expression, arguments, false, false);
         } else if (expression instanceof JSTargetableWrapperNode) {

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/nodes/access/OptionalChainNode.java

@@ -66,7 +66,12 @@ protected OptionalChainNode(JavaScriptNode chainNode, Object result) {
     }
 
     public static JavaScriptNode createTarget(JavaScriptNode chainNode) {
-        return new OptionalChainNode(chainNode, chainNode instanceof DeletePropertyNode ? Boolean.TRUE : Undefined.instance);
+        Object result = (chainNode instanceof DeletePropertyNode) ? Boolean.TRUE : Undefined.instance;
+        if (chainNode instanceof JSTargetableNode) {
+            return new OptionalTargetableNode((JSTargetableNode) chainNode, result);
+        } else {
+            return new OptionalChainNode(chainNode, result);
+        }
     }
 
     public static JavaScriptNode createShortCircuit(JavaScriptNode expressionNode) {
@@ -120,6 +125,58 @@ public JavaScriptNode getAccessNode() {
         return accessNode;
     }
 
+    public static final class OptionalTargetableNode extends JSTargetableNode {
+        private static final Object SHORT_CIRCUIT_MARKER = new Object();
+        @Child private JSTargetableNode delegateNode;
+        private final Object result;
+
+        protected OptionalTargetableNode(JSTargetableNode delegateNode, Object result) {
+            this.delegateNode = delegateNode;
+            this.result = result;
+        }
+
+        @Override
+        public JavaScriptNode getTarget() {
+            return delegateNode.getTarget();
+        }
+
+        @Override
+        public Object evaluateTarget(VirtualFrame frame) {
+            try {
+                return delegateNode.evaluateTarget(frame);
+            } catch (ShortCircuitException ex) {
+                return SHORT_CIRCUIT_MARKER;
+            }
+        }
+
+        @Override
+        public Object executeWithTarget(VirtualFrame frame, Object target) {
+            if (target == SHORT_CIRCUIT_MARKER) {
+                return result;
+            }
+            return delegateNode.executeWithTarget(frame, target);
+        }
+
+        @Override
+        public Object execute(VirtualFrame frame) {
+            try {
+                return delegateNode.execute(frame);
+            } catch (ShortCircuitException ex) {
+                return result;
+            }
+        }
+
+        @Override
+        protected JavaScriptNode copyUninitialized(Set<Class<? extends Tag>> materializedTags) {
+            return new OptionalTargetableNode(cloneUninitialized(delegateNode, materializedTags), result);
+        }
+
+        public JavaScriptNode getDelegateNode() {
+            return delegateNode;
+        }
+
+    }
+
     /**
      * Evaluates an optional expression and if its value is null or undefined, jumps out of the
      * short-circuiting expression to the parent {@link OptionalChainNode}.