Address some MR comments

- add feature flag
- restructure tests accordingly
- add factory method for InvokeCustomMatcherOrThrowNode
- resolve some todos (better error msgs, ...)
- ...

Author: fs-c

graal-js/src/com.oracle.js.parser/src/com/oracle/js/parser/Parser.java

@@ -2864,7 +2864,7 @@ private JavaScriptNode transformAssignmentImpl(Expression assignmentExpression,
                     assert callNode.isWebCompatAssignmentTargetType();
                     assignedNode = factory.createDual(context, transform(lhsExpression), factory.createThrowError(JSErrorType.ReferenceError, INVALID_LHS));
                     // todo-lw: merge
-                    // assignedNode = transformAssignmentExtractor((CallNode) lhsExpression, assignedValue, binaryOp, returnOldValue, convertLHSToNumeric, initializationAssignment);
+                    // assignedNode = transformAssignmentExtractor((CallNode) lhsExpression, assignedValue, initializationAssignment);
                 } else {
                     assignedNode = transformAssignmentIdent((IdentNode) lhsExpression, assignedValue, binaryOp, returnOldValue, convertLHSToNumeric, initializationAssignment);
                 }
@@ -3079,10 +3079,10 @@ private JavaScriptNode transformDestructuringArrayAssignment(Expression lhsExpre
         LiteralNode.ArrayLiteralNode arrayLiteralNode = (LiteralNode.ArrayLiteralNode) lhsExpression;
         List<Expression> elementExpressions = arrayLiteralNode.getElementExpressions();
 
-        return this.transformDestructuringArrayAssignment(elementExpressions, getIterator, valueTempVar, initializationAssignment);
+        return this.transformDestructuringArrayAssignment(elementExpressions, getIterator, valueTempVar.createReadNode(), initializationAssignment);
     }
 
-    private JavaScriptNode transformDestructuringArrayAssignment(List<Expression> elementExpressions, JavaScriptNode getIterator, VarRef valueTempVar, boolean initializationAssignment) {
+    private JavaScriptNode transformDestructuringArrayAssignment(List<Expression> elementExpressions, JavaScriptNode getIterator, JavaScriptNode valueTempNode, boolean initializationAssignment) {
         JavaScriptNode[] initElements = javaScriptNodeArray(elementExpressions.size());
         VarRef iteratorTempVar = environment.createTempVar();
         // By default, we use the hint to track the type of iterator.
@@ -3107,7 +3107,6 @@ private JavaScriptNode transformDestructuringArrayAssignment(List<Expression> el
             if (init != null) {
                 rhsNode = factory.createNotUndefinedOr(rhsNode, transform(init));
             }
-            // todo-lw: this change is kind of sus
             if (lhsExpr != null && (lhsExpr.isTokenType(TokenType.SPREAD_ARRAY) || lhsExpr.isTokenType(TokenType.SPREAD_ARGUMENT))) {
                 rhsNode = factory.createIteratorToArray(context, iteratorTempVar.createReadNode());
                 lhsExpr = ((UnaryNode) lhsExpr).getExpression();
@@ -3122,13 +3121,11 @@ private JavaScriptNode transformDestructuringArrayAssignment(List<Expression> el
         JavaScriptNode resetIterator = iteratorTempVar.createWriteNode(factory.createConstant(JSFrameUtil.DEFAULT_VALUE));
         JavaScriptNode resetValue = valueTempVar.createWriteNode(factory.createConstant(JSFrameUtil.DEFAULT_VALUE));
         return factory.createTryFinally(
-                        factory.createExprBlock(initIteratorTempVar, closeIfNotDone, valueTempVar.createReadNode()),
+                        factory.createExprBlock(initIteratorTempVar, closeIfNotDone, valueTempNode),
                         factory.createExprBlock(resetIterator, resetValue));
     }
 
-    private JavaScriptNode transformAssignmentExtractor(CallNode fakeCallNode, JavaScriptNode assignedValue, BinaryOperation binaryOp, boolean returnOldValue, boolean convertToNumeric, boolean initializationAssignment) {
-        // todo-lw: call node :(
-
+    private JavaScriptNode transformAssignmentExtractor(CallNode fakeCallNode, JavaScriptNode assignedValue, boolean initializationAssignment) {
         final var functionExpr = fakeCallNode.getFunction();
         final var function = transform(functionExpr);
 
@@ -3138,11 +3135,12 @@ private JavaScriptNode transformAssignmentExtractor(CallNode fakeCallNode, JavaS
             receiver = transform(accessNode.getBase());
         }
 
-        final var invokeCustomMatcherOrThrowNode = InvokeCustomMatcherOrThrowNode.create(context, function, assignedValue, receiver);
+        final var invokeCustomMatcherOrThrowNode = factory.createInvokeCustomMatcherOrThrow(context, function, assignedValue, receiver);
 
         final var args = fakeCallNode.getArgs();
         VarRef valueTempVar = environment.createTempVar();
-        return this.transformDestructuringArrayAssignment(args, invokeCustomMatcherOrThrowNode, valueTempVar, initializationAssignment);
+        return this.transformDestructuringArrayAssignment(args, invokeCustomMatcherOrThrowNode,
+                createBlock(valueTempVar.createWriteNode(assignedValue), valueTempVar.createReadNode()), initializationAssignment);
     }
 
     private JavaScriptNode transformDestructuringObjectAssignment(Expression lhsExpression, JavaScriptNode assignedValue, boolean initializationAssignment) {

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

@@ -1,4 +1,4 @@
-load('../assert.js');
+load('../js/assert.js');
 
 const DateExtractor = {
     [Symbol.customMatcher](value) {

graal-js/src/com.oracle.truffle.js.test/js/extractors/as-default.js renamed to graal-js/src/com.oracle.truffle.js.test/js-extractors/as-default.js

@@ -1,4 +1,4 @@
-load('../assert.js');
+load('../js/assert.js');
 
 {
     class C {
@@ -15,10 +15,26 @@ load('../assert.js');
 
     const subject = new C("data");
 
-    let x;
-    C(x) = subject;
+    {
+        let x;
+        C(x) = subject;
+        assertSame("data", x);
+    }
+
+    {
+        let C(x) = subject;
+        assertSame("data", x);
+    }
+
+    {
+        var C(x) = subject;
+        assertSame("data", x);
+    }
 
-    assertSame(x, "data");
+    {
+        const C(x) = subject;
+        assertSame("data", x);
+    }
 }
 
 {
@@ -34,11 +50,20 @@ load('../assert.js');
 
     const subject = new C({ x: 1, y: 2 });
 
-    let x, y;
-    C({ x, y }) = subject;
+    {
+        let x, y;
+        C({x, y}) = subject;
+
+        assertSame(1, x);
+        assertSame(2, y);
+    }
+
+    {
+        let C({x, y}) = subject;
 
-    assertSame(x, 1);
-    assertSame(y, 2);
+        assertSame(1, x);
+        assertSame(2, y);
+    }
 }
 
 {
@@ -56,9 +81,18 @@ load('../assert.js');
 
     const subject = new C(undefined, 2);
 
-    const C(x = -1, y) = subject;
-    assertSame(x, -1);
-    assertSame(y, 2);
+    {
+        let x = -1, y = 100;
+        C(x = -1, y) = subject;
+        assertSame(-1, x);
+        assertSame(2, y);
+    }
+
+    {
+        const C(x = -1, y) = subject;
+        assertSame(-1, x);
+        assertSame(2, y);
+    }
 }
 
 {
@@ -80,7 +114,22 @@ load('../assert.js');
 
     const subject = new C(1, 2, 3);
 
-    const C(x, ...y) = subject;
-    assertSame(x, 1);
-    assertSameContent(y, [2, 3]);
+    {
+        let x, y;
+        C(x, ...y) = subject;
+        assertSame(1, x);
+        assertSameContent([2, 3], y);
+    }
+
+    {
+        const C(x, ...y) = subject;
+        assertSame(1, x);
+        assertSameContent([2, 3], y);
+    }
+
+    {
+        assertSame(subject, C() = subject)
+        assertSame(subject, C(x) = subject)
+        assertSame(subject, C(x, ...y) = subject)
+    }
 }

graal-js/src/com.oracle.truffle.js.test/js/extractors/assignment.js renamed to graal-js/src/com.oracle.truffle.js.test/js-extractors/assignment-binding.js

@@ -0,0 +1,25 @@
+load('../js/assert.js');
+
+{
+    class C {
+        #data;
+
+        constructor(data) {
+            this.#data = data;
+        }
+
+        static [Symbol.customMatcher](subject) {
+            return undefined;
+        }
+    }
+
+    const subject = new C("data");
+
+    try {
+        const C(x) = subject;
+        assertTrue(false);
+    } catch (e) {
+        // ensure a meaningful error message is thrown
+        assertTrue(e.message.includes("customMatcher"));
+    }
+}

graal-js/src/com.oracle.truffle.js.test/js-extractors/error.js

@@ -1,4 +1,4 @@
-load('../assert.js');
+load('../js/assert.js');
 
 class C {
     #data1;
@@ -25,3 +25,9 @@ const subject = new C(new D("data"));
 const C(D(x)) = subject;
 
 assertSame(x, "data");
+
+const { a: C(D(y)) } = { a: subject };
+assertSame(y, "data");
+
+const { a: { b: C(D(z)) } } = { a: { b: subject } };
+assertSame(z, "data");``

graal-js/src/com.oracle.truffle.js.test/js/extractors/nested.js renamed to graal-js/src/com.oracle.truffle.js.test/js-extractors/nested.js

@@ -1,4 +1,4 @@
-load('../assert.js');
+load('../js/assert.js');
 
 const MapExtractor = {
     [Symbol.customMatcher](map) {

graal-js/src/com.oracle.truffle.js.test/js/extractors/object.js renamed to graal-js/src/com.oracle.truffle.js.test/js-extractors/object.js

@@ -1,4 +1,4 @@
-load('../assert.js');
+load('../js/assert.js');
 
 class C {
     #f;

graal-js/src/com.oracle.truffle.js.test/js/extractors/receiver.js renamed to graal-js/src/com.oracle.truffle.js.test/js-extractors/receiver.js

@@ -1,4 +1,4 @@
-load('../assert.js');
+load('../js/assert.js');
 
 // potentially built-in as part of Pattern Matching
 RegExp.prototype[Symbol.customMatcher] = function (value) {