Skip to content

Commit 0e24698

Browse files
avpfacebook-github-bot
avp
authored and
facebook-github-bot
committed
Remove collapseNestedAP.
Summary: The code: ``` var [a,b,...[c,d]] = iter; ``` must run `iter` to completion because it contains the rest element `...[c,d]`. `collapseNestedAP` is an optimization that was causing a correctness issue because `[a,b,...[c,d]]` is not equivalent to `[a,b,c,d]`, which only runs the iterator 4 times and then closes it. Remove `collapseNestedAP`. Reviewed By: tmikov Differential Revision: D18439034 fbshipit-source-id: fe048c3e995112e7f3db6c6fc64c34363e78ed39
1 parent 4ce69ab commit 0e24698

6 files changed

Lines changed: 597 additions & 390 deletions

File tree

‎lib/AST/SemanticValidator.cpp‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,6 @@ void SemanticValidator::visit(UnaryExpressionNode *unaryExpr) {
454454
}
455455

456456
void SemanticValidator::visit(ArrayPatternNode *AP) {
457-
collapseNestedAP(AP->_elements);
458457
visitESTreeChildren(*this, AP);
459458
}
460459

@@ -636,8 +635,6 @@ void SemanticValidator::visitFunction(
636635
}
637636
}
638637

639-
collapseNestedAP(params);
640-
641638
visitParamsAndBody(node);
642639
}
643640

@@ -863,26 +860,6 @@ LabelDecorationBase *SemanticValidator::getLabelDecorationBase(
863860
return nullptr;
864861
}
865862

866-
/// Collapse array pattern rest elements into their parent:
867-
/// [a, ...[b, c]] => [a, b, c].
868-
void SemanticValidator::collapseNestedAP(NodeList &elements) {
869-
if (elements.empty())
870-
return;
871-
auto *restElement = dyn_cast<RestElementNode>(&elements.back());
872-
if (!restElement)
873-
return;
874-
auto *nestedAP = dyn_cast<ArrayPatternNode>(restElement->_argument);
875-
if (!nestedAP)
876-
return;
877-
878-
elements.pop_back();
879-
while (!nestedAP->_elements.empty()) {
880-
auto *elem = &nestedAP->_elements.front();
881-
nestedAP->_elements.pop_front();
882-
elements.push_back(*elem);
883-
}
884-
}
885-
886863
//===----------------------------------------------------------------------===//
887864
// FunctionContext
888865

‎lib/AST/SemanticValidator.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,10 +217,6 @@ class SemanticValidator {
217217
/// Get the LabelDecorationBase depending on the node type.
218218
static LabelDecorationBase *getLabelDecorationBase(StatementNode *node);
219219

220-
/// Collapse array pattern rest elements into their parent:
221-
/// [a, ...[b, c]] => [a, b, c].
222-
static void collapseNestedAP(NodeList &elements);
223-
224220
/// Visit the parameters and body of \p node, setting isFormalParams_
225221
/// correctly.
226222
void visitParamsAndBody(FunctionLikeNode *node);

‎test/AST/es6/simplify-rest-array.js‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,24 @@
3333
//CHECK-NEXT: "typeAnnotation": null
3434
//CHECK-NEXT: },
3535
//CHECK-NEXT: {
36-
//CHECK-NEXT: "type": "Identifier",
37-
//CHECK-NEXT: "name": "c",
38-
//CHECK-NEXT: "typeAnnotation": null
39-
//CHECK-NEXT: },
40-
//CHECK-NEXT: {
4136
//CHECK-NEXT: "type": "RestElement",
4237
//CHECK-NEXT: "argument": {
43-
//CHECK-NEXT: "type": "Identifier",
44-
//CHECK-NEXT: "name": "d",
45-
//CHECK-NEXT: "typeAnnotation": null
38+
//CHECK-NEXT: "type": "ArrayPattern",
39+
//CHECK-NEXT: "elements": [
40+
//CHECK-NEXT: {
41+
//CHECK-NEXT: "type": "Identifier",
42+
//CHECK-NEXT: "name": "c",
43+
//CHECK-NEXT: "typeAnnotation": null
44+
//CHECK-NEXT: },
45+
//CHECK-NEXT: {
46+
//CHECK-NEXT: "type": "RestElement",
47+
//CHECK-NEXT: "argument": {
48+
//CHECK-NEXT: "type": "Identifier",
49+
//CHECK-NEXT: "name": "d",
50+
//CHECK-NEXT: "typeAnnotation": null
51+
//CHECK-NEXT: }
52+
//CHECK-NEXT: }
53+
//CHECK-NEXT: ]
4654
//CHECK-NEXT: }
4755
//CHECK-NEXT: }
4856
//CHECK-NEXT: ]

‎test/AST/es6/simplify-rest-param.js‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,22 @@ function foo(a, ...[b, c]) {}
2828
//CHECK-NEXT: "typeAnnotation": null
2929
//CHECK-NEXT: },
3030
//CHECK-NEXT: {
31-
//CHECK-NEXT: "type": "Identifier",
32-
//CHECK-NEXT: "name": "b",
33-
//CHECK-NEXT: "typeAnnotation": null
34-
//CHECK-NEXT: },
35-
//CHECK-NEXT: {
36-
//CHECK-NEXT: "type": "Identifier",
37-
//CHECK-NEXT: "name": "c",
38-
//CHECK-NEXT: "typeAnnotation": null
31+
//CHECK-NEXT: "type": "RestElement",
32+
//CHECK-NEXT: "argument": {
33+
//CHECK-NEXT: "type": "ArrayPattern",
34+
//CHECK-NEXT: "elements": [
35+
//CHECK-NEXT: {
36+
//CHECK-NEXT: "type": "Identifier",
37+
//CHECK-NEXT: "name": "b",
38+
//CHECK-NEXT: "typeAnnotation": null
39+
//CHECK-NEXT: },
40+
//CHECK-NEXT: {
41+
//CHECK-NEXT: "type": "Identifier",
42+
//CHECK-NEXT: "name": "c",
43+
//CHECK-NEXT: "typeAnnotation": null
44+
//CHECK-NEXT: }
45+
//CHECK-NEXT: ]
46+
//CHECK-NEXT: }
3947
//CHECK-NEXT: }
4048
//CHECK-NEXT: ],
4149
//CHECK-NEXT: "body": {

0 commit comments

Comments
 (0)