@samitouri / QOS-React-1 / commits / 55fdcf87bd

[compiler] Fix merging of queued states in InferReferenceEffects

Fixes a bug found by mofeiZ in #29878. When we merge queued states, if the new state does not introduce changes relative to the queued state we should use the queued state, not the new state. ghstack-source-id: c59f69de15fa89bd1ed049d0a7d221651577ae34 Pull Request resolved: https://github.com/facebook/react/pull/29879

Joe Savona committed Jun 12, 2024 at 14:49 UTC 55fdcf87bdab39853252369ad0dc68cb88a15102
3 files changed +82 -2
compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts
+2 -2
@@ -201,7 +201,7 @@ export default function inferReferenceEffects(
201 let queuedState = queuedStates.get(blockId);
202 if (queuedState != null) {
203 // merge the queued states for this block
204 - state = queuedState.merge(state) ?? state;
204 + state = queuedState.merge(state) ?? queuedState;
205 queuedStates.set(blockId, state);
206 } else {
207 /*
@@ -765,7 +765,7 @@ class InferenceState {
765 result.values[id] = { kind, value: printMixedHIR(value) };
766 }
767 for (const [variable, values] of this.#variables) {
768 - result.variables[variable] = [...values].map(identify);
768 + result.variables[`$${variable}`] = [...values].map(identify);
769 }
770 return result;
771 }
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.expect.md new
+61
@@ -0,0 +1,61 @@
1 +
2 +## Input
3 +
4 +```javascript
5 +import { arrayPush } from "shared-runtime";
6 +
7 +function Foo(cond) {
8 + let x = null;
9 + if (cond) {
10 + x = [];
11 + } else {
12 + }
13 + // Here, x = phi(x$null, x$[]) should receive a ValueKind of Mutable
14 + arrayPush(x, 2);
15 +
16 + return x;
17 +}
18 +
19 +export const FIXTURE_ENTRYPOINT = {
20 + fn: Foo,
21 + params: [{ cond: true }],
22 + sequentialRenders: [{ cond: true }, { cond: true }],
23 +};
24 +
25 +```
26 +
27 +## Code
28 +
29 +```javascript
30 +import { c as _c } from "react/compiler-runtime";
31 +import { arrayPush } from "shared-runtime";
32 +
33 +function Foo(cond) {
34 + const $ = _c(2);
35 + let x;
36 + if ($[0] !== cond) {
37 + x = null;
38 + if (cond) {
39 + x = [];
40 + }
41 +
42 + arrayPush(x, 2);
43 + $[0] = cond;
44 + $[1] = x;
45 + } else {
46 + x = $[1];
47 + }
48 + return x;
49 +}
50 +
51 +export const FIXTURE_ENTRYPOINT = {
52 + fn: Foo,
53 + params: [{ cond: true }],
54 + sequentialRenders: [{ cond: true }, { cond: true }],
55 +};
56 +
57 +```
58 +
59 +### Eval output
60 +(kind: ok) [2]
61 +[2]
\ No newline at end of file
compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.ts new
+19
@@ -0,0 +1,19 @@
1 +import { arrayPush } from "shared-runtime";
2 +
3 +function Foo(cond) {
4 + let x = null;
5 + if (cond) {
6 + x = [];
7 + } else {
8 + }
9 + // Here, x = phi(x$null, x$[]) should receive a ValueKind of Mutable
10 + arrayPush(x, 2);
11 +
12 + return x;
13 +}
14 +
15 +export const FIXTURE_ENTRYPOINT = {
16 + fn: Foo,
17 + params: [{ cond: true }],
18 + sequentialRenders: [{ cond: true }, { cond: true }],
19 +};