From 963970b7a2b0fd08669095de9b88011e6dfdcd64 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 18 Jun 2025 15:59:35 -0700 Subject: [PATCH 1/3] [compiler] Fix By accident we were only ever checking the compiled output, but the intention was in general to be able to compare memoization with/without forget. --- ...-control-flow-sensitive-mutation.expect.md | 13 +++---- .../todo-control-flow-sensitive-mutation.tsx | 9 ++--- ...tivity-createfrom-capture-lambda.expect.md | 4 +-- ...transitivity-createfrom-capture-lambda.tsx | 2 +- ...ity-add-captured-array-to-itself.expect.md | 8 ++--- ...nsitivity-add-captured-array-to-itself.tsx | 4 +-- ...tivity-capture-createfrom-lambda.expect.md | 4 +-- ...transitivity-capture-createfrom-lambda.tsx | 2 +- .../transitivity-capture-createfrom.expect.md | 4 +-- .../transitivity-capture-createfrom.tsx | 2 +- .../transitivity-createfrom-capture.expect.md | 4 +-- .../transitivity-createfrom-capture.tsx | 2 +- ...ansitivity-phi-assign-or-capture.expect.md | 4 +-- .../transitivity-phi-assign-or-capture.tsx | 2 +- ...d-identity-function-frozen-input.expect.md | 34 ++++++++++-------- .../typed-identity-function-frozen-input.js | 3 +- .../compiler/weakmap-constructor.expect.md | 36 +++++++++++++++---- .../fixtures/compiler/weakmap-constructor.js | 19 ++++++++-- .../compiler/weakset-constructor.expect.md | 36 +++++++++++++++---- .../fixtures/compiler/weakset-constructor.js | 19 ++++++++-- .../snap/src/sprout/shared-runtime.ts | 4 +-- 21 files changed, 141 insertions(+), 74 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.expect.md index d3555c0b2711a..a3cf9f638aefc 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.expect.md @@ -23,14 +23,9 @@ function Component({a, b, c}: {a: number; b: number; c: number}) { return ( <> - ; + ; {/* TODO: should only depend on c */} - - ; + ; ); } @@ -98,7 +93,7 @@ function Component(t0) { } let t3; if ($[9] !== t2 || $[10] !== x) { - t3 = ; + t3 = ; $[9] = t2; $[10] = x; $[11] = t3; @@ -117,7 +112,7 @@ function Component(t0) { } let t5; if ($[16] !== t4 || $[17] !== x[0]) { - t5 = ; + t5 = ; $[16] = t4; $[17] = x[0]; $[18] = t5; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.tsx index 80386c46af86d..61f8c47e453d4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-control-flow-sensitive-mutation.tsx @@ -19,14 +19,9 @@ function Component({a, b, c}: {a: number; b: number; c: number}) { return ( <> - ; + ; {/* TODO: should only depend on c */} - - ; + ; ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.expect.md index 9210bef3fb42c..9dba055973f52 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.expect.md @@ -22,7 +22,7 @@ function Component({a, b}) { typedMutate(z, b); // TODO: this *should* only depend on `a` - return ; + return ; } export const FIXTURE_ENTRYPOINT = { @@ -86,7 +86,7 @@ function Component(t0) { } let t3; if ($[7] !== t2 || $[8] !== x) { - t3 = ; + t3 = ; $[7] = t2; $[8] = x; $[9] = t3; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.tsx index 9e10bec1b4b87..d6bd1690f6557 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/todo-transitivity-createfrom-capture-lambda.tsx @@ -18,7 +18,7 @@ function Component({a, b}) { typedMutate(z, b); // TODO: this *should* only depend on `a` - return ; + return ; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.expect.md index 2813e072e2a12..fb3c8d0a8958b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.expect.md @@ -20,8 +20,8 @@ function Component({a, b}) { return ( <> - ; - ; + ; + ; ); } @@ -92,7 +92,7 @@ function Component(t0) { } let t5; if ($[8] !== o || $[9] !== t4) { - t5 = ; + t5 = ; $[8] = o; $[9] = t4; $[10] = t5; @@ -110,7 +110,7 @@ function Component(t0) { } let t7; if ($[14] !== t6 || $[15] !== x) { - t7 = ; + t7 = ; $[14] = t6; $[15] = x; $[16] = t7; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.tsx index 7d7ec91bc5936..d81c069e336ba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-add-captured-array-to-itself.tsx @@ -16,8 +16,8 @@ function Component({a, b}) { return ( <> - ; - ; + ; + ; ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.expect.md index 85c356f894bff..bfb4ede0ad08c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.expect.md @@ -21,7 +21,7 @@ function Component({a, b}: {a: number; b: number}) { // mutates x typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { @@ -85,7 +85,7 @@ function Component(t0) { } let t3; if ($[7] !== t2 || $[8] !== x) { - t3 = ; + t3 = ; $[7] = t2; $[8] = x; $[9] = t3; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.tsx index cee7901a6eefa..72289eb833571 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom-lambda.tsx @@ -17,7 +17,7 @@ function Component({a, b}: {a: number; b: number}) { // mutates x typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.expect.md index af15d568cebee..bd0a76965d42c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.expect.md @@ -17,7 +17,7 @@ function Component({a, b}: {a: number; b: number}) { // mutates x typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { @@ -76,7 +76,7 @@ function Component(t0) { } let t3; if ($[7] !== t2 || $[8] !== x) { - t3 = ; + t3 = ; $[7] = t2; $[8] = x; $[9] = t3; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.tsx index df89c3413e1d8..d06ad11eb5753 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-capture-createfrom.tsx @@ -13,7 +13,7 @@ function Component({a, b}: {a: number; b: number}) { // mutates x typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.expect.md index 25dbcdf3ac09d..742a233e80195 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.expect.md @@ -17,7 +17,7 @@ function Component({a, b}) { // does not mutate x, so x should not depend on b typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { @@ -73,7 +73,7 @@ function Component(t0) { } let t4; if ($[4] !== t3 || $[5] !== x) { - t4 = ; + t4 = ; $[4] = t3; $[5] = x; $[6] = t4; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.tsx index a8a02564bd6c7..32d65e61e01ec 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-createfrom-capture.tsx @@ -13,7 +13,7 @@ function Component({a, b}) { // does not mutate x, so x should not depend on b typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.expect.md index b5350352cfd21..c50fe47aed6f9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.expect.md @@ -21,7 +21,7 @@ function Component({a, b}) { // could mutate x typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { @@ -92,7 +92,7 @@ function Component(t0) { } let t4; if ($[9] !== t3 || $[10] !== x) { - t4 = ; + t4 = ; $[9] = t3; $[10] = x; $[11] = t4; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.tsx b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.tsx index e583bfa83aea0..90b7597694607 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.tsx +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitivity-phi-assign-or-capture.tsx @@ -17,7 +17,7 @@ function Component({a, b}) { // could mutate x typedMutate(z, b); - return ; + return ; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.expect.md index b15248df0782c..dc8bedaf83522 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.expect.md @@ -4,6 +4,7 @@ ```javascript // @enableNewMutationAliasingModel +import {useMemo} from 'react'; import { identity, makeObject_Primitives, @@ -14,7 +15,7 @@ import { function Component({a, b}) { // create a mutable value with input `a` - const x = makeObject_Primitives(a); + const x = useMemo(() => makeObject_Primitives(a), [a]); // freeze the value useIdentity(x); @@ -49,6 +50,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +import { useMemo } from "react"; import { identity, makeObject_Primitives, @@ -61,13 +63,15 @@ function Component(t0) { const $ = _c(7); const { a, b } = t0; let t1; + let t2; if ($[0] !== a) { - t1 = makeObject_Primitives(a); + t2 = makeObject_Primitives(a); $[0] = a; - $[1] = t1; + $[1] = t2; } else { - t1 = $[1]; + t2 = $[1]; } + t1 = t2; const x = t1; useIdentity(x); @@ -75,24 +79,24 @@ function Component(t0) { const x2 = typedIdentity(x); identity(x2, b); - let t2; + let t3; if ($[2] !== a) { - t2 = [a]; + t3 = [a]; $[2] = a; - $[3] = t2; + $[3] = t3; } else { - t2 = $[3]; + t3 = $[3]; } - let t3; - if ($[4] !== t2 || $[5] !== x) { - t3 = ; - $[4] = t2; + let t4; + if ($[4] !== t3 || $[5] !== x) { + t4 = ; + $[4] = t3; $[5] = x; - $[6] = t3; + $[6] = t4; } else { - t3 = $[6]; + t4 = $[6]; } - return t3; + return t4; } export const FIXTURE_ENTRYPOINT = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.js index bcf6ecef02367..d0f677ee4df17 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/typed-identity-function-frozen-input.js @@ -1,5 +1,6 @@ // @enableNewMutationAliasingModel +import {useMemo} from 'react'; import { identity, makeObject_Primitives, @@ -10,7 +11,7 @@ import { function Component({a, b}) { // create a mutable value with input `a` - const x = makeObject_Primitives(a); + const x = useMemo(() => makeObject_Primitives(a), [a]); // freeze the value useIdentity(x); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.expect.md index aebaedf6a8754..a6def457a4194 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +import {useMemo} from 'react'; import {ValidateMemoization} from 'shared-runtime'; function Component({a, b, c}) { @@ -13,9 +14,21 @@ function Component({a, b, c}) { return ( <> - - - + + + ); } @@ -44,6 +57,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; +import { useMemo } from "react"; import { ValidateMemoization } from "shared-runtime"; function Component(t0) { @@ -76,7 +90,9 @@ function Component(t0) { } let t2; if ($[7] !== map || $[8] !== t1) { - t2 = ; + t2 = ( + + ); $[7] = map; $[8] = t1; $[9] = t2; @@ -94,7 +110,13 @@ function Component(t0) { } let t4; if ($[13] !== mapAlias || $[14] !== t3) { - t4 = ; + t4 = ( + + ); $[13] = mapAlias; $[14] = t3; $[15] = t4; @@ -119,7 +141,9 @@ function Component(t0) { } let t7; if ($[20] !== t5 || $[21] !== t6) { - t7 = ; + t7 = ( + + ); $[20] = t5; $[21] = t6; $[22] = t7; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.js index ecdec6c9e9621..d005c9f271d17 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakmap-constructor.js @@ -1,3 +1,4 @@ +import {useMemo} from 'react'; import {ValidateMemoization} from 'shared-runtime'; function Component({a, b, c}) { @@ -9,9 +10,21 @@ function Component({a, b, c}) { return ( <> - - - + + + ); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.expect.md index 5ebf32d533c91..94e0c7f05547b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.expect.md @@ -2,6 +2,7 @@ ## Input ```javascript +import {useMemo} from 'react'; import {ValidateMemoization} from 'shared-runtime'; function Component({a, b, c}) { @@ -13,9 +14,21 @@ function Component({a, b, c}) { return ( <> - - - + + + ); } @@ -44,6 +57,7 @@ export const FIXTURE_ENTRYPOINT = { ```javascript import { c as _c } from "react/compiler-runtime"; +import { useMemo } from "react"; import { ValidateMemoization } from "shared-runtime"; function Component(t0) { @@ -76,7 +90,9 @@ function Component(t0) { } let t2; if ($[7] !== set || $[8] !== t1) { - t2 = ; + t2 = ( + + ); $[7] = set; $[8] = t1; $[9] = t2; @@ -94,7 +110,13 @@ function Component(t0) { } let t4; if ($[13] !== setAlias || $[14] !== t3) { - t4 = ; + t4 = ( + + ); $[13] = setAlias; $[14] = t3; $[15] = t4; @@ -119,7 +141,9 @@ function Component(t0) { } let t7; if ($[20] !== t5 || $[21] !== t6) { - t7 = ; + t7 = ( + + ); $[20] = t5; $[21] = t6; $[22] = t7; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.js index 8c0a7deb186f6..9114233812e55 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/weakset-constructor.js @@ -1,3 +1,4 @@ +import {useMemo} from 'react'; import {ValidateMemoization} from 'shared-runtime'; function Component({a, b, c}) { @@ -9,9 +10,21 @@ function Component({a, b, c}) { return ( <> - - - + + + ); } diff --git a/compiler/packages/snap/src/sprout/shared-runtime.ts b/compiler/packages/snap/src/sprout/shared-runtime.ts index 1e0ab108db3f1..f37ca82709022 100644 --- a/compiler/packages/snap/src/sprout/shared-runtime.ts +++ b/compiler/packages/snap/src/sprout/shared-runtime.ts @@ -269,12 +269,10 @@ export function ValidateMemoization({ inputs, output: rawOutput, onlyCheckCompiled = false, - alwaysCheck = false, }: { inputs: Array; output: any; onlyCheckCompiled?: boolean; - alwaysCheck?: boolean; }): React.ReactElement { 'use no forget'; // Wrap rawOutput as it might be a function, which useState would invoke. @@ -282,7 +280,7 @@ export function ValidateMemoization({ const [previousInputs, setPreviousInputs] = React.useState(inputs); const [previousOutput, setPreviousOutput] = React.useState(output); if ( - alwaysCheck || + !onlyCheckCompiled || (onlyCheckCompiled && (globalThis as any).__SNAP_EVALUATOR_MODE === 'forget') ) { From fc2c4f2f15942cfc49e9766e2f5b6fe8dbe4e583 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 18 Jun 2025 15:59:35 -0700 Subject: [PATCH 2/3] [compiler] Preserve Create effects, guarantee effects initialize once Ensures that effects are well-formed with respect to the rules: * For a given instruction, each place is only initialized once (w one of Create, CreateFrom, Assign) * Ensures that Alias targets are already initialized within the same instruction (should have a Create before them) * Preserves Create and similar instructions * Avoids duplicate instructions when inferring effects of function expressions --- .../src/Inference/AnalyseFunctions.ts | 14 ++- .../Inference/InferMutationAliasingEffects.ts | 102 +++++++++++++----- 2 files changed, 88 insertions(+), 28 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index 2bd46abc1f75f..eed8946c81496 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -20,10 +20,11 @@ import {inferReactiveScopeVariables} from '../ReactiveScopes'; import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; import {inferMutableRanges} from './InferMutableRanges'; import inferReferenceEffects from './InferReferenceEffects'; -import {assertExhaustive} from '../Utils/utils'; +import {assertExhaustive, retainWhere} from '../Utils/utils'; import {inferMutationAliasingEffects} from './InferMutationAliasingEffects'; import {inferFunctionExpressionAliasingEffectsSignature} from './InferFunctionExpressionAliasingEffectsSignature'; import {inferMutationAliasingRanges} from './InferMutationAliasingRanges'; +import {hashEffect} from './AliasingEffects'; export default function analyseFunctions(func: HIRFunction): void { for (const [_, block] of func.body.blocks) { @@ -81,6 +82,17 @@ function lowerWithMutationAliasing(fn: HIRFunction): void { fn.aliasingEffects ??= []; fn.aliasingEffects?.push(...effects); } + if (fn.aliasingEffects != null) { + const seen = new Set(); + retainWhere(fn.aliasingEffects, effect => { + const hash = hashEffect(effect); + if (seen.has(hash)) { + return false; + } + seen.add(hash); + return true; + }); + } /** * Phase 2: populate the Effect of each context variable to use in inferring diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 93f00508b2042..0ef421ad6bf7c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -362,6 +362,11 @@ function inferBlock( } else if (terminal.kind === 'maybe-throw') { const handlerParam = context.catchHandlers.get(terminal.handler); if (handlerParam != null) { + CompilerError.invariant(state.kind(handlerParam) != null, { + reason: + 'Expected catch binding to be intialized with a DeclareLocal Catch instruction', + loc: terminal.loc, + }); const effects: Array = []; for (const instr of block.instructions) { if ( @@ -476,14 +481,14 @@ function applySignature( * Track which values we've already aliased once, so that we can switch to * appendAlias() for subsequent aliases into the same value */ - const aliased = new Set(); + const initialized = new Set(); if (DEBUG) { console.log(printInstruction(instruction)); } for (const effect of signature.effects) { - applyEffect(context, state, effect, aliased, effects); + applyEffect(context, state, effect, initialized, effects); } if (DEBUG) { console.log( @@ -508,7 +513,7 @@ function applyEffect( context: Context, state: InferenceState, _effect: AliasingEffect, - aliased: Set, + initialized: Set, effects: Array, ): void { const effect = context.internEffect(_effect); @@ -524,6 +529,13 @@ function applyEffect( break; } case 'Create': { + CompilerError.invariant(!initialized.has(effect.into.identifier.id), { + reason: `Cannot re-initialize variable within an instruction`, + description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`, + loc: effect.into.loc, + }); + initialized.add(effect.into.identifier.id); + let value = context.effectInstructionValueCache.get(effect); if (value == null) { value = { @@ -538,6 +550,7 @@ function applyEffect( reason: new Set([effect.reason]), }); state.define(effect.into, value); + effects.push(effect); break; } case 'ImmutableCapture': { @@ -555,6 +568,13 @@ function applyEffect( break; } case 'CreateFrom': { + CompilerError.invariant(!initialized.has(effect.into.identifier.id), { + reason: `Cannot re-initialize variable within an instruction`, + description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`, + loc: effect.into.loc, + }); + initialized.add(effect.into.identifier.id); + const fromValue = state.kind(effect.from); let value = context.effectInstructionValueCache.get(effect); if (value == null) { @@ -573,10 +593,21 @@ function applyEffect( switch (fromValue.kind) { case ValueKind.Primitive: case ValueKind.Global: { - // no need to track this data flow + effects.push({ + kind: 'Create', + value: fromValue.kind, + into: effect.into, + reason: [...fromValue.reason][0] ?? ValueReason.Other, + }); break; } case ValueKind.Frozen: { + effects.push({ + kind: 'Create', + value: fromValue.kind, + into: effect.into, + reason: [...fromValue.reason][0] ?? ValueReason.Other, + }); applyEffect( context, state, @@ -585,7 +616,7 @@ function applyEffect( from: effect.from, into: effect.into, }, - aliased, + initialized, effects, ); break; @@ -597,6 +628,13 @@ function applyEffect( break; } case 'CreateFunction': { + CompilerError.invariant(!initialized.has(effect.into.identifier.id), { + reason: `Cannot re-initialize variable within an instruction`, + description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`, + loc: effect.into.loc, + }); + initialized.add(effect.into.identifier.id); + effects.push(effect); /** * We consider the function mutable if it has any mutable context variables or @@ -653,7 +691,7 @@ function applyEffect( from: capture, into: effect.into, }, - aliased, + initialized, effects, ); } @@ -661,6 +699,14 @@ function applyEffect( } case 'Alias': case 'Capture': { + CompilerError.invariant( + effect.kind === 'Capture' || initialized.has(effect.into.identifier.id), + { + reason: `Expected destination value to already be initialized within this instruction for Alias effect`, + description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`, + loc: effect.into.loc, + }, + ); /* * Capture describes potential information flow: storing a pointer to one value * within another. If the destination is not mutable, or the source value has @@ -698,7 +744,7 @@ function applyEffect( from: effect.from, into: effect.into, }, - aliased, + initialized, effects, ); break; @@ -714,6 +760,13 @@ function applyEffect( break; } case 'Assign': { + CompilerError.invariant(!initialized.has(effect.into.identifier.id), { + reason: `Cannot re-initialize variable within an instruction`, + description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`, + loc: effect.into.loc, + }); + initialized.add(effect.into.identifier.id); + /* * Alias represents potential pointer aliasing. If the type is a global, * a primitive (copy-on-write semantics) then we can prune the effect @@ -730,7 +783,7 @@ function applyEffect( from: effect.from, into: effect.into, }, - aliased, + initialized, effects, ); let value = context.effectInstructionValueCache.get(effect); @@ -768,12 +821,7 @@ function applyEffect( break; } default: { - if (aliased.has(effect.into.identifier.id)) { - state.appendAlias(effect.into, effect.from); - } else { - aliased.add(effect.into.identifier.id); - state.alias(effect.into, effect.from); - } + state.assign(effect.into, effect.from); effects.push(effect); break; } @@ -826,11 +874,11 @@ function applyEffect( context, state, {kind: 'MutateTransitiveConditionally', value: effect.function}, - aliased, + initialized, effects, ); for (const signatureEffect of signatureEffects) { - applyEffect(context, state, signatureEffect, aliased, effects); + applyEffect(context, state, signatureEffect, initialized, effects); } break; } @@ -858,7 +906,7 @@ function applyEffect( console.log('apply aliasing signature effects'); } for (const signatureEffect of signatureEffects) { - applyEffect(context, state, signatureEffect, aliased, effects); + applyEffect(context, state, signatureEffect, initialized, effects); } } else if (effect.signature != null) { if (DEBUG) { @@ -873,7 +921,7 @@ function applyEffect( effect.loc, ); for (const legacyEffect of legacyEffects) { - applyEffect(context, state, legacyEffect, aliased, effects); + applyEffect(context, state, legacyEffect, initialized, effects); } } else { if (DEBUG) { @@ -888,7 +936,7 @@ function applyEffect( value: ValueKind.Mutable, reason: ValueReason.Other, }, - aliased, + initialized, effects, ); /* @@ -911,21 +959,21 @@ function applyEffect( kind: 'MutateTransitiveConditionally', value: operand, }, - aliased, + initialized, effects, ); } const mutateIterator = arg.kind === 'Spread' ? conditionallyMutateIterator(operand) : null; if (mutateIterator) { - applyEffect(context, state, mutateIterator, aliased, effects); + applyEffect(context, state, mutateIterator, initialized, effects); } applyEffect( context, state, // OK: recording information flow {kind: 'Alias', from: operand, into: effect.into}, - aliased, + initialized, effects, ); for (const otherArg of [ @@ -953,7 +1001,7 @@ function applyEffect( from: operand, into: other, }, - aliased, + initialized, effects, ); } @@ -1009,7 +1057,7 @@ function applyEffect( suggestions: null, }, }, - aliased, + initialized, effects, ); } @@ -1028,7 +1076,7 @@ function applyEffect( suggestions: null, }, }, - aliased, + initialized, effects, ); } else { @@ -1059,7 +1107,7 @@ function applyEffect( suggestions: null, }, }, - aliased, + initialized, effects, ); } @@ -1166,7 +1214,7 @@ class InferenceState { } // Updates the value at @param place to point to the same value as @param value. - alias(place: Place, value: Place): void { + assign(place: Place, value: Place): void { const values = this.#variables.get(value.identifier.id); CompilerError.invariant(values != null, { reason: `[InferMutationAliasingEffects] Expected value for identifier to be initialized`, From a427c3c1d96d687e54785fd2bd1fb9eff03d2af8 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Wed, 18 Jun 2025 15:59:35 -0700 Subject: [PATCH 3/3] [compiler] Cleanup debugging code Removes unnecessary debugging code in the new inference passes now that they've stabilized more. --- .../Inference/InferMutationAliasingEffects.ts | 70 ++---------- .../Inference/InferMutationAliasingRanges.ts | 105 ++---------------- 2 files changed, 17 insertions(+), 158 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts index 0ef421ad6bf7c..b6c9812918ff9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -57,7 +57,6 @@ import { import { printAliasingEffect, printAliasingSignature, - printFunction, printIdentifier, printInstruction, printInstructionValue, @@ -194,19 +193,15 @@ export function inferMutationAliasingEffects( hoistedContextDeclarations, ); - let count = 0; + let iterationCount = 0; while (queuedStates.size !== 0) { - count++; - if (count > 100) { - console.log( - 'oops infinite loop', - fn.id, - typeof fn.loc !== 'symbol' ? fn.loc?.filename : null, - ); - if (DEBUG) { - console.log(printFunction(fn)); - } - throw new Error('infinite loop'); + iterationCount++; + if (iterationCount > 100) { + CompilerError.invariant(false, { + reason: `[InferMutationAliasingEffects] Potential infinite loop`, + description: `A value, temporary place, or effect was not cached properly`, + loc: fn.loc, + }); } for (const [blockId, block] of fn.body.blocks) { const incomingState = queuedStates.get(blockId); @@ -217,11 +212,6 @@ export function inferMutationAliasingEffects( statesByBlock.set(blockId, incomingState); const state = incomingState.clone(); - if (DEBUG) { - console.log('*************'); - console.log(`bb${block.id}`); - console.log('*************'); - } inferBlock(context, state, block); for (const nextBlockId of eachTerminalSuccessor(block.terminal)) { @@ -867,9 +857,6 @@ function applyEffect( ), ); if (signatureEffects != null) { - if (DEBUG) { - console.log('apply function expression effects'); - } applyEffect( context, state, @@ -902,16 +889,10 @@ function applyEffect( ); } if (signatureEffects != null) { - if (DEBUG) { - console.log('apply aliasing signature effects'); - } for (const signatureEffect of signatureEffects) { applyEffect(context, state, signatureEffect, initialized, effects); } } else if (effect.signature != null) { - if (DEBUG) { - console.log('apply legacy signature effects'); - } const legacyEffects = computeEffectsForLegacySignature( state, effect.signature, @@ -924,9 +905,6 @@ function applyEffect( applyEffect(context, state, legacyEffect, initialized, effects); } } else { - if (DEBUG) { - console.log('default effects'); - } applyEffect( context, state, @@ -1292,9 +1270,6 @@ class InferenceState { kind: ValueKind.Frozen, reason: new Set([reason]), }); - if (DEBUG) { - console.log(`freeze value: ${printInstructionValue(value)} ${reason}`); - } if ( value.kind === 'FunctionExpression' && (this.env.config.enablePreserveExistingMemoizationGuarantees || @@ -2334,17 +2309,6 @@ function computeEffectsForSignature( // Too many args and there is no rest param to hold them (args.length > signature.params.length && signature.rest == null) ) { - if (DEBUG) { - if (signature.params.length > args.length) { - console.log( - `not enough args: ${args.length} args for ${signature.params.length} params`, - ); - } else { - console.log( - `too many args: ${args.length} args for ${signature.params.length} params, with no rest param`, - ); - } - } return null; } // Build substitutions @@ -2359,9 +2323,6 @@ function computeEffectsForSignature( continue; } else if (params == null || i >= params.length || arg.kind === 'Spread') { if (signature.rest == null) { - if (DEBUG) { - console.log(`no rest value to hold param`); - } return null; } const place = arg.kind === 'Identifier' ? arg : arg.place; @@ -2469,23 +2430,14 @@ function computeEffectsForSignature( case 'Apply': { const applyReceiver = substitutions.get(effect.receiver.identifier.id); if (applyReceiver == null || applyReceiver.length !== 1) { - if (DEBUG) { - console.log(`too many substitutions for receiver`); - } return null; } const applyFunction = substitutions.get(effect.function.identifier.id); if (applyFunction == null || applyFunction.length !== 1) { - if (DEBUG) { - console.log(`too many substitutions for function`); - } return null; } const applyInto = substitutions.get(effect.into.identifier.id); if (applyInto == null || applyInto.length !== 1) { - if (DEBUG) { - console.log(`too many substitutions for into`); - } return null; } const applyArgs: Array = []; @@ -2495,18 +2447,12 @@ function computeEffectsForSignature( } else if (arg.kind === 'Identifier') { const applyArg = substitutions.get(arg.identifier.id); if (applyArg == null || applyArg.length !== 1) { - if (DEBUG) { - console.log(`too many substitutions for arg`); - } return null; } applyArgs.push(applyArg[0]); } else { const applyArg = substitutions.get(arg.place.identifier.id); if (applyArg == null || applyArg.length !== 1) { - if (DEBUG) { - console.log(`too many substitutions for arg`); - } return null; } applyArgs.push({kind: 'Spread', place: applyArg[0]}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts index e0fb84fe5a1cf..864eb29d8ee80 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import prettyFormat from 'pretty-format'; import {CompilerError, SourceLocation} from '..'; import { BlockId, @@ -23,14 +22,9 @@ import { eachTerminalOperand, } from '../HIR/visitors'; import {assertExhaustive, getOrInsertWith} from '../Utils/utils'; -import {printFunction} from '../HIR'; -import {printIdentifier, printPlace} from '../HIR/PrintHIR'; import {MutationKind} from './InferFunctionExpressionAliasingEffectsSignature'; import {Result} from '../Utils/Result'; -const DEBUG = false; -const VERBOSE = false; - /** * Infers mutable ranges for all values in the program, using previously inferred * mutation/aliasing effects. This pass builds a data flow graph using the effects, @@ -56,10 +50,6 @@ export function inferMutationAliasingRanges( fn: HIRFunction, {isFunctionExpression}: {isFunctionExpression: boolean}, ): Result { - if (VERBOSE) { - console.log(); - console.log(printFunction(fn)); - } /** * Part 1: Infer mutable ranges for values. We build an abstract model of * values, the alias/capture edges between them, and the set of mutations. @@ -115,20 +105,6 @@ export function inferMutationAliasingRanges( seenBlocks.add(block.id); for (const instr of block.instructions) { - if ( - instr.value.kind === 'FunctionExpression' || - instr.value.kind === 'ObjectMethod' - ) { - state.create(instr.lvalue, { - kind: 'Function', - function: instr.value.loweredFunc.func, - }); - } else { - for (const lvalue of eachInstructionLValue(instr)) { - state.create(lvalue, {kind: 'Object'}); - } - } - if (instr.effects == null) continue; for (const effect of instr.effects) { if (effect.kind === 'Create') { @@ -141,6 +117,15 @@ export function inferMutationAliasingRanges( } else if (effect.kind === 'CreateFrom') { state.createFrom(index++, effect.from, effect.into); } else if (effect.kind === 'Assign') { + /** + * TODO: Invariant that the node is not initialized yet + * + * InferFunctionExpressionAliasingEffectSignatures currently infers + * Assign effects in some places that should be Alias, leading to + * Assign effects that reinitialize a value. The end result appears to + * be fine, but we should fix that inference pass so that we add the + * invariant here. + */ if (!state.nodes.has(effect.into.identifier)) { state.create(effect.into, {kind: 'Object'}); } @@ -216,10 +201,6 @@ export function inferMutationAliasingRanges( } } - if (VERBOSE) { - console.log(state.debug()); - console.log(pretty(mutations)); - } for (const mutation of mutations) { state.mutate( mutation.index, @@ -234,9 +215,6 @@ export function inferMutationAliasingRanges( for (const render of renders) { state.render(render.index, render.place.identifier, errors); } - if (DEBUG) { - console.log(pretty([...state.nodes.keys()])); - } fn.aliasingEffects ??= []; for (const param of [...fn.context, ...fn.params]) { const place = param.kind === 'Identifier' ? param : param.place; @@ -458,9 +436,6 @@ export function inferMutationAliasingRanges( } } - if (VERBOSE) { - console.log(printFunction(fn)); - } return errors.asResult(); } @@ -511,11 +486,6 @@ class AliasingState { const fromNode = this.nodes.get(from.identifier); const toNode = this.nodes.get(into.identifier); if (fromNode == null || toNode == null) { - if (VERBOSE) { - console.log( - `skip: createFrom ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, - ); - } return; } fromNode.edges.push({index, node: into.identifier, kind: 'alias'}); @@ -528,11 +498,6 @@ class AliasingState { const fromNode = this.nodes.get(from.identifier); const toNode = this.nodes.get(into.identifier); if (fromNode == null || toNode == null) { - if (VERBOSE) { - console.log( - `skip: capture ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, - ); - } return; } fromNode.edges.push({index, node: into.identifier, kind: 'capture'}); @@ -545,11 +510,6 @@ class AliasingState { const fromNode = this.nodes.get(from.identifier); const toNode = this.nodes.get(into.identifier); if (fromNode == null || toNode == null) { - if (VERBOSE) { - console.log( - `skip: assign ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, - ); - } return; } fromNode.edges.push({index, node: into.identifier, kind: 'alias'}); @@ -604,11 +564,6 @@ class AliasingState { loc: SourceLocation, errors: CompilerError, ): void { - if (DEBUG) { - console.log( - `mutate ix=${index} start=$${start.id} end=[${end}]${transitive ? ' transitive' : ''} kind=${kind}`, - ); - } const seen = new Set(); const queue: Array<{ place: Identifier; @@ -623,18 +578,8 @@ class AliasingState { seen.add(current); const node = this.nodes.get(current); if (node == null) { - if (DEBUG) { - console.log( - `no node! ${printIdentifier(start)} for identifier ${printIdentifier(current)}`, - ); - } continue; } - if (DEBUG) { - console.log( - ` mutate $${node.id.id} transitive=${transitive} direction=${direction}`, - ); - } node.id.mutableRange.end = makeInstructionId( Math.max(node.id.mutableRange.end, end), ); @@ -701,37 +646,5 @@ class AliasingState { } } } - if (DEBUG) { - const nodes = new Map(); - for (const id of seen) { - const node = this.nodes.get(id); - nodes.set(id.id, node); - } - console.log(pretty(nodes)); - } } - - debug(): string { - return pretty(this.nodes); - } -} - -export function pretty(v: any): string { - return prettyFormat(v, { - plugins: [ - { - test: v => - v !== null && typeof v === 'object' && v.kind === 'Identifier', - serialize: v => printPlace(v), - }, - { - test: v => - v !== null && - typeof v === 'object' && - typeof v.declarationId === 'number', - serialize: v => - `${printIdentifier(v)}:${v.mutableRange.start}:${v.mutableRange.end}`, - }, - ], - }); }