diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts index 7c05c05531ae9..f396ae18b0461 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -549,8 +549,16 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [ [ 'map', addFunction(BUILTIN_SHAPES, [], { + /** + * Note `map`'s arguments are annotated as Effect.ConditionallyMutate as + * calling `.map(fn)` might invoke `fn`, which means replaying its + * effects. + * + * (Note that Effect.Read / Effect.Capture on a function type means + * potential data dependency or aliasing respectively.) + */ positionalParams: [], - restParam: Effect.Read, + restParam: Effect.ConditionallyMutate, returnType: {kind: 'Object', shapeId: BuiltInArrayId}, calleeEffect: Effect.ConditionallyMutate, returnValueKind: ValueKind.Mutable, diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mixedreadonly-mutating-map.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mixedreadonly-mutating-map.expect.md new file mode 100644 index 0000000000000..4867388a864d2 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mixedreadonly-mutating-map.expect.md @@ -0,0 +1,156 @@ + +## Input + +```javascript +import { + arrayPush, + identity, + makeArray, + Stringify, + useFragment, +} from 'shared-runtime'; + +/** + * Bug repro showing why it's invalid for function references to be annotated + * with a `Read` effect when that reference might lead to the function being + * invoked. + * + * Note that currently, `Array.map` is annotated to have `Read` effects on its + * operands. This is incorrect as function effects must be replayed when `map` + * is called + * - Read: non-aliasing data dependency + * - Capture: maybe-aliasing data dependency + * - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke + * but only if the value is mutable + * + * Invalid evaluator result: Found differences in evaluator results Non-forget + * (expected): (kind: ok) + *
{"x":[2,2,2],"count":3}
{"item":1}
+ *
{"x":[2,2,2],"count":4}
{"item":1}
+ * Forget: + * (kind: ok) + *
{"x":[2,2,2],"count":3}
{"item":1}
+ *
{"x":[2,2,2,2,2,2],"count":4}
{"item":1}
+ */ + +function Component({extraJsx}) { + const x = makeArray(); + const items = useFragment(); + // This closure has the following effects that must be replayed: + // - MaybeFreeze / Capture of `items` + // - ConditionalMutate of x + const jsx = items.a.map((item, i) => { + arrayPush(x, 2); + return ; + }); + const offset = jsx.length; + for (let i = 0; i < extraJsx; i++) { + jsx.push(); + } + const count = jsx.length; + identity(count); + return ( + <> + + {jsx[0]} + + ); +} +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{extraJsx: 0}], + sequentialRenders: [{extraJsx: 0}, {extraJsx: 1}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { + arrayPush, + identity, + makeArray, + Stringify, + useFragment, +} from "shared-runtime"; + +/** + * Bug repro showing why it's invalid for function references to be annotated + * with a `Read` effect when that reference might lead to the function being + * invoked. + * + * Note that currently, `Array.map` is annotated to have `Read` effects on its + * operands. This is incorrect as function effects must be replayed when `map` + * is called + * - Read: non-aliasing data dependency + * - Capture: maybe-aliasing data dependency + * - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke + * but only if the value is mutable + * + * Invalid evaluator result: Found differences in evaluator results Non-forget + * (expected): (kind: ok) + *
{"x":[2,2,2],"count":3}
{"item":1}
+ *
{"x":[2,2,2],"count":4}
{"item":1}
+ * Forget: + * (kind: ok) + *
{"x":[2,2,2],"count":3}
{"item":1}
+ *
{"x":[2,2,2,2,2,2],"count":4}
{"item":1}
+ */ + +function Component(t0) { + const $ = _c(6); + const { extraJsx } = t0; + const x = makeArray(); + const items = useFragment(); + + const jsx = items.a.map((item, i) => { + arrayPush(x, 2); + return ; + }); + const offset = jsx.length; + for (let i_0 = 0; i_0 < extraJsx; i_0++) { + jsx.push(); + } + + const count = jsx.length; + identity(count); + let t1; + if ($[0] !== count || $[1] !== x) { + t1 = ; + $[0] = count; + $[1] = x; + $[2] = t1; + } else { + t1 = $[2]; + } + const t2 = jsx[0]; + let t3; + if ($[3] !== t1 || $[4] !== t2) { + t3 = ( + <> + {t1} + {t2} + + ); + $[3] = t1; + $[4] = t2; + $[5] = t3; + } else { + t3 = $[5]; + } + return t3; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ extraJsx: 0 }], + sequentialRenders: [{ extraJsx: 0 }, { extraJsx: 1 }], +}; + +``` + +### Eval output +(kind: ok)
{"x":[2,2,2],"count":3}
{"item":1}
+
{"x":[2,2,2],"count":4}
{"item":1}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mixedreadonly-mutating-map.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mixedreadonly-mutating-map.js new file mode 100644 index 0000000000000..858a4ab3dc693 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/mixedreadonly-mutating-map.js @@ -0,0 +1,59 @@ +import { + arrayPush, + identity, + makeArray, + Stringify, + useFragment, +} from 'shared-runtime'; + +/** + * Bug repro showing why it's invalid for function references to be annotated + * with a `Read` effect when that reference might lead to the function being + * invoked. + * + * Note that currently, `Array.map` is annotated to have `Read` effects on its + * operands. This is incorrect as function effects must be replayed when `map` + * is called + * - Read: non-aliasing data dependency + * - Capture: maybe-aliasing data dependency + * - ConditionallyMutate: maybe-aliasing data dependency; maybe-write / invoke + * but only if the value is mutable + * + * Invalid evaluator result: Found differences in evaluator results Non-forget + * (expected): (kind: ok) + *
{"x":[2,2,2],"count":3}
{"item":1}
+ *
{"x":[2,2,2],"count":4}
{"item":1}
+ * Forget: + * (kind: ok) + *
{"x":[2,2,2],"count":3}
{"item":1}
+ *
{"x":[2,2,2,2,2,2],"count":4}
{"item":1}
+ */ + +function Component({extraJsx}) { + const x = makeArray(); + const items = useFragment(); + // This closure has the following effects that must be replayed: + // - MaybeFreeze / Capture of `items` + // - ConditionalMutate of x + const jsx = items.a.map((item, i) => { + arrayPush(x, 2); + return ; + }); + const offset = jsx.length; + for (let i = 0; i < extraJsx; i++) { + jsx.push(); + } + const count = jsx.length; + identity(count); + return ( + <> + + {jsx[0]} + + ); +} +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{extraJsx: 0}], + sequentialRenders: [{extraJsx: 0}, {extraJsx: 1}], +};