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 new file mode 100644 index 0000000000000..d0ad9e2f9dbe5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.expect.md @@ -0,0 +1,107 @@ + +## 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 new file mode 100644 index 0000000000000..c46ecd6250b42 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-aliased-mutate.js @@ -0,0 +1,53 @@ +// @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/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/bug-aliased-capture-mutate.expect.md new file mode 100644 index 0000000000000..c35efe6a16bb5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.expect.md @@ -0,0 +1,87 @@ + +## Input + +```javascript +// @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}; + + setPropertyByKey(obj, 'arr', arr); + const obj_alias = obj; + const cb = () => obj_alias.arr.length; + for (let i = 0; i < a; i++) { + arr.push(i); + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2}], + sequentialRenders: [{a: 2}, {a: 3}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { setPropertyByKey, Stringify } from "shared-runtime"; + +function useFoo(t0) { + const $ = _c(4); + const { a } = t0; + let t1; + if ($[0] !== a) { + const arr = []; + const obj = { value: a }; + + 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; + for (let i = 0; i < a; i++) { + arr.push(i); + } + + t1 = ; + $[0] = a; + $[1] = t1; + } else { + t1 = $[1]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{ a: 2 }], + sequentialRenders: [{ a: 2 }, { a: 3 }], +}; + +``` + \ 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/bug-aliased-capture-mutate.js new file mode 100644 index 0000000000000..a7e57672665f6 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-aliased-capture-mutate.js @@ -0,0 +1,34 @@ +// @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}; + + setPropertyByKey(obj, 'arr', arr); + const obj_alias = obj; + const cb = () => obj_alias.arr.length; + for (let i = 0; i < a; i++) { + arr.push(i); + } + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: useFoo, + params: [{a: 2}], + sequentialRenders: [{a: 2}, {a: 3}], +}; diff --git a/compiler/packages/snap/src/SproutTodoFilter.ts b/compiler/packages/snap/src/SproutTodoFilter.ts index b868afc52b82d..1971fe0d33161 100644 --- a/compiler/packages/snap/src/SproutTodoFilter.ts +++ b/compiler/packages/snap/src/SproutTodoFilter.ts @@ -480,6 +480,8 @@ const skipFilter = new Set([ 'fbt/bug-fbt-plural-multiple-mixed-call-tag', 'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr', 'bug-invalid-hoisting-functionexpr', + 'bug-aliased-capture-aliased-mutate', + 'bug-aliased-capture-mutate', 'bug-functiondecl-hoisting', 'bug-try-catch-maybe-null-dependency', 'reduce-reactive-deps/bug-infer-function-cond-access-not-hoisted',