@samitouri / QOS-React / commits / d4cdaf2523

Fix assignment expression with context variables

Fixes the one case discovered in the previous PR; for AssignmentExpression we correctly lowered the store instruction to a local/context, but then always used a `LoadLocal` to read the result back. The load instruction appears like it might be dangling - i think what was happening is that DCE cleaned up the unused LoadLocal whereas it leaves the LoadContext alone. But this works for now, we can always clean up the extra instruction later since this case isn't too common.

Joe Savona committed Feb 9, 2024 at 14:25 UTC d4cdaf252337027615812a01c366c7ae0265ea95
4 files changed +54 -26
compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts
+2 -1
@@ -1869,6 +1869,7 @@ function lowerExpression(
1869 type: null,
1870 loc: exprLoc,
1871 });
1872 + return { kind: "LoadLocal", place: identifier, loc: exprLoc };
1873 } else {
1874 lowerValueToTemporary(builder, {
1875 kind: "StoreContext",
@@ -1879,8 +1880,8 @@ function lowerExpression(
1880 value: { ...binaryPlace },
1881 loc: exprLoc,
1882 });
1883 + return { kind: "LoadContext", place: identifier, loc: exprLoc };
1884 }
1883 - return { kind: "LoadLocal", place: identifier, loc: exprLoc };
1885 }
1886 case "MemberExpression": {
1887 // a.b.c += <right>
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-repro-scope-missing-mutable-range.expect.md deleted
-25
@@ -1,25 +0,0 @@
1 -
2 -## Input
3 -
4 -```javascript
5 -function HomeDiscoStoreItemTileRating(props) {
6 - const item = useFragment();
7 - let count = 0;
8 - const aggregates = item?.aggregates || [];
9 - aggregates.forEach((aggregate) => {
10 - count += aggregate.count || 0;
11 - });
12 -
13 - return <Text>{count}</Text>;
14 -}
15 -
16 -```
17 -
18 -
19 -## Error
20 -
21 -```
22 -[ReactForget] Invariant: Expected all references to a variable to be consistently local or context references. Identifier <unknown> count$6 is referenced as a local variable, but was previously referenced as a context variable (6:6)
23 -```
24 -
25 -
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.expect.md new
+52
@@ -0,0 +1,52 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +function HomeDiscoStoreItemTileRating(props) {
6 + const item = useFragment();
7 + let count = 0;
8 + const aggregates = item?.aggregates || [];
9 + aggregates.forEach((aggregate) => {
10 + count += aggregate.count || 0;
11 + });
12 +
13 + return <Text>{count}</Text>;
14 +}
15 +
16 +```
17 +
18 +## Code
19 +
20 +```javascript
21 +import { unstable_useMemoCache as useMemoCache } from "react";
22 +function HomeDiscoStoreItemTileRating(props) {
23 + const $ = useMemoCache(4);
24 + const item = useFragment();
25 + let count;
26 + if ($[0] !== item) {
27 + count = 0;
28 + const aggregates = item?.aggregates || [];
29 + aggregates.forEach((aggregate) => {
30 + count = count + (aggregate.count || 0);
31 + count;
32 + });
33 + $[0] = item;
34 + $[1] = count;
35 + } else {
36 + count = $[1];
37 + }
38 +
39 + const t0 = count;
40 + let t1;
41 + if ($[2] !== t0) {
42 + t1 = <Text>{t0}</Text>;
43 + $[2] = t0;
44 + $[3] = t1;
45 + } else {
46 + t1 = $[3];
47 + }
48 + return t1;
49 +}
50 +
51 +```
52 +
\ No newline at end of file
compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-scope-missing-mutable-range.js renamed