ValidateFrozenLambdas: visit terminals
The original version of the code wasn't checking return values. I missed this since my examples were passing functions _into_ the return value, as opposed to return the functions directly. This revealed some existing test fixtures that were technically invalid, but easy to fix by changing the return value.
Joe Savona committed
Jun 16, 2023 at 14:42 UTC
278c7f56babc574a66392eacace297715e4d5446
15 files changed
+134
-181
compiler/forget/packages/babel-plugin-react-forget/src/HIR/ValidateFrozenLambdas.ts
+55
-32
@@ -15,11 +15,12 @@ import {
15
FunctionExpression,
16
HIRFunction,
17
IdentifierId,
18
+ Place,
19
isMutableEffect,
20
isRefValueType,
21
isUseRefType,
22
} from "./HIR";
22
-import { eachInstructionValueOperand } from "./visitors";
23
+import { eachInstructionValueOperand, eachTerminalOperand } from "./visitors";
24
25
/**
26
* Various APIs in React take ownership of the values passed to them, such that it is invalid
@@ -47,56 +48,78 @@ import { eachInstructionValueOperand } from "./visitors";
48
* the developer fix the mistake earlier.
49
*/
50
export function validateFrozenLambdas(fn: HIRFunction): void {
50
- const lambdas = new Map<IdentifierId, FunctionExpression>();
51
- const temporaries = new Map<IdentifierId, IdentifierId>();
51
+ const state = new State();
52
53
const errors = new CompilerError();
54
for (const [, block] of fn.body.blocks) {
55
for (const instr of block.instructions) {
56
if (instr.value.kind === "FunctionExpression") {
57
- lambdas.set(instr.lvalue.identifier.id, instr.value);
57
+ state.lambdas.set(instr.lvalue.identifier.id, instr.value);
58
} else if (instr.value.kind === "LoadLocal") {
59
const resolvedId =
60
- temporaries.get(instr.value.place.identifier.id) ??
60
+ state.temporaries.get(instr.value.place.identifier.id) ??
61
instr.value.place.identifier.id;
62
- temporaries.set(instr.lvalue.identifier.id, resolvedId);
62
+ state.temporaries.set(instr.lvalue.identifier.id, resolvedId);
63
} else if (instr.value.kind === "StoreLocal") {
64
const resolvedId =
65
- temporaries.get(instr.value.value.identifier.id) ??
65
+ state.temporaries.get(instr.value.value.identifier.id) ??
66
instr.value.value.identifier.id;
67
- temporaries.set(instr.value.lvalue.place.identifier.id, resolvedId);
67
+ state.temporaries.set(
68
+ instr.value.lvalue.place.identifier.id,
69
+ resolvedId
70
+ );
71
} else {
72
for (const operand of eachInstructionValueOperand(instr.value)) {
70
- if (operand.effect === Effect.Freeze) {
71
- const operandId =
72
- temporaries.get(operand.identifier.id) ?? operand.identifier.id;
73
- const lambda = lambdas.get(operandId);
74
- if (
75
- lambda !== undefined &&
76
- lambda.dependencies.some(
77
- (place) =>
78
- isMutableEffect(place.effect, place.loc) &&
79
- !isRefValueType(place.identifier) &&
80
- !isUseRefType(place.identifier)
81
- )
82
- ) {
83
- errors.pushErrorDetail(
84
- new CompilerErrorDetail({
85
- codeframe: null,
86
- description: null,
87
- loc: typeof operand.loc !== "symbol" ? operand.loc : null,
88
- reason:
89
- "Cannot use a mutable function where an immutable value is expected",
90
- severity: ErrorSeverity.InvalidInput,
91
- })
92
- );
93
- }
73
+ const operandError = validateOperand(operand, state);
74
+ if (operandError !== null) {
75
+ errors.pushErrorDetail(operandError);
76
}
77
}
78
}
79
}
80
+ for (const operand of eachTerminalOperand(block.terminal)) {
81
+ const operandError = validateOperand(operand, state);
82
+ if (operandError !== null) {
83
+ errors.pushErrorDetail(operandError);
84
+ }
85
+ }
86
}
87
if (errors.hasErrors()) {
88
throw errors;
89
}
90
}
91
+
92
+class State {
93
+ lambdas: Map<IdentifierId, FunctionExpression> = new Map();
94
+ temporaries: Map<IdentifierId, IdentifierId> = new Map();
95
+}
96
+
97
+function validateOperand(
98
+ operand: Place,
99
+ state: State
100
+): CompilerErrorDetail | null {
101
+ if (operand.effect === Effect.Freeze) {
102
+ const operandId =
103
+ state.temporaries.get(operand.identifier.id) ?? operand.identifier.id;
104
+ const lambda = state.lambdas.get(operandId);
105
+ if (
106
+ lambda !== undefined &&
107
+ lambda.dependencies.some(
108
+ (place) =>
109
+ isMutableEffect(place.effect, place.loc) &&
110
+ !isRefValueType(place.identifier) &&
111
+ !isUseRefType(place.identifier)
112
+ )
113
+ ) {
114
+ return new CompilerErrorDetail({
115
+ codeframe: null,
116
+ description: null,
117
+ loc: typeof operand.loc !== "symbol" ? operand.loc : null,
118
+ reason:
119
+ "Cannot use a mutable function where an immutable value is expected",
120
+ severity: ErrorSeverity.InvalidInput,
121
+ });
122
+ }
123
+ }
124
+ return null;
125
+}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate-2.expect.md
+7
-7
@@ -10,7 +10,7 @@ function component(a, b) {
10
y.b;
11
};
12
x();
13
- return x;
13
+ return z;
14
}
15
16
```
@@ -33,21 +33,21 @@ function component(a, b) {
33
const y = t0;
34
const c_2 = $[2] !== a;
35
const c_3 = $[3] !== y.b;
36
- let x;
36
+ let z;
37
if (c_2 || c_3) {
38
- const z = { a };
39
- x = function () {
38
+ z = { a };
39
+ const x = function () {
40
z.a = 2;
41
y.b;
42
};
43
x();
44
$[2] = a;
45
$[3] = y.b;
46
- $[4] = x;
46
+ $[4] = z;
47
} else {
48
- x = $[4];
48
+ z = $[4];
49
}
50
- return x;
50
+ return z;
51
}
52
53
```
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate-2.js
+1
-1
@@ -6,5 +6,5 @@ function component(a, b) {
6
y.b;
7
};
8
x();
9
- return x;
9
+ return z;
10
}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate-3.expect.md
+7
-32
@@ -9,7 +9,7 @@ function component(a, b) {
9
z.a = 2;
10
y.b;
11
};
12
- return x;
12
+ return z;
13
}
14
15
```
@@ -19,43 +19,18 @@ function component(a, b) {
19
```javascript
20
import { unstable_useMemoCache as useMemoCache } from "react";
21
function component(a, b) {
22
- const $ = useMemoCache(7);
23
- const c_0 = $[0] !== b;
22
+ const $ = useMemoCache(2);
23
+ const c_0 = $[0] !== a;
24
let t0;
25
if (c_0) {
26
- t0 = { b };
27
- $[0] = b;
26
+ t0 = { a };
27
+ $[0] = a;
28
$[1] = t0;
29
} else {
30
t0 = $[1];
31
}
32
- const y = t0;
33
- const c_2 = $[2] !== a;
34
- let t1;
35
- if (c_2) {
36
- t1 = { a };
37
- $[2] = a;
38
- $[3] = t1;
39
- } else {
40
- t1 = $[3];
41
- }
42
- const z = t1;
43
- const c_4 = $[4] !== z.a;
44
- const c_5 = $[5] !== y.b;
45
- let t2;
46
- if (c_4 || c_5) {
47
- t2 = function () {
48
- z.a = 2;
49
- y.b;
50
- };
51
- $[4] = z.a;
52
- $[5] = y.b;
53
- $[6] = t2;
54
- } else {
55
- t2 = $[6];
56
- }
57
- const x = t2;
58
- return x;
32
+ const z = t0;
33
+ return z;
34
}
35
36
```
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate-3.js
+1
-1
@@ -5,5 +5,5 @@ function component(a, b) {
5
z.a = 2;
6
y.b;
7
};
8
- return x;
8
+ return z;
9
}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate-nested.expect.md
+7
-7
@@ -8,7 +8,7 @@ function component(a) {
8
y.b.a = 2;
9
};
10
x();
11
- return x;
11
+ return y;
12
}
13
14
```
@@ -20,19 +20,19 @@ import { unstable_useMemoCache as useMemoCache } from "react";
20
function component(a) {
21
const $ = useMemoCache(2);
22
const c_0 = $[0] !== a;
23
- let x;
23
+ let y;
24
if (c_0) {
25
- const y = { b: { a } };
26
- x = function () {
25
+ y = { b: { a } };
26
+ const x = function () {
27
y.b.a = 2;
28
};
29
x();
30
$[0] = a;
31
- $[1] = x;
31
+ $[1] = y;
32
} else {
33
- x = $[1];
33
+ y = $[1];
34
}
35
- return x;
35
+ return y;
36
}
37
38
```
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate-nested.js
+1
-1
@@ -4,5 +4,5 @@ function component(a) {
4
y.b.a = 2;
5
};
6
x();
7
- return x;
7
+ return y;
8
}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate.expect.md
+9
-18
@@ -10,7 +10,7 @@ function component(a, b) {
10
y.b;
11
};
12
x();
13
- return x;
13
+ return z;
14
}
15
16
```
@@ -20,34 +20,25 @@ function component(a, b) {
20
```javascript
21
import { unstable_useMemoCache as useMemoCache } from "react";
22
function component(a, b) {
23
- const $ = useMemoCache(5);
23
+ const $ = useMemoCache(3);
24
const c_0 = $[0] !== a;
25
const c_1 = $[1] !== b;
26
- let x;
26
+ let z;
27
if (c_0 || c_1) {
28
- const z = { a };
29
- const c_3 = $[3] !== b;
30
- let t0;
31
- if (c_3) {
32
- t0 = { b };
33
- $[3] = b;
34
- $[4] = t0;
35
- } else {
36
- t0 = $[4];
37
- }
38
- const y = t0;
39
- x = function () {
28
+ z = { a };
29
+ const y = { b };
30
+ const x = function () {
31
z.a = 2;
32
y.b;
33
};
34
x();
35
$[0] = a;
36
$[1] = b;
46
- $[2] = x;
37
+ $[2] = z;
38
} else {
48
- x = $[2];
39
+ z = $[2];
40
}
50
- return x;
41
+ return z;
42
}
43
44
```
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-func-mutate.js
+1
-1
@@ -6,5 +6,5 @@ function component(a, b) {
6
y.b;
7
};
8
x();
9
- return x;
9
+ return z;
10
}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-function-conditional-capture-mutate.expect.md
deleted
-55
@@ -1,55 +0,0 @@
1
-
2
-## Input
3
-
4
-```javascript
5
-function component(a, b) {
6
- let z = { a };
7
- let y = b;
8
- let x = function () {
9
- if (y) {
10
- mutate(z);
11
- }
12
- };
13
- return x;
14
-}
15
-
16
-```
17
-
18
-## Code
19
-
20
-```javascript
21
-import { unstable_useMemoCache as useMemoCache } from "react";
22
-function component(a, b) {
23
- const $ = useMemoCache(5);
24
- const c_0 = $[0] !== a;
25
- let t0;
26
- if (c_0) {
27
- t0 = { a };
28
- $[0] = a;
29
- $[1] = t0;
30
- } else {
31
- t0 = $[1];
32
- }
33
- const z = t0;
34
- const y = b;
35
- const c_2 = $[2] !== y;
36
- const c_3 = $[3] !== z;
37
- let t1;
38
- if (c_2 || c_3) {
39
- t1 = function () {
40
- if (y) {
41
- mutate(z);
42
- }
43
- };
44
- $[2] = y;
45
- $[3] = z;
46
- $[4] = t1;
47
- } else {
48
- t1 = $[4];
49
- }
50
- const x = t1;
51
- return x;
52
-}
53
-
54
-```
55
-
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-function-conditional-capture-mutate.js
deleted
-10
@@ -1,10 +0,0 @@
1
-function component(a, b) {
2
- let z = { a };
3
- let y = b;
4
- let x = function () {
5
- if (y) {
6
- mutate(z);
7
- }
8
- };
9
- return x;
10
-}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-nested-member-call.expect.md
+3
-15
@@ -7,7 +7,7 @@ function component(a) {
7
let x = function () {
8
z.a.a();
9
};
10
- return x;
10
+ return z;
11
}
12
13
```
@@ -17,7 +17,7 @@ function component(a) {
17
```javascript
18
import { unstable_useMemoCache as useMemoCache } from "react";
19
function component(a) {
20
- const $ = useMemoCache(6);
20
+ const $ = useMemoCache(4);
21
const c_0 = $[0] !== a;
22
let t0;
23
if (c_0) {
@@ -37,19 +37,7 @@ function component(a) {
37
t1 = $[3];
38
}
39
const z = t1;
40
- const c_4 = $[4] !== z.a;
41
- let t2;
42
- if (c_4) {
43
- t2 = function () {
44
- z.a.a();
45
- };
46
- $[4] = z.a;
47
- $[5] = t2;
48
- } else {
49
- t2 = $[5];
50
- }
51
- const x = t2;
52
- return x;
40
+ return z;
41
}
42
43
```
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/capturing-nested-member-call.js
+1
-1
@@ -3,5 +3,5 @@ function component(a) {
3
let x = function () {
4
z.a.a();
5
};
6
- return x;
6
+ return z;
7
}
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-capturing-function-conditional-capture-mutate.expect.md
new
+28
@@ -0,0 +1,28 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function component(a, b) {
6
+ let z = { a };
7
+ let y = b;
8
+ let x = function () {
9
+ if (y) {
10
+ // we don't know for sure this mutates, so we should assume
11
+ // that there is no mutation so long as `x` isn't called
12
+ // during render
13
+ maybeMutate(z);
14
+ }
15
+ };
16
+ return x;
17
+}
18
+
19
+```
20
+
21
+
22
+## Error
23
+
24
+```
25
+[ReactForget] InvalidInput: Cannot use a mutable function where an immutable value is expected (12:12)
26
+```
27
+
28
+
\ No newline at end of file
compiler/forget/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-capturing-function-conditional-capture-mutate.js
new
+13
@@ -0,0 +1,13 @@
1
+function component(a, b) {
2
+ let z = { a };
3
+ let y = b;
4
+ let x = function () {
5
+ if (y) {
6
+ // we don't know for sure this mutates, so we should assume
7
+ // that there is no mutation so long as `x` isn't called
8
+ // during render
9
+ maybeMutate(z);
10
+ }
11
+ };
12
+ return x;
13
+}