Skip to content

Commit bdd4888

Browse files
fs-cfs-c
committed
Address some MR comments
- add feature flag - restructure tests accordingly - add factory method for InvokeCustomMatcherOrThrowNode
1 parent e855928 commit bdd4888

File tree

13 files changed

+144
-68
lines changed

13 files changed

+144
-68
lines changed

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

Lines changed: 39 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ public class Parser extends AbstractParser {
244244
private static final boolean ES2019_OPTIONAL_CATCH_BINDING = Options.getBooleanProperty("parser.optional.catch.binding", true);
245245
private static final boolean ES2020_CLASS_FIELDS = Options.getBooleanProperty("parser.class.fields", true);
246246
private static final boolean ES2022_TOP_LEVEL_AWAIT = Options.getBooleanProperty("parser.top.level.await", true);
247+
private static final boolean ESNEXT_EXTRACTORS = Options.getBooleanProperty("parser.extractors", true);
247248

248249
private static final int REPARSE_IS_PROPERTY_ACCESSOR = 1 << 0;
249250
private static final int REPARSE_IS_METHOD = 1 << 1;
@@ -1046,7 +1047,7 @@ private Expression verifyAssignment(final long op, final Expression lhs, final E
10461047
throw invalidLHSError(lhs);
10471048
}
10481049
break;
1049-
} else if ((opType == ASSIGN || opType == ASSIGN_INIT) && (isDestructuringLhs(lhs) || isExtractorLhs(lhs)) && (inPatternPosition || !lhs.isParenthesized())) {
1050+
} else if ((opType == ASSIGN || opType == ASSIGN_INIT) && isDestructuringOrExtractorLhs(lhs) && (inPatternPosition || !lhs.isParenthesized())) {
10501051
verifyDestructuringAssignmentPattern(lhs, CONTEXT_ASSIGNMENT_TARGET);
10511052
break;
10521053
} else {
@@ -1060,20 +1061,19 @@ private Expression verifyAssignment(final long op, final Expression lhs, final E
10601061
return new BinaryNode(op, lhs, rhsExpr);
10611062
}
10621063

1063-
private boolean isDestructuringLhs(Expression lhs) {
1064+
private boolean isDestructuringOrExtractorLhs(Expression lhs) {
10641065
if (lhs instanceof ObjectNode || lhs instanceof ArrayLiteralNode) {
10651066
return ES6_DESTRUCTURING && isES6();
10661067
}
1068+
if (lhs instanceof CallNode) {
1069+
return ESNEXT_EXTRACTORS;
1070+
}
10671071
return false;
10681072
}
10691073

1070-
private boolean isExtractorLhs(Expression lhs) {
1071-
// todo-lw: call node :(
1072-
return lhs instanceof CallNode;
1073-
}
10741074

10751075
private void verifyDestructuringAssignmentPattern(Expression pattern, String contextString) {
1076-
assert pattern instanceof ObjectNode || pattern instanceof ArrayLiteralNode;
1076+
assert pattern instanceof ObjectNode || pattern instanceof ArrayLiteralNode || pattern instanceof CallNode;
10771077
pattern.accept(new VerifyDestructuringPatternNodeVisitor(new LexicalContext()) {
10781078
@Override
10791079
protected void verifySpreadElement(Expression lvalue) {
@@ -1108,26 +1108,6 @@ public boolean enterIndexNode(IndexNode indexNode) {
11081108
return false;
11091109
}
11101110

1111-
@Override
1112-
public boolean enterCallNode(CallNode callNode) {
1113-
if (callNode.isParenthesized()) {
1114-
throw error(AbstractParser.message(MSG_INVALID_LVALUE), callNode.getToken());
1115-
}
1116-
// todo-lw: surely there is a better way to do this
1117-
for (final var arg : callNode.getArgs()) {
1118-
if (arg instanceof IdentNode) {
1119-
enterIdentNode((IdentNode) arg);
1120-
} else if (arg instanceof LiteralNode<?>) {
1121-
enterLiteralNode((LiteralNode<?>) arg);
1122-
} else if (arg instanceof ObjectNode) {
1123-
enterObjectNode((ObjectNode) arg);
1124-
} else {
1125-
enterDefault(arg);
1126-
}
1127-
}
1128-
return false;
1129-
}
1130-
11311111
@Override
11321112
protected boolean enterDefault(Node node) {
11331113
throw error(String.format("unexpected node in AssignmentPattern: %s", node));
@@ -2834,20 +2814,40 @@ public boolean enterBinaryNode(BinaryNode binaryNode) {
28342814
return enterDefault(binaryNode);
28352815
}
28362816
}
2817+
2818+
@Override
2819+
public boolean enterCallNode(CallNode callNode) {
2820+
if (callNode.isParenthesized()) {
2821+
throw error(AbstractParser.message(MSG_INVALID_LVALUE), callNode.getToken());
2822+
}
2823+
// todo-lw: surely there is a better way to do this
2824+
for (final var arg : callNode.getArgs()) {
2825+
if (arg instanceof IdentNode) {
2826+
enterIdentNode((IdentNode) arg);
2827+
} else if (arg instanceof LiteralNode<?>) {
2828+
enterLiteralNode((LiteralNode<?>) arg);
2829+
} else if (arg instanceof ObjectNode) {
2830+
enterObjectNode((ObjectNode) arg);
2831+
} else {
2832+
enterDefault(arg);
2833+
}
2834+
}
2835+
return false;
2836+
}
28372837
}
28382838

28392839
/**
28402840
* Verify destructuring variable declaration binding pattern and extract bound variable
28412841
* declarations.
28422842
*/
28432843
private void verifyDestructuringBindingPattern(Expression pattern, Consumer<IdentNode> identifierCallback) {
2844-
assert pattern instanceof ObjectNode || pattern instanceof ArrayLiteralNode;
2844+
assert pattern instanceof ObjectNode || pattern instanceof ArrayLiteralNode || pattern instanceof CallNode;
28452845
pattern.accept(new VerifyDestructuringPatternNodeVisitor(new LexicalContext()) {
28462846
@Override
28472847
protected void verifySpreadElement(Expression lvalue) {
28482848
if (lvalue instanceof IdentNode) {
28492849
enterIdentNode((IdentNode) lvalue);
2850-
} else if (isDestructuringLhs(lvalue)) {
2850+
} else if (isDestructuringOrExtractorLhs(lvalue)) {
28512851
verifyDestructuringBindingPattern(lvalue, identifierCallback);
28522852
} else {
28532853
throw error("Expected a valid binding identifier", lvalue.getToken());
@@ -2863,26 +2863,6 @@ public boolean enterIdentNode(IdentNode identNode) {
28632863
return false;
28642864
}
28652865

2866-
// todo-lw: this is duplicate code
2867-
@Override
2868-
public boolean enterCallNode(CallNode callNode) {
2869-
if (callNode.isParenthesized()) {
2870-
throw error(AbstractParser.message(MSG_INVALID_LVALUE), callNode.getToken());
2871-
}
2872-
for (final var arg : callNode.getArgs()) {
2873-
if (arg instanceof IdentNode) {
2874-
enterIdentNode((IdentNode) arg);
2875-
} else if (arg instanceof LiteralNode<?>) {
2876-
enterLiteralNode((LiteralNode<?>) arg);
2877-
} else if (arg instanceof ObjectNode) {
2878-
enterObjectNode((ObjectNode) arg);
2879-
} else {
2880-
enterDefault(arg);
2881-
}
2882-
}
2883-
return false;
2884-
}
2885-
28862866
@Override
28872867
protected boolean enterDefault(Node node) {
28882868
throw error(String.format("unexpected node in BindingPattern: %s", node));
@@ -3120,8 +3100,8 @@ private void forStatement(boolean yield, boolean await) {
31203100
throw error(AbstractParser.message(MSG_MANY_VARS_IN_FOR_IN_LOOP, isForOf || isForAwaitOf ? CONTEXT_OF : CONTEXT_IN), varDeclList.secondBinding.getToken());
31213101
}
31223102
init = varDeclList.firstBinding;
3123-
assert init instanceof IdentNode || isDestructuringLhs(init) : init;
3124-
if (varDeclList.declarationWithInitializerToken != 0 && (isStrictMode || type != IN || varType != VAR || isDestructuringLhs(init))) {
3103+
assert init instanceof IdentNode || isDestructuringOrExtractorLhs(init) : init;
3104+
if (varDeclList.declarationWithInitializerToken != 0 && (isStrictMode || type != IN || varType != VAR || isDestructuringOrExtractorLhs(init))) {
31253105
/*
31263106
* ES5 legacy: for (var i = AssignmentExpressionNoIn in Expression).
31273107
* Invalid in ES6, but allowed in non-strict mode if no ES6 features
@@ -3198,7 +3178,7 @@ private boolean checkValidLValue(Expression init, String contextString) {
31983178
return true;
31993179
} else if (init instanceof AccessNode || init instanceof IndexNode) {
32003180
return !((BaseNode) init).isOptional();
3201-
} else if (isDestructuringLhs(init)) {
3181+
} else if (isDestructuringOrExtractorLhs(init)) {
32023182
verifyDestructuringAssignmentPattern(init, contextString);
32033183
return true;
32043184
} else {
@@ -5129,7 +5109,7 @@ private Expression memberExpression(boolean yield, boolean await, CoverExpressio
51295109
* This helps report the first error location for cases like: <code>({x=i}[{y=j}])</code>.
51305110
*/
51315111
private void verifyPrimaryExpression(Expression lhs, CoverExpressionError coverExpression) {
5132-
if (coverExpression != CoverExpressionError.DENY && coverExpression.hasError() && isDestructuringLhs(lhs)) {
5112+
if (coverExpression != CoverExpressionError.DENY && coverExpression.hasError() && isDestructuringOrExtractorLhs(lhs)) {
51335113
/**
51345114
* These token types indicate that the preceding PrimaryExpression is part of an
51355115
* unfinished MemberExpression or other LeftHandSideExpression, which also means that it
@@ -5725,7 +5705,7 @@ private static void addDefaultParameter(long paramToken, int paramFinish, int pa
57255705
}
57265706

57275707
private void addDestructuringParameter(long paramToken, int paramFinish, int paramLine, Expression target, Expression initializer, ParserContextFunctionNode function, boolean isRest) {
5728-
assert isDestructuringLhs(target);
5708+
assert isDestructuringOrExtractorLhs(target);
57295709
// desugar to: target := (param === undefined) ? initializer : param;
57305710
// we use an special positional parameter node not subjected to TDZ rules;
57315711
// thereby, we forego the need for a synthetic param symbol to refer to the passed value.
@@ -6480,7 +6460,7 @@ private Expression assignmentExpression(boolean in, boolean yield, boolean await
64806460
assert !(exprLhs instanceof ExpressionList);
64816461

64826462
if (type.isAssignment()) {
6483-
if (canBeAssignmentPattern && !isDestructuringLhs(exprLhs)) {
6463+
if (canBeAssignmentPattern && !isDestructuringOrExtractorLhs(exprLhs)) {
64846464
// If LHS is not an AssignmentPattern, verify that it is a valid expression.
64856465
verifyExpression(coverExprLhs);
64866466
}
@@ -6632,7 +6612,7 @@ private void convertArrowFunctionParameterList(Expression paramList, ParserConte
66326612
return;
66336613
}
66346614
final int functionLine = function.getLineNumber();
6635-
if (paramListExpr instanceof IdentNode || paramListExpr.isTokenType(ASSIGN) || isDestructuringLhs(paramListExpr) || paramListExpr.isTokenType(SPREAD_ARGUMENT)) {
6615+
if (paramListExpr instanceof IdentNode || paramListExpr.isTokenType(ASSIGN) || isDestructuringOrExtractorLhs(paramListExpr) || paramListExpr.isTokenType(SPREAD_ARGUMENT)) {
66366616
convertArrowParameter(paramListExpr, 0, functionLine, function);
66376617
} else if (paramListExpr instanceof BinaryNode && Token.descType(paramListExpr.getToken()) == COMMARIGHT) {
66386618
List<Expression> params = new ArrayList<>();
@@ -6685,15 +6665,15 @@ private void convertArrowParameter(Expression param, int index, int paramLine, P
66856665

66866666
addDefaultParameter(paramToken, param.getFinish(), paramLine, ident, initializer, currentFunction);
66876667
return;
6688-
} else if (isDestructuringLhs(lhs)) {
6668+
} else if (isDestructuringOrExtractorLhs(lhs)) {
66896669
// binding pattern with initializer
66906670
verifyDestructuringParameterBindingPattern(lhs, paramToken, paramLine);
66916671

66926672
addDestructuringParameter(paramToken, param.getFinish(), paramLine, lhs, initializer, currentFunction, false);
66936673
} else {
66946674
throw error(AbstractParser.message(MSG_INVALID_ARROW_PARAMETER), paramToken);
66956675
}
6696-
} else if (isDestructuringLhs(param)) {
6676+
} else if (isDestructuringOrExtractorLhs(param)) {
66976677
// binding pattern
66986678
long paramToken = param.getToken();
66996679

@@ -6709,7 +6689,7 @@ private void convertArrowParameter(Expression param, int index, int paramLine, P
67096689
if (restParam instanceof IdentNode) {
67106690
IdentNode ident = ((IdentNode) restParam).setIsRestParameter();
67116691
convertArrowParameter(ident, index, paramLine, currentFunction);
6712-
} else if (isDestructuringLhs(restParam)) {
6692+
} else if (isDestructuringOrExtractorLhs(restParam)) {
67136693
verifyDestructuringParameterBindingPattern(restParam, restParam.getToken(), paramLine);
67146694
addDestructuringParameter(restParam.getToken(), restParam.getFinish(), paramLine, restParam, null, currentFunction, true);
67156695
} else {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3121,7 +3121,7 @@ private JavaScriptNode transformAssignmentExtractor(CallNode fakeCallNode, JavaS
31213121
receiver = transform(accessNode.getBase());
31223122
}
31233123

3124-
final var invokeCustomMatcherOrThrowNode = InvokeCustomMatcherOrThrowNode.create(context, function, assignedValue, receiver);
3124+
final var invokeCustomMatcherOrThrowNode = factory.createInvokeCustomMatcherOrThrow(context, function, assignedValue, receiver);
31253125

31263126
final var args = fakeCallNode.getArgs();
31273127
VarRef valueTempVar = environment.createTempVar();

‎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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
const DateExtractor = {
44
[Symbol.customMatcher](value) {

‎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.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
{
44
class C {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
class C {
44
#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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
class C {
44
#data1;

‎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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
const MapExtractor = {
44
[Symbol.customMatcher](map) {

‎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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
class C {
44
#f;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
load('../assert.js');
1+
load('../js/assert.js');
22

33
// potentially built-in as part of Pattern Matching
44
RegExp.prototype[Symbol.customMatcher] = function (value) {

‎graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/parser/ParserTest.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ class C {
217217
}
218218
extractor = {
219219
[Symbol.customMatcher](subject, _kind, receiver) {
220-
return receiver.#f(subject);
220+
return [receiver.#f(subject)];
221221
}
222222
};
223223
}
@@ -230,4 +230,28 @@ class C {
230230
ctx.eval(src);
231231
}
232232
}
233+
234+
@Test
235+
public void testInvalidExtractorStatement() {
236+
try (Context ctx = JSTest.newContextBuilder().build()) {
237+
org.graalvm.polyglot.Source src = org.graalvm.polyglot.Source.newBuilder("js", """
238+
class C {
239+
#data;
240+
constructor(data) {
241+
this.#data = data;
242+
}
243+
static [Symbol.customMatcher](subject) {
244+
return #data in subject && [subject.#data];
245+
}
246+
}
247+
248+
249+
const subject = new C("data");
250+
251+
let y;
252+
C(y) = subject;
253+
""", "test").buildLiteral();
254+
ctx.eval(src);
255+
}
256+
}
233257
}

0 commit comments

Comments
 (0)