From af174973a4b6f979e5d4fc8d6efcaecd28a96bbf Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 12 Jun 2024 14:10:20 -0700 Subject: [PATCH] [compiler] Fix merging of queues states in InferReferenceEffects Fixes a bug found by mofeiZ in #29878. When we merge queues 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: cb931b77f82a118fc6e434f2cd7f6368f9add3d4 Pull Request resolved: https://github.com/facebook/react/pull/29879 --- .../src/Inference/InferReferenceEffects.ts | 5 +- .../compiler/phi-reference-effects.expect.md | 61 +++++++++++++++++++ .../compiler/phi-reference-effects.ts | 19 ++++++ 3 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.ts diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts index 619d1d90ffbce..58e7668cded9c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReferenceEffects.ts @@ -34,6 +34,7 @@ import { } from "../HIR/HIR"; import { FunctionSignature } from "../HIR/ObjectShape"; import { + printFunction, printIdentifier, printMixedHIR, printPlace, @@ -201,7 +202,7 @@ export default function inferReferenceEffects( let queuedState = queuedStates.get(blockId); if (queuedState != null) { // merge the queued states for this block - state = queuedState.merge(state) ?? state; + state = queuedState.merge(state) ?? queuedState; queuedStates.set(blockId, state); } else { /* @@ -765,7 +766,7 @@ class InferenceState { result.values[id] = { kind, value: printMixedHIR(value) }; } for (const [variable, values] of this.#variables) { - result.variables[variable] = [...values].map(identify); + result.variables[`$${variable}`] = [...values].map(identify); } return result; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.expect.md new file mode 100644 index 0000000000000..caab2cd74df75 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.expect.md @@ -0,0 +1,61 @@ + +## Input + +```javascript +import { arrayPush } from "shared-runtime"; + +function Foo(cond) { + let x = null; + if (cond) { + x = []; + } else { + } + // Here, x = phi(x$null, x$[]) does not receive the correct ValueKind + arrayPush(x, 2); + + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: true }], + sequentialRenders: [{ cond: true }, { cond: true }], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { arrayPush } from "shared-runtime"; + +function Foo(cond) { + const $ = _c(2); + let x; + if ($[0] !== cond) { + x = null; + if (cond) { + x = []; + } + + arrayPush(x, 2); + $[0] = cond; + $[1] = x; + } else { + x = $[1]; + } + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: true }], + sequentialRenders: [{ cond: true }, { cond: true }], +}; + +``` + +### Eval output +(kind: ok) [2] +[2] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.ts new file mode 100644 index 0000000000000..31337809e3be9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/phi-reference-effects.ts @@ -0,0 +1,19 @@ +import { arrayPush } from "shared-runtime"; + +function Foo(cond) { + let x = null; + if (cond) { + x = []; + } else { + } + // Here, x = phi(x$null, x$[]) does not receive the correct ValueKind + arrayPush(x, 2); + + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Foo, + params: [{ cond: true }], + sequentialRenders: [{ cond: true }, { cond: true }], +};