Mutation within a reactively controlled block propagates reactivity
In InferReactivePlaces, we already account for reactively controlled values: where a value is never assigned a non-reactive value, but _which_ value is assigned is based on a reactive condition (the test conditions of an if, switch, loop, etc). This PR extends that reactively-controlled inference to mutation that is conditioned upon a reactive value. From the test case: ```javascript let x = []; if (props.cond) { // This mutation has no reactive inputs. // *But* the mutation conditionally occurs based on props.cond which is reactive x.push(1); } let y = false; if (x[0]) { // therefore the value observed here is reactive y = true; } // so the value of y here is reactive via the reactive control dependency x[0] return [y]; ```
Joe Savona committed
Jan 19, 2024 at 10:03 UTC
c8323f3b42887e292fec37a7dbb476f0fabc1d09
5 files changed
+218
-43
compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts
+44
-43
@@ -97,10 +97,48 @@ export function inferReactivePlaces(fn: HIRFunction): void {
97
includeThrowsAsExitNode: false,
98
});
99
const postDominatorFrontierCache = new Map<BlockId, Set<BlockId>>();
100
+
101
+ function isReactiveControlledBlock(id: BlockId): boolean {
102
+ let controlBlocks = postDominatorFrontierCache.get(id);
103
+ if (controlBlocks === undefined) {
104
+ controlBlocks = postDominatorFrontier(fn, postDominators, id);
105
+ postDominatorFrontierCache.set(id, controlBlocks);
106
+ }
107
+ for (const blockId of controlBlocks) {
108
+ const controlBlock = fn.body.blocks.get(blockId)!;
109
+ switch (controlBlock.terminal.kind) {
110
+ case "if":
111
+ case "branch": {
112
+ if (reactiveIdentifiers.isReactive(controlBlock.terminal.test)) {
113
+ return true;
114
+ }
115
+ break;
116
+ }
117
+ case "switch": {
118
+ if (reactiveIdentifiers.isReactive(controlBlock.terminal.test)) {
119
+ return true;
120
+ }
121
+ for (const case_ of controlBlock.terminal.cases) {
122
+ if (
123
+ case_.test !== null &&
124
+ reactiveIdentifiers.isReactive(case_.test)
125
+ ) {
126
+ return true;
127
+ }
128
+ }
129
+ break;
130
+ }
131
+ }
132
+ }
133
+ return false;
134
+ }
135
+
136
const hasLoop = hasBackEdge(fn);
137
do {
138
const identifierMapping = new Map<Identifier, Identifier>();
139
for (const [, block] of fn.body.blocks) {
140
+ let hasReactiveControl = isReactiveControlledBlock(block.id);
141
+
142
for (const phi of block.phis) {
143
if (reactiveIdentifiers.isReactiveIdentifier(phi.id)) {
144
// Already marked reactive on a previous pass
@@ -116,48 +154,10 @@ export function inferReactivePlaces(fn: HIRFunction): void {
154
if (isPhiReactive) {
155
reactiveIdentifiers.markReactiveIdentifier(phi.id);
156
} else {
119
- // check to see if it has a reactive control dependency
120
- for (const [pred, _operand] of phi.operands) {
121
- let controlBlocks = postDominatorFrontierCache.get(pred);
122
- if (controlBlocks === undefined) {
123
- controlBlocks = postDominatorFrontier(fn, postDominators, pred);
124
- postDominatorFrontierCache.set(pred, controlBlocks);
125
- }
126
- control: for (const blockId of controlBlocks) {
127
- const controlBlock = fn.body.blocks.get(blockId)!;
128
- switch (controlBlock.terminal.kind) {
129
- case "if":
130
- case "branch": {
131
- if (
132
- reactiveIdentifiers.isReactive(controlBlock.terminal.test)
133
- ) {
134
- // control dependency is reactive
135
- reactiveIdentifiers.markReactiveIdentifier(phi.id);
136
- break control;
137
- }
138
- break;
139
- }
140
- case "switch": {
141
- if (
142
- reactiveIdentifiers.isReactive(controlBlock.terminal.test)
143
- ) {
144
- // control dependency is reactive
145
- reactiveIdentifiers.markReactiveIdentifier(phi.id);
146
- break control;
147
- }
148
- for (const case_ of controlBlock.terminal.cases) {
149
- if (
150
- case_.test !== null &&
151
- reactiveIdentifiers.isReactive(case_.test)
152
- ) {
153
- // control dependency is reactive
154
- reactiveIdentifiers.markReactiveIdentifier(phi.id);
155
- break control;
156
- }
157
- }
158
- break;
159
- }
160
- }
157
+ for (const [pred] of phi.operands) {
158
+ if (isReactiveControlledBlock(pred)) {
159
+ reactiveIdentifiers.markReactiveIdentifier(phi.id);
160
+ break;
161
}
162
}
163
}
@@ -197,7 +197,8 @@ export function inferReactivePlaces(fn: HIRFunction): void {
197
}
198
reactiveIdentifiers.markReactive(lvalue);
199
}
200
-
200
+ }
201
+ if (hasReactiveInput || hasReactiveControl) {
202
for (const operand of eachInstructionValueOperand(value)) {
203
switch (operand.effect) {
204
case Effect.Capture:
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.expect.md
new
+63
@@ -0,0 +1,63 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ // x is mutated conditionally based on a reactive value,
7
+ // so it needs to be considered reactive
8
+ let x = [];
9
+ if (props.cond) {
10
+ x.push(1);
11
+ }
12
+ // Since x is reactive, y is now reactively controlled too:
13
+ let y = false;
14
+ if (x[0]) {
15
+ y = true;
16
+ }
17
+ // Thus this value should be reactive on `y`:
18
+ return [y];
19
+}
20
+
21
+export const FIXTURE_ENTRYPOINT = {
22
+ fn: Component,
23
+ params: [{ cond: true }],
24
+};
25
+
26
+```
27
+
28
+## Code
29
+
30
+```javascript
31
+import { unstable_useMemoCache as useMemoCache } from "react";
32
+function Component(props) {
33
+ const $ = useMemoCache(2);
34
+
35
+ const x = [];
36
+ if (props.cond) {
37
+ x.push(1);
38
+ }
39
+
40
+ let y = false;
41
+ if (x[0]) {
42
+ y = true;
43
+ }
44
+ let t0;
45
+ if ($[0] !== y) {
46
+ t0 = [y];
47
+ $[0] = y;
48
+ $[1] = t0;
49
+ } else {
50
+ t0 = $[1];
51
+ }
52
+ return t0;
53
+}
54
+
55
+export const FIXTURE_ENTRYPOINT = {
56
+ fn: Component,
57
+ params: [{ cond: true }],
58
+};
59
+
60
+```
61
+
62
+### Eval output
63
+(kind: ok) [true]
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-if.js
new
+20
@@ -0,0 +1,20 @@
1
+function Component(props) {
2
+ // x is mutated conditionally based on a reactive value,
3
+ // so it needs to be considered reactive
4
+ let x = [];
5
+ if (props.cond) {
6
+ x.push(1);
7
+ }
8
+ // Since x is reactive, y is now reactively controlled too:
9
+ let y = false;
10
+ if (x[0]) {
11
+ y = true;
12
+ }
13
+ // Thus this value should be reactive on `y`:
14
+ return [y];
15
+}
16
+
17
+export const FIXTURE_ENTRYPOINT = {
18
+ fn: Component,
19
+ params: [{ cond: true }],
20
+};
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.expect.md
new
+68
@@ -0,0 +1,68 @@
1
+
2
+## Input
3
+
4
+```javascript
5
+function Component(props) {
6
+ // x is mutated conditionally based on a reactive value,
7
+ // so it needs to be considered reactive
8
+ let x = [];
9
+ if (props.cond) {
10
+ x.push(1);
11
+ }
12
+ // Since x is reactive, y is now reactively controlled too:
13
+ let y = false;
14
+ switch (x[0]) {
15
+ case 1: {
16
+ y = true;
17
+ break;
18
+ }
19
+ }
20
+ // Thus this value should be reactive on `y`:
21
+ return [y];
22
+}
23
+
24
+export const FIXTURE_ENTRYPOINT = {
25
+ fn: Component,
26
+ params: [{ cond: true }],
27
+};
28
+
29
+```
30
+
31
+## Code
32
+
33
+```javascript
34
+import { unstable_useMemoCache as useMemoCache } from "react";
35
+function Component(props) {
36
+ const $ = useMemoCache(2);
37
+
38
+ const x = [];
39
+ if (props.cond) {
40
+ x.push(1);
41
+ }
42
+
43
+ let y = false;
44
+ switch (x[0]) {
45
+ case 1: {
46
+ y = true;
47
+ }
48
+ }
49
+ let t0;
50
+ if ($[0] !== y) {
51
+ t0 = [y];
52
+ $[0] = y;
53
+ $[1] = t0;
54
+ } else {
55
+ t0 = $[1];
56
+ }
57
+ return t0;
58
+}
59
+
60
+export const FIXTURE_ENTRYPOINT = {
61
+ fn: Component,
62
+ params: [{ cond: true }],
63
+};
64
+
65
+```
66
+
67
+### Eval output
68
+(kind: ok) [true]
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactive-control-dependency-via-mutation-switch.js
new
+23
@@ -0,0 +1,23 @@
1
+function Component(props) {
2
+ // x is mutated conditionally based on a reactive value,
3
+ // so it needs to be considered reactive
4
+ let x = [];
5
+ if (props.cond) {
6
+ x.push(1);
7
+ }
8
+ // Since x is reactive, y is now reactively controlled too:
9
+ let y = false;
10
+ switch (x[0]) {
11
+ case 1: {
12
+ y = true;
13
+ break;
14
+ }
15
+ }
16
+ // Thus this value should be reactive on `y`:
17
+ return [y];
18
+}
19
+
20
+export const FIXTURE_ENTRYPOINT = {
21
+ fn: Component,
22
+ params: [{ cond: true }],
23
+};