From 17ffa32fe975c08bc9ced7b8e275771897965632 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 8 May 2025 16:13:59 -0700 Subject: [PATCH] [compiler] Move co-mutation range extension to InferMutableRanges We've occassionally added logic that extends mutable ranges into InferReactiveScopeVariables to handle a specific case, but inevitably discover that the logic needs to be part of the InferMutableRanges fixpoint loop. That happened in the past with extending the range of phi operands to account for subsequent mutations, which I moved to InferMutableRanges a while back. But InferReactiveScopeVariables also has logic to group co-mutations in the same scope, which also extends ranges of the co-mutating operands to have the same end point. Recently mofeiz found some cases where this is insufficient, where a closure captures a value that could change via a co-mutation, and where failure to extend the ranges in the fixpoint meant the function expression appeared independently memoizable when it wasn't. The fix is to make InferMutableRanges update ranges to account for co-mutations. That is relatively straightforward, but not enough! The problem is that the fixpoint loop stopped once the alias sets coalesced, but co-mutations only affect ranges and not aliases. So the other part of the fix is to have the fixpoint condition use a custom canonicalization that describes each identifiers root _and_ the mutable range of that root. ghstack-source-id: 829d114d127b8c33bf3d373fe566ebaa9d681c3e Pull Request resolved: https://github.com/facebook/react/pull/33157 --- .../src/HIR/PrintHIR.ts | 2 +- .../src/Inference/AnalyseFunctions.ts | 12 -- .../src/Inference/InferMutableRanges.ts | 50 +++++++- .../InferMutableRangesForComutation.ts | 91 +++++++++++++++ .../aliased-capture-aliased-mutate.expect.md | 71 ++++++++++++ .../aliased-capture-aliased-mutate.js | 22 ++++ ...ct.md => aliased-capture-mutate.expect.md} | 30 +---- ...re-mutate.js => aliased-capture-mutate.js} | 13 --- ...g-aliased-capture-aliased-mutate.expect.md | 107 ------------------ .../bug-aliased-capture-aliased-mutate.js | 53 --------- .../packages/snap/src/SproutTodoFilter.ts | 2 - 11 files changed, 236 insertions(+), 217 deletions(-) create mode 100644 compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md create mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{bug-aliased-capture-mutate.expect.md => aliased-capture-mutate.expect.md} (62%) rename compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/{bug-aliased-capture-mutate.js => aliased-capture-mutate.js} (52%) delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md delete mode 100644 compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 93acb4c944..5325494f57 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -731,7 +731,7 @@ function isMutable(range: MutableRange): boolean { } const DEBUG_MUTABLE_RANGES = false; -function printMutableRange(identifier: Identifier): string { +export function printMutableRange(identifier: Identifier): string { if (DEBUG_MUTABLE_RANGES) { // if debugging, print both the identifier and scope range if they differ const range = identifier.mutableRange; 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 b7322d901a..fbd4ce8fa0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -25,8 +25,6 @@ import { eachInstructionLValue, eachInstructionValueOperand, } from '../HIR/visitors'; -import prettyFormat from 'pretty-format'; -import {printIdentifier} from '../HIR/PrintHIR'; import {Iterable_some} from '../Utils/utils'; export default function analyseFunctions(func: HIRFunction): void { @@ -69,16 +67,6 @@ function lower(func: HIRFunction): DisjointSet { return aliases; } -export function debugAliases(aliases: DisjointSet): void { - console.log( - prettyFormat( - aliases - .buildSets() - .map(set => [...set].map(ident => printIdentifier(ident))), - ), - ); -} - /** * The alias sets returned by InferMutableRanges() accounts only for aliases that * are known to mutate together. Notably this skips cases where a value is captured diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts index 27230b9e7f..3b022c13f7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +import prettyFormat from 'pretty-format'; import {HIRFunction, Identifier} from '../HIR/HIR'; import DisjointSet from '../Utils/DisjointSet'; import {inferAliasForUncalledFunctions} from './InerAliasForUncalledFunctions'; @@ -14,7 +15,9 @@ import {inferAliasForPhis} from './InferAliasForPhis'; import {inferAliasForStores} from './InferAliasForStores'; import {inferMutableLifetimes} from './InferMutableLifetimes'; import {inferMutableRangesForAlias} from './InferMutableRangesForAlias'; +import {inferMutableRangesForComutation} from './InferMutableRangesForComutation'; import {inferTryCatchAliases} from './InferTryCatchAliases'; +import {printIdentifier, printMutableRange} from '../HIR/PrintHIR'; export function inferMutableRanges(ir: HIRFunction): DisjointSet { // Infer mutable ranges for non fields @@ -32,11 +35,13 @@ export function inferMutableRanges(ir: HIRFunction): DisjointSet { * Eagerly canonicalize so that if nothing changes we can bail out * after a single iteration */ - let prevAliases: Map = aliases.canonicalize(); + let prevAliases: Map = canonicalize(aliases); while (true) { // Infer mutable ranges for aliases that are not fields inferMutableRangesForAlias(ir, aliases); + inferMutableRangesForComutation(ir); + // Update aliasing information of fields inferAliasForStores(ir, aliases); @@ -45,7 +50,7 @@ export function inferMutableRanges(ir: HIRFunction): DisjointSet { // Update aliasing information of phis inferAliasForPhis(ir, aliases); - const nextAliases = aliases.canonicalize(); + const nextAliases = canonicalize(aliases); if (areEqualMaps(prevAliases, nextAliases)) { break; } @@ -77,12 +82,13 @@ export function inferMutableRanges(ir: HIRFunction): DisjointSet { * but does not modify values that `y` "contains" such as the * object literal or `z`. */ - prevAliases = aliases.canonicalize(); + prevAliases = canonicalize(aliases); while (true) { inferMutableRangesForAlias(ir, aliases); + inferMutableRangesForComutation(ir); inferAliasForPhis(ir, aliases); inferAliasForUncalledFunctions(ir, aliases); - const nextAliases = aliases.canonicalize(); + const nextAliases = canonicalize(aliases); if (areEqualMaps(prevAliases, nextAliases)) { break; } @@ -92,7 +98,41 @@ export function inferMutableRanges(ir: HIRFunction): DisjointSet { return aliases; } -function areEqualMaps(a: Map, b: Map): boolean { +export function debugAliases(aliases: DisjointSet): void { + console.log( + prettyFormat( + aliases + .buildSets() + .map(set => + [...set].map( + ident => printIdentifier(ident) + printMutableRange(ident), + ), + ), + ), + ); +} + +/** + * Canonicalizes the alias set and mutable range information calculated at the current time. + * The returned value maps each identifier in the program to the root identifier of its alias + * set and the the mutable range of that set. + * + * This ensures that we fixpoint the mutable ranges themselves and not just the alias sets. + */ +function canonicalize( + aliases: DisjointSet, +): Map { + const entries = new Map(); + aliases.forEach((item, root) => { + entries.set( + item, + `${root.id}:${root.mutableRange.start}:${root.mutableRange.end}`, + ); + }); + return entries; +} + +function areEqualMaps(a: Map, b: Map): boolean { if (a.size !== b.size) { return false; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts new file mode 100644 index 0000000000..09e3476714 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRangesForComutation.ts @@ -0,0 +1,91 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { + HIRFunction, + Identifier, + isRefOrRefValue, + makeInstructionId, +} from '../HIR'; +import {eachInstructionOperand} from '../HIR/visitors'; +import {isMutable} from '../ReactiveScopes/InferReactiveScopeVariables'; + +/** + * Finds instructions with operands that co-mutate and extends all their mutable ranges + * to end at the same point (the highest `end` value of the group). Note that the + * alias sets used in InferMutableRanges are meant for values that strictly alias: + * a mutation of one value in the set would directly modify the same object as some + * other value in the set. + * + * However, co-mutation can cause an alias to one object to be stored within another object, + * for example: + * + * ``` + * const a = {}; + * const b = {}; + * const f = () => b.c; // + * setProperty(a, 'b', b); // equiv to a.b = b + * + * a.b.c = 'c'; // this mutates b! + * ``` + * + * Here, the co-mutation in `setProperty(a, 'b', b)` means that a reference to b may be stored + * in a, vice-versa, or both. We need to extend the mutable range of both a and b to reflect + * the fact the values may mutate together. + * + * Previously this was implemented in InferReactiveScopeVariables, but that is too late: + * we need this to be part of the InferMutableRanges fixpoint iteration to account for functions + * like `f` in the example, which capture a reference to a value that may change later. `f` + * cannot be independently memoized from the `setProperty()` call due to the co-mutation. + * + * See aliased-capture-mutate and aliased-capture-aliased-mutate fixtures for examples. + */ +export function inferMutableRangesForComutation(fn: HIRFunction): void { + for (const block of fn.body.blocks.values()) { + for (const instr of block.instructions) { + let operands: Array | null = null; + for (const operand of eachInstructionOperand(instr)) { + if ( + isMutable(instr, operand) && + operand.identifier.mutableRange.start > 0 + ) { + if ( + instr.value.kind === 'FunctionExpression' || + instr.value.kind === 'ObjectMethod' + ) { + if (operand.identifier.type.kind === 'Primitive') { + continue; + } + } + operands ??= []; + operands.push(operand.identifier); + } + } + if (operands != null) { + // Find the last instruction which mutates any of the mutable operands + let lastMutatingInstructionId = makeInstructionId(0); + for (const id of operands) { + if (id.mutableRange.end > lastMutatingInstructionId) { + lastMutatingInstructionId = id.mutableRange.end; + } + } + + /** + * Update all mutable operands's mutable ranges to end at the same point + */ + for (const id of operands) { + if ( + id.mutableRange.end < lastMutatingInstructionId && + !isRefOrRefValue(id) + ) { + id.mutableRange.end = lastMutatingInstructionId; + } + } + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md new file mode 100644 index 0000000000..5b9900a120 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +// @flow @enableTransitivelyFreezeFunctionExpressions:false +import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; + +function useFoo({a, b}: {a: number, b: number}) { + const x = []; + const y = {value: a}; + + arrayPush(x, y); // x and y co-mutate + const y_alias = y; + const cb = () => y_alias.value; + setPropertyByKey(x[0], 'value', b); // might overwrite y.value + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 10}], + sequentialRenders: [ + {a: 2, b: 10}, + {a: 2, b: 11}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(3); + const { a, b } = t0; + let t1; + if ($[0] !== a || $[1] !== b) { + const x = []; + const y = { value: a }; + + arrayPush(x, y); + const y_alias = y; + const cb = () => y_alias.value; + setPropertyByKey(x[0], "value", b); + t1 = ; + $[0] = a; + $[1] = b; + $[2] = t1; + } else { + t1 = $[2]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ a: 2, b: 10 }], + sequentialRenders: [ + { a: 2, b: 10 }, + { a: 2, b: 11 }, + ], +}; + +``` + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
+
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js new file mode 100644 index 0000000000..d1f92144d4 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-aliased-mutate.js @@ -0,0 +1,22 @@ +// @flow @enableTransitivelyFreezeFunctionExpressions:false +import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; + +function useFoo({a, b}: {a: number, b: number}) { + const x = []; + const y = {value: a}; + + arrayPush(x, y); // x and y co-mutate + const y_alias = y; + const cb = () => y_alias.value; + setPropertyByKey(x[0], 'value', b); // might overwrite y.value + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2, b: 10}], + sequentialRenders: [ + {a: 2, b: 10}, + {a: 2, b: 11}, + ], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.expect.md similarity index 62% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.expect.md index c35efe6a16..0bd15a268e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.expect.md @@ -5,19 +5,6 @@ // @flow @enableTransitivelyFreezeFunctionExpressions:false import {setPropertyByKey, Stringify} from 'shared-runtime'; -/** - * Variation of bug in `bug-aliased-capture-aliased-mutate` - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- */ - function useFoo({a}: {a: number, b: number}) { const arr = []; const obj = {value: a}; @@ -46,7 +33,7 @@ import { c as _c } from "react/compiler-runtime"; import { setPropertyByKey, Stringify } from "shared-runtime"; function useFoo(t0) { - const $ = _c(4); + const $ = _c(2); const { a } = t0; let t1; if ($[0] !== a) { @@ -55,15 +42,7 @@ function useFoo(t0) { setPropertyByKey(obj, "arr", arr); const obj_alias = obj; - let t2; - if ($[2] !== obj_alias.arr.length) { - t2 = () => obj_alias.arr.length; - $[2] = obj_alias.arr.length; - $[3] = t2; - } else { - t2 = $[3]; - } - const cb = t2; + const cb = () => obj_alias.arr.length; for (let i = 0; i < a; i++) { arr.push(i); } @@ -84,4 +63,7 @@ export const FIXTURE_ENTRYPOINT = { }; ``` - \ No newline at end of file + +### Eval output +(kind: ok)
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
+
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.js similarity index 52% rename from compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js rename to compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.js index a7e5767266..be12a7c9be 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/aliased-capture-mutate.js @@ -1,19 +1,6 @@ // @flow @enableTransitivelyFreezeFunctionExpressions:false import {setPropertyByKey, Stringify} from 'shared-runtime'; -/** - * Variation of bug in `bug-aliased-capture-aliased-mutate` - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":3},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":2},"shouldInvokeFns":true}
- */ - function useFoo({a}: {a: number, b: number}) { const arr = []; const obj = {value: a}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md deleted file mode 100644 index d0ad9e2f9d..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md +++ /dev/null @@ -1,107 +0,0 @@ - -## Input - -```javascript -// @flow @enableTransitivelyFreezeFunctionExpressions:false -import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * 1. `InferMutableRanges` derives the mutable range of identifiers and their - * aliases from `LoadLocal`, `PropertyLoad`, etc - * - After this pass, y's mutable range only extends to `arrayPush(x, y)` - * - We avoid assigning mutable ranges to loads after y's mutable range, as - * these are working with an immutable value. As a result, `LoadLocal y` and - * `PropertyLoad y` do not get mutable ranges - * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes, - * as according to the 'co-mutation' of different values - * - Here, we infer that - * - `arrayPush(y, x)` might alias `x` and `y` to each other - * - `setPropertyKey(x, ...)` may mutate both `x` and `y` - * - This pass correctly extends the mutable range of `y` - * - Since we didn't run `InferMutableRange` logic again, the LoadLocal / - * PropertyLoads still don't have a mutable range - * - * Note that the this bug is an edge case. Compiler output is only invalid for: - * - function expressions with - * `enableTransitivelyFreezeFunctionExpressions:false` - * - functions that throw and get retried without clearing the memocache - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- */ -function useFoo({a, b}: {a: number, b: number}) { - const x = []; - const y = {value: a}; - - arrayPush(x, y); // x and y co-mutate - const y_alias = y; - const cb = () => y_alias.value; - setPropertyByKey(x[0], 'value', b); // might overwrite y.value - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2, b: 10}], - sequentialRenders: [ - {a: 2, b: 10}, - {a: 2, b: 11}, - ], -}; - -``` - -## Code - -```javascript -import { c as _c } from "react/compiler-runtime"; -import { arrayPush, setPropertyByKey, Stringify } from "shared-runtime"; - -function useFoo(t0) { - const $ = _c(5); - const { a, b } = t0; - let t1; - if ($[0] !== a || $[1] !== b) { - const x = []; - const y = { value: a }; - - arrayPush(x, y); - const y_alias = y; - let t2; - if ($[3] !== y_alias.value) { - t2 = () => y_alias.value; - $[3] = y_alias.value; - $[4] = t2; - } else { - t2 = $[4]; - } - const cb = t2; - setPropertyByKey(x[0], "value", b); - t1 = ; - $[0] = a; - $[1] = b; - $[2] = t1; - } else { - t1 = $[2]; - } - return t1; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{ a: 2, b: 10 }], - sequentialRenders: [ - { a: 2, b: 10 }, - { a: 2, b: 11 }, - ], -}; - -``` - \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js deleted file mode 100644 index c46ecd6250..0000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js +++ /dev/null @@ -1,53 +0,0 @@ -// @flow @enableTransitivelyFreezeFunctionExpressions:false -import {arrayPush, setPropertyByKey, Stringify} from 'shared-runtime'; - -/** - * 1. `InferMutableRanges` derives the mutable range of identifiers and their - * aliases from `LoadLocal`, `PropertyLoad`, etc - * - After this pass, y's mutable range only extends to `arrayPush(x, y)` - * - We avoid assigning mutable ranges to loads after y's mutable range, as - * these are working with an immutable value. As a result, `LoadLocal y` and - * `PropertyLoad y` do not get mutable ranges - * 2. `InferReactiveScopeVariables` extends mutable ranges and creates scopes, - * as according to the 'co-mutation' of different values - * - Here, we infer that - * - `arrayPush(y, x)` might alias `x` and `y` to each other - * - `setPropertyKey(x, ...)` may mutate both `x` and `y` - * - This pass correctly extends the mutable range of `y` - * - Since we didn't run `InferMutableRange` logic again, the LoadLocal / - * PropertyLoads still don't have a mutable range - * - * Note that the this bug is an edge case. Compiler output is only invalid for: - * - function expressions with - * `enableTransitivelyFreezeFunctionExpressions:false` - * - functions that throw and get retried without clearing the memocache - * - * Found differences in evaluator results - * Non-forget (expected): - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":11},"shouldInvokeFns":true}
- * Forget: - * (kind: ok) - *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- *
{"cb":{"kind":"Function","result":10},"shouldInvokeFns":true}
- */ -function useFoo({a, b}: {a: number, b: number}) { - const x = []; - const y = {value: a}; - - arrayPush(x, y); // x and y co-mutate - const y_alias = y; - const cb = () => y_alias.value; - setPropertyByKey(x[0], 'value', b); // might overwrite y.value - return ; -} - -export const FIXTURE_ENTRYPOINT = { - fn: useFoo, - params: [{a: 2, b: 10}], - sequentialRenders: [ - {a: 2, b: 10}, - {a: 2, b: 11}, - ], -}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index 62b8a7703f..217b4a9689 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -453,8 +453,6 @@ const skipFilter = new Set([ 'inner-function/nullable-objects/bug-invalid-array-map-manual', 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', `bug-capturing-func-maybealias-captured-mutate`, - 'bug-aliased-capture-aliased-mutate', - 'bug-aliased-capture-mutate', 'bug-functiondecl-hoisting', 'bug-type-inference-control-flow', 'fbt/bug-fbt-plural-multiple-function-calls',