Support destructuring assignment of context variables
Fixes T178003134. Previously we did not check whether values reassigned during a destructuring assignment were context variables. This would either miscompile, or as of my fix earlier in #2579, would fail validation. Specifically, this happened on AssignmentEpression with an object/array pattern lvalue, where the pattern contained an identifier that is a context variable. This is now fixed: we track whether the outermost assignment is a normal assignment or destructuring, and force destructuring to a temporary whenever the identifier is a context variable. We apply the same logic to variable declarations that are destructuring to a context variable.
Joe Savona committed
Feb 13, 2024 at 16:45 UTC
f7f05501e07bbd693637f50c635a15a492a7d0e8
11 files changed
+394
-49
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+68
-16
@@ -134,7 +134,8 @@ export function lower(
134
param.node.loc ?? GeneratedSource,
135
InstructionKind.Let,
136
param,
137
- place
137
+ place,
138
+ "Assignment"
139
);
140
} else if (param.isRestElement()) {
141
const place: Place = {
@@ -153,7 +154,8 @@ export function lower(
154
param.node.loc ?? GeneratedSource,
155
InstructionKind.Let,
156
param.get("argument"),
156
- place
157
+ place,
158
+ "Assignment"
159
);
160
} else {
161
builder.errors.push({
@@ -816,7 +818,10 @@ function lowerStatement(
818
stmt.node.loc ?? GeneratedSource,
819
kind,
820
id,
819
- value
821
+ value,
822
+ id.isObjectPattern() || id.isArrayPattern()
823
+ ? "Destructure"
824
+ : "Assignment"
825
);
826
} else if (id.isIdentifier()) {
827
const identifier = builder.resolveIdentifier(id);
@@ -978,7 +983,8 @@ function lowerStatement(
983
stmt.node.loc ?? GeneratedSource,
984
InstructionKind.Let,
985
id,
981
- fn
986
+ fn,
987
+ "Assignment"
988
);
989
990
return;
@@ -1043,7 +1049,8 @@ function lowerStatement(
1049
leftLoc,
1050
InstructionKind.Let,
1051
id,
1046
- nextIterableOf
1052
+ nextIterableOf,
1053
+ "Assignment"
1054
);
1055
test = lowerValueToTemporary(builder, assign);
1056
} else {
@@ -1128,7 +1135,8 @@ function lowerStatement(
1135
leftLoc,
1136
InstructionKind.Let,
1137
id,
1131
- nextIterableOf
1138
+ nextIterableOf,
1139
+ "Assignment"
1140
);
1141
test = lowerValueToTemporary(builder, assign);
1142
} else {
@@ -1229,7 +1237,8 @@ function lowerStatement(
1237
handlerBinding.path.node.loc ?? GeneratedSource,
1238
InstructionKind.Catch,
1239
handlerBinding.path,
1232
- { ...handlerBinding.place }
1240
+ { ...handlerBinding.place },
1241
+ "Assignment"
1242
);
1243
}
1244
lowerStatement(builder, handlerPath.get("body"));
@@ -1814,7 +1823,10 @@ function lowerExpression(
1823
left.node.loc ?? GeneratedSource,
1824
InstructionKind.Reassign,
1825
left,
1817
- lowerExpressionToTemporary(builder, expr.get("right"))
1826
+ lowerExpressionToTemporary(builder, expr.get("right")),
1827
+ left.isArrayPattern() || left.isObjectPattern()
1828
+ ? "Destructure"
1829
+ : "Assignment"
1830
);
1831
}
1832
@@ -3238,7 +3250,8 @@ function lowerAssignment(
3250
loc: SourceLocation,
3251
kind: InstructionKind,
3252
lvaluePath: NodePath<t.LVal>,
3241
- value: Place
3253
+ value: Place,
3254
+ assignmentKind: "Destructure" | "Assignment"
3255
): InstructionValue {
3256
const lvalueNode = lvaluePath.node;
3257
switch (lvalueNode.type) {
@@ -3382,7 +3395,12 @@ function lowerAssignment(
3395
}
3396
if (element.isRestElement()) {
3397
const argument = element.get("argument");
3385
- if (argument.isIdentifier() && !forceTemporaries) {
3398
+ if (
3399
+ argument.isIdentifier() &&
3400
+ !forceTemporaries &&
3401
+ (assignmentKind === "Assignment" ||
3402
+ getStoreKind(builder, argument) === "StoreLocal")
3403
+ ) {
3404
const identifier = lowerIdentifierForAssignment(
3405
builder,
3406
element.node.loc ?? GeneratedSource,
@@ -3407,7 +3425,12 @@ function lowerAssignment(
3425
});
3426
followups.push({ place: temp, path: argument as NodePath<t.LVal> }); // TODO remove type cast
3427
}
3410
- } else if (element.isIdentifier() && !forceTemporaries) {
3428
+ } else if (
3429
+ element.isIdentifier() &&
3430
+ !forceTemporaries &&
3431
+ (assignmentKind === "Assignment" ||
3432
+ getStoreKind(builder, element) === "StoreLocal")
3433
+ ) {
3434
const identifier = lowerIdentifierForAssignment(
3435
builder,
3436
element.node.loc ?? GeneratedSource,
@@ -3440,7 +3463,14 @@ function lowerAssignment(
3463
loc,
3464
});
3465
for (const { place, path } of followups) {
3443
- lowerAssignment(builder, path.node.loc ?? loc, kind, path, place);
3466
+ lowerAssignment(
3467
+ builder,
3468
+ path.node.loc ?? loc,
3469
+ kind,
3470
+ path,
3471
+ place,
3472
+ assignmentKind
3473
+ );
3474
}
3475
return { kind: "LoadLocal", place: temporary, loc: value.loc };
3476
}
@@ -3478,7 +3508,10 @@ function lowerAssignment(
3508
});
3509
continue;
3510
}
3481
- if (forceTemporaries) {
3511
+ if (
3512
+ forceTemporaries ||
3513
+ getStoreKind(builder, argument) === "StoreContext"
3514
+ ) {
3515
const temp = buildTemporaryPlace(
3516
builder,
3517
property.node.loc ?? GeneratedSource
@@ -3537,7 +3570,12 @@ function lowerAssignment(
3570
});
3571
continue;
3572
}
3540
- if (element.isIdentifier() && !forceTemporaries) {
3573
+ if (
3574
+ element.isIdentifier() &&
3575
+ !forceTemporaries &&
3576
+ (assignmentKind === "Assignment" ||
3577
+ getStoreKind(builder, element) === "StoreLocal")
3578
+ ) {
3579
const identifier = lowerIdentifierForAssignment(
3580
builder,
3581
element.node.loc ?? GeneratedSource,
@@ -3581,7 +3619,14 @@ function lowerAssignment(
3619
loc,
3620
});
3621
for (const { place, path } of followups) {
3584
- lowerAssignment(builder, path.node.loc ?? loc, kind, path, place);
3622
+ lowerAssignment(
3623
+ builder,
3624
+ path.node.loc ?? loc,
3625
+ kind,
3626
+ path,
3627
+ place,
3628
+ assignmentKind
3629
+ );
3630
}
3631
return { kind: "LoadLocal", place: temporary, loc: value.loc };
3632
}
@@ -3668,7 +3713,14 @@ function lowerAssignment(
3713
continuationBlock
3714
);
3715
3671
- return lowerAssignment(builder, loc, kind, lvalue.get("left"), temp);
3716
+ return lowerAssignment(
3717
+ builder,
3718
+ loc,
3719
+ kind,
3720
+ lvalue.get("left"),
3721
+ temp,
3722
+ assignmentKind
3723
+ );
3724
}
3725
default: {
3726
builder.errors.push({
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.expect.md
new
+66
@@ -0,0 +1,66 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(props) {
8
+ let x;
9
+ [x] = props.value;
10
+ const foo = () => {
11
+ x = identity(props.value[0]);
12
+ };
13
+ foo();
14
+ return { x };
15
+}
16
+
17
+export const FIXTURE_ENTRYPOINT = {
18
+ fn: Component,
19
+ params: [{ value: [42] }],
20
+};
21
+
22
+```
23
+
24
+## Code
25
+
26
+```javascript
27
+import { unstable_useMemoCache as useMemoCache } from "react";
28
+import { identity } from "shared-runtime";
29
+
30
+function Component(props) {
31
+ const $ = useMemoCache(4);
32
+ let x;
33
+ if ($[0] !== props.value) {
34
+ const [t31] = props.value;
35
+ x = t31;
36
+ const foo = () => {
37
+ x = identity(props.value[0]);
38
+ };
39
+
40
+ foo();
41
+ $[0] = props.value;
42
+ $[1] = x;
43
+ } else {
44
+ x = $[1];
45
+ }
46
+ const t0 = x;
47
+ let t1;
48
+ if ($[2] !== t0) {
49
+ t1 = { x: t0 };
50
+ $[2] = t0;
51
+ $[3] = t1;
52
+ } else {
53
+ t1 = $[3];
54
+ }
55
+ return t1;
56
+}
57
+
58
+export const FIXTURE_ENTRYPOINT = {
59
+ fn: Component,
60
+ params: [{ value: [42] }],
61
+};
62
+
63
+```
64
+
65
+### Eval output
66
+(kind: ok) {"x":42}
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-array-assignment-to-context-var.js
new
+16
@@ -0,0 +1,16 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(props) {
4
+ let x;
5
+ [x] = props.value;
6
+ const foo = () => {
7
+ x = identity(props.value[0]);
8
+ };
9
+ foo();
10
+ return { x };
11
+}
12
+
13
+export const FIXTURE_ENTRYPOINT = {
14
+ fn: Component,
15
+ params: [{ value: [42] }],
16
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.expect.md
new
+66
@@ -0,0 +1,66 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(props) {
8
+ let [x] = props.value;
9
+ const foo = () => {
10
+ x = identity(props.value[0]);
11
+ };
12
+ foo();
13
+ return { x };
14
+}
15
+
16
+export const FIXTURE_ENTRYPOINT = {
17
+ fn: Component,
18
+ params: [{ value: [42] }],
19
+};
20
+
21
+```
22
+
23
+## Code
24
+
25
+```javascript
26
+import { unstable_useMemoCache as useMemoCache } from "react";
27
+import { identity } from "shared-runtime";
28
+
29
+function Component(props) {
30
+ const $ = useMemoCache(5);
31
+ const [t0] = props.value;
32
+ let x;
33
+ if ($[0] !== t0 || $[1] !== props.value) {
34
+ x = t0;
35
+ const foo = () => {
36
+ x = identity(props.value[0]);
37
+ };
38
+
39
+ foo();
40
+ $[0] = t0;
41
+ $[1] = props.value;
42
+ $[2] = x;
43
+ } else {
44
+ x = $[2];
45
+ }
46
+ const t1 = x;
47
+ let t2;
48
+ if ($[3] !== t1) {
49
+ t2 = { x: t1 };
50
+ $[3] = t1;
51
+ $[4] = t2;
52
+ } else {
53
+ t2 = $[4];
54
+ }
55
+ return t2;
56
+}
57
+
58
+export const FIXTURE_ENTRYPOINT = {
59
+ fn: Component,
60
+ params: [{ value: [42] }],
61
+};
62
+
63
+```
64
+
65
+### Eval output
66
+(kind: ok) {"x":42}
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-array-declaration-to-context-var.js
new
+15
@@ -0,0 +1,15 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(props) {
4
+ let [x] = props.value;
5
+ const foo = () => {
6
+ x = identity(props.value[0]);
7
+ };
8
+ foo();
9
+ return { x };
10
+}
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: Component,
14
+ params: [{ value: [42] }],
15
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.expect.md
new
+66
@@ -0,0 +1,66 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(props) {
8
+ let x;
9
+ ({ x } = props);
10
+ const foo = () => {
11
+ x = identity(props.x);
12
+ };
13
+ foo();
14
+ return { x };
15
+}
16
+
17
+export const FIXTURE_ENTRYPOINT = {
18
+ fn: Component,
19
+ params: [{ x: 42 }],
20
+};
21
+
22
+```
23
+
24
+## Code
25
+
26
+```javascript
27
+import { unstable_useMemoCache as useMemoCache } from "react";
28
+import { identity } from "shared-runtime";
29
+
30
+function Component(props) {
31
+ const $ = useMemoCache(4);
32
+ let x;
33
+ if ($[0] !== props) {
34
+ const { x: t27 } = props;
35
+ x = t27;
36
+ const foo = () => {
37
+ x = identity(props.x);
38
+ };
39
+
40
+ foo();
41
+ $[0] = props;
42
+ $[1] = x;
43
+ } else {
44
+ x = $[1];
45
+ }
46
+ const t0 = x;
47
+ let t1;
48
+ if ($[2] !== t0) {
49
+ t1 = { x: t0 };
50
+ $[2] = t0;
51
+ $[3] = t1;
52
+ } else {
53
+ t1 = $[3];
54
+ }
55
+ return t1;
56
+}
57
+
58
+export const FIXTURE_ENTRYPOINT = {
59
+ fn: Component,
60
+ params: [{ x: 42 }],
61
+};
62
+
63
+```
64
+
65
+### Eval output
66
+(kind: ok) {"x":42}
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-object-assignment-to-context-var.js
new
+16
@@ -0,0 +1,16 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(props) {
4
+ let x;
5
+ ({ x } = props);
6
+ const foo = () => {
7
+ x = identity(props.x);
8
+ };
9
+ foo();
10
+ return { x };
11
+}
12
+
13
+export const FIXTURE_ENTRYPOINT = {
14
+ fn: Component,
15
+ params: [{ x: 42 }],
16
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.expect.md
new
+66
@@ -0,0 +1,66 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+import { identity } from "shared-runtime";
6
+
7
+function Component(props) {
8
+ let { x } = props;
9
+ const foo = () => {
10
+ x = identity(props.x);
11
+ };
12
+ foo();
13
+ return { x };
14
+}
15
+
16
+export const FIXTURE_ENTRYPOINT = {
17
+ fn: Component,
18
+ params: [{ x: 42 }],
19
+};
20
+
21
+```
22
+
23
+## Code
24
+
25
+```javascript
26
+import { unstable_useMemoCache as useMemoCache } from "react";
27
+import { identity } from "shared-runtime";
28
+
29
+function Component(props) {
30
+ const $ = useMemoCache(5);
31
+ const { x: t0 } = props;
32
+ let x;
33
+ if ($[0] !== t0 || $[1] !== props.x) {
34
+ x = t0;
35
+ const foo = () => {
36
+ x = identity(props.x);
37
+ };
38
+
39
+ foo();
40
+ $[0] = t0;
41
+ $[1] = props.x;
42
+ $[2] = x;
43
+ } else {
44
+ x = $[2];
45
+ }
46
+ const t1 = x;
47
+ let t2;
48
+ if ($[3] !== t1) {
49
+ t2 = { x: t1 };
50
+ $[3] = t1;
51
+ $[4] = t2;
52
+ } else {
53
+ t2 = $[4];
54
+ }
55
+ return t2;
56
+}
57
+
58
+export const FIXTURE_ENTRYPOINT = {
59
+ fn: Component,
60
+ params: [{ x: 42 }],
61
+};
62
+
63
+```
64
+
65
+### Eval output
66
+(kind: ok) {"x":42}
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/destructure-object-declaration-to-context-var.js
new
+15
@@ -0,0 +1,15 @@
1
+import { identity } from "shared-runtime";
2
+
3
+function Component(props) {
4
+ let { x } = props;
5
+ const foo = () => {
6
+ x = identity(props.x);
7
+ };
8
+ foo();
9
+ return { x };
10
+}
11
+
12
+export const FIXTURE_ENTRYPOINT = {
13
+ fn: Component,
14
+ params: [{ x: 42 }],
15
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.destructure-assignment-to-context-var.expect.md
deleted
-24
@@ -1,24 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function useFoo(props) {
6
- let x;
7
- [x] = props;
8
- const foo = () => {
9
- x = getX(props);
10
- };
11
- foo();
12
- return { x };
13
-}
14
-
15
-```
16
-
17
-
18
-## Error
19
-
20
-```
21
-[ReactForget] Invariant: Expected all references to a variable to be consistently local or context references. Identifier <unknown> x$1 is referenced as a local variable, but was previously referenced as a context variable (3:3)
22
-```
23
-
24
-
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo.destructure-assignment-to-context-var.js
deleted
-9
@@ -1,9 +0,0 @@
1
-function useFoo(props) {
2
- let x;
3
- [x] = props;
4
- const foo = () => {
5
- x = getX(props);
6
- };
7
- foo();
8
- return { x };
9
-}