@samitouri / QOS-React-2 / commits / 57163f0a52

InferReactivePlaces account for immutable aliases of mutably aliased values

I found this by adding logic to reject inputs where reactivity gets newly propagated in PruneNonReactiveDependencies. It's possible to create a readonly alias to a mutable value such that we don't know the value is reactive yet when the alias is created. Thus we need to do a fixpoint iteration even if there are no loops in order to be able to revisit such aliases and reflow the reactivity forward. Example: ```javascript const x = []; const y = x; const z = [y]; // y isn't reactive yet when we first visit this, so z is initially non-reactive y.push(props.value); // then we realize y is reactive. we need a fixpoint to propagate this back to z const a = [z]; // need an indirection to get past the partial propagation in PruneNonReactiveDependencies let b = 0; if (a[0][0]) { b = 1; } return [b]; ``` Existing fixtures don't change because the basic reactivity propagation in PruneNonReactiveDependencies is enough to make common cases work. I confirmed that the new fixture does not work on previous PR in the stack.

Joe Savona committed Jan 22, 2024 at 15:35 UTC 57163f0a52bed6229364cc558a786595cd6fa9d3
3 files changed +157 -3
compiler/packages/babel-plugin-react-forget/src/Inference/InferReactivePlaces.ts
+20 -3
@@ -23,7 +23,6 @@ import {
23 eachInstructionValueOperand,
24 eachTerminalOperand,
25 } from "../HIR/visitors";
26 -import { hasBackEdge } from "../Optimization/DeadCodeElimination";
26 import {
27 findDisjointMutableValues,
28 isMutable,
@@ -89,6 +88,25 @@ import { assertExhaustive } from "../Utils/utils";
88 * The algorithm uses a fixpoint iteration in order to propagate reactivity "forward" through
89 * the control-flow graph. We track whether each IdentifierId is reactive and terminate when
90 * there are no changes after a given pass over the CFG.
91 + *
92 + * Note that in Forget it's possible to create a "readonly" reference to a value where
93 + * the reference is created within that value's mutable range:
94 + *
95 + * ```javascript
96 + * const x = [];
97 + * const z = [x];
98 + * x.push(props.input);
99 + *
100 + * return <div>{z}</div>;
101 + * ```
102 + *
103 + * Here `z` is never used to mutate the value, but it is aliasing `x` which
104 + * is mutated after the creation of the alias. The pass needs to account for
105 + * values which become reactive via mutability, and propagate this reactivity
106 + * to these readonly aliases. Using forward data flow is insufficient since
107 + * this information needs to propagate "backwards" from the `x.push(props.input)`
108 + * to the previous `z = [x]` line. We use a fixpoint iteration even if the
109 + * program has no back edges to accomplish this.
110 */
111 export function inferReactivePlaces(fn: HIRFunction): void {
112 const reactiveIdentifiers = new ReactivityMap(findDisjointMutableValues(fn));
@@ -137,7 +155,6 @@ export function inferReactivePlaces(fn: HIRFunction): void {
155 return false;
156 }
157
140 - const hasLoop = hasBackEdge(fn);
158 do {
159 const identifierMapping = new Map<Identifier, Identifier>();
160 for (const [, block] of fn.body.blocks) {
@@ -270,7 +287,7 @@ export function inferReactivePlaces(fn: HIRFunction): void {
287 reactiveIdentifiers.isReactive(operand);
288 }
289 }
273 - } while (reactiveIdentifiers.snapshot() && hasLoop);
290 + } while (reactiveIdentifiers.snapshot());
291 }
292
293 /*
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-via-readonly-alias-of-mutable-value.expect.md new
+100
@@ -0,0 +1,100 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function Component(props) {
6 + const x = [];
7 + const y = x;
8 +
9 + // y isn't reactive yet when we first visit this, so z is initially non-reactive
10 + const z = [y];
11 +
12 + // then we realize y is reactive. we need a fixpoint to propagate this back to z
13 + y.push(props.input);
14 +
15 + // PruneNonReactiveDependencies partially propagates reactivity (for now) which
16 + // we bypass with an indirection of storing into another variable
17 + const a = [z];
18 +
19 + // b's value is conditional on `a`, which is reactive per above
20 + let b = 0;
21 + if (a[0][0][0] === 42) {
22 + b = 1;
23 + }
24 +
25 + return [b];
26 +}
27 +
28 +export const FIXTURE_ENTRYPOINT = {
29 + fn: Component,
30 + params: [],
31 + sequentialRenders: [
32 + { input: 42 },
33 + { input: 42 },
34 + { input: "sathya" },
35 + { input: "sathya" },
36 + { input: 42 },
37 + { input: "sathya" },
38 + { input: 42 },
39 + { input: "sathya" },
40 + ],
41 +};
42 +
43 +```
44 +
45 +## Code
46 +
47 +```javascript
48 +import { unstable_useMemoCache as useMemoCache } from "react";
49 +function Component(props) {
50 + const $ = useMemoCache(2);
51 + const x = [];
52 + const y = x;
53 +
54 + const z = [y];
55 +
56 + y.push(props.input);
57 +
58 + const a = [z];
59 +
60 + let b = 0;
61 + if (a[0][0][0] === 42) {
62 + b = 1;
63 + }
64 + let t0;
65 + if ($[0] !== b) {
66 + t0 = [b];
67 + $[0] = b;
68 + $[1] = t0;
69 + } else {
70 + t0 = $[1];
71 + }
72 + return t0;
73 +}
74 +
75 +export const FIXTURE_ENTRYPOINT = {
76 + fn: Component,
77 + params: [],
78 + sequentialRenders: [
79 + { input: 42 },
80 + { input: 42 },
81 + { input: "sathya" },
82 + { input: "sathya" },
83 + { input: 42 },
84 + { input: "sathya" },
85 + { input: 42 },
86 + { input: "sathya" },
87 + ],
88 +};
89 +
90 +```
91 +
92 +### Eval output
93 +(kind: ok) [1]
94 +[1]
95 +[0]
96 +[0]
97 +[1]
98 +[0]
99 +[1]
100 +[0]
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/reactivity-via-readonly-alias-of-mutable-value.js new
+37
@@ -0,0 +1,37 @@
1 +function Component(props) {
2 + const x = [];
3 + const y = x;
4 +
5 + // y isn't reactive yet when we first visit this, so z is initially non-reactive
6 + const z = [y];
7 +
8 + // then we realize y is reactive. we need a fixpoint to propagate this back to z
9 + y.push(props.input);
10 +
11 + // PruneNonReactiveDependencies partially propagates reactivity (for now) which
12 + // we bypass with an indirection of storing into another variable
13 + const a = [z];
14 +
15 + // b's value is conditional on `a`, which is reactive per above
16 + let b = 0;
17 + if (a[0][0][0] === 42) {
18 + b = 1;
19 + }
20 +
21 + return [b];
22 +}
23 +
24 +export const FIXTURE_ENTRYPOINT = {
25 + fn: Component,
26 + params: [],
27 + sequentialRenders: [
28 + { input: 42 },
29 + { input: 42 },
30 + { input: "sathya" },
31 + { input: "sathya" },
32 + { input: 42 },
33 + { input: "sathya" },
34 + { input: 42 },
35 + { input: "sathya" },
36 + ],
37 +};