diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index 8d2e72b22e4e9..5f063ad2f9995 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -246,7 +246,7 @@ export const EnvironmentConfigSchema = z.object({ /** * Enable a new model for mutability and aliasing inference */ - enableNewMutationAliasingModel: z.boolean().default(false), + enableNewMutationAliasingModel: z.boolean().default(true), /** * Enables inference of optional dependency chains. Without this flag 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 f853d5b815b43..442d9bcc9d7d3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/ObjectShape.ts @@ -192,6 +192,7 @@ export type FunctionSignature = { canonicalName?: string; aliasing?: AliasingSignature | null; + todo_aliasing?: AliasingSignature | null; }; /* @@ -320,6 +321,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ params: [], rest: makeIdentifierId(1), returns: makeIdentifierId(2), + temporaries: [], effects: [ // Push directly mutates the array itself {kind: 'Mutate', value: signatureArgument(0)}, @@ -367,7 +369,59 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [ returnValueKind: ValueKind.Mutable, noAlias: true, mutableOnlyIfOperandsAreMutable: true, - aliasing: null, + aliasing: { + receiver: makeIdentifierId(0), + params: [makeIdentifierId(1)], + rest: null, + returns: makeIdentifierId(2), + temporaries: [ + // Temporary representing captured items of the receiver + signatureArgument(3), + // Temporary representing the result of the callback + signatureArgument(4), + /* + * Undefined `this` arg to the callback. Note the signature does not + * support passing an explicit thisArg second param + */ + signatureArgument(5), + ], + effects: [ + // Map creates a new mutable array + { + kind: 'Create', + into: signatureArgument(2), + value: ValueKind.Mutable, + }, + // The first arg to the callback is an item extracted from the receiver array + { + kind: 'CreateFrom', + from: signatureArgument(0), + into: signatureArgument(3), + }, + // The undefined this for the callback + { + kind: 'Create', + into: signatureArgument(5), + value: ValueKind.Primitive, + }, + // calls the callback, returning the result into a temporary + { + kind: 'Apply', + receiver: signatureArgument(5), + args: [signatureArgument(3), {kind: 'Hole'}, signatureArgument(0)], + function: signatureArgument(1), + into: signatureArgument(4), + signature: null, + mutatesFunction: false, + }, + // captures the result of the callback into the return array + { + kind: 'Capture', + from: signatureArgument(4), + into: signatureArgument(2), + }, + ], + }, }), ], [ 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 85488a6ed2cb6..fc7190e1ca87b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -972,6 +972,8 @@ export function printAliasingEffect(effect: AliasingEffect): string { .map(arg => { if (arg.kind === 'Identifier') { return printPlaceForAliasEffect(arg); + } else if (arg.kind === 'Hole') { + return ' '; } return `...${printPlaceForAliasEffect(arg.place)}`; }) @@ -986,6 +988,9 @@ export function printAliasingEffect(effect: AliasingEffect): string { } return `Apply ${printPlaceForAliasEffect(effect.into)} = ${receiverCallee}(${args})${signature != '' ? '\n ' : ''}${signature}`; } + case 'CreateFunction': { + return `Function ${printPlaceForAliasEffect(effect.into)} = Function captures=[${effect.captures.map(printPlaceForAliasEffect).join(', ')}]`; + } case 'Freeze': { return `Freeze ${printPlaceForAliasEffect(effect.value)} ${effect.reason}`; } @@ -1007,6 +1012,13 @@ function printPlaceForAliasEffect(place: Place): string { export function printAliasingSignature(signature: AliasingSignature): string { const tokens: Array = ['function ']; + if (signature.temporaries.length !== 0) { + tokens.push('<'); + tokens.push( + signature.temporaries.map(temp => `$${temp.identifier.id}`).join(', '), + ); + tokens.push('>'); + } tokens.push('('); tokens.push('this=$' + String(signature.receiver)); for (const param of signature.params) { 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 3f6bbd1a18468..0b40386e235bb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -96,6 +96,7 @@ function lowerWithMutationAliasing(fn: HIRFunction): void { capturedOrMutated.add(effect.value.identifier.id); break; } + case 'CreateFunction': case 'Create': case 'Freeze': case 'ImmutableCapture': { 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 bb246671cb927..dfa69574916aa 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -10,13 +10,17 @@ import { BasicBlock, BlockId, Environment, + FunctionExpression, HIRFunction, + Hole, IdentifierId, Instruction, InstructionValue, isArrayType, isMapType, isSetType, + makeIdentifierId, + ObjectMethod, Phi, Place, SpreadPattern, @@ -40,6 +44,7 @@ import { } from '../Utils/utils'; import { printAliasingEffect, + printAliasingSignature, printIdentifier, printInstruction, printInstructionValue, @@ -49,6 +54,7 @@ import { import {FunctionSignature} from '../HIR/ObjectShape'; import {getWriteErrorReason} from './InferFunctionEffects'; import prettyFormat from 'pretty-format'; +import {createTemporaryPlace} from '../HIR/HIRBuilder'; export function inferMutationAliasingEffects( fn: HIRFunction, @@ -399,7 +405,7 @@ function applyEffect( default: { // TODO: should this be Alias?? effects.push({ - kind: 'Capture', + kind: 'Alias', from: effect.from, into: effect.into, }); @@ -407,6 +413,39 @@ function applyEffect( } break; } + case 'CreateFunction': { + const isMutable = effect.captures.some(capture => { + switch (state.kind(capture).kind) { + case ValueKind.Context: + case ValueKind.Mutable: { + return true; + } + default: { + return false; + } + } + }); + state.initialize(effect.function, { + kind: isMutable ? ValueKind.Mutable : ValueKind.Frozen, + reason: new Set([]), + }); + state.define(effect.into, effect.function); + for (const capture of effect.captures) { + applyEffect( + state, + { + kind: 'Capture', + from: capture, + into: effect.into, + }, + instruction, + effectInstructionValueCache, + aliased, + effects, + ); + } + break; + } case 'Capture': { /* * Capture describes potential information flow: storing a pointer to one value @@ -509,9 +548,56 @@ function applyEffect( break; } case 'Apply': { + const functionValues = state.values(effect.function); + console.log('function is ' + printPlace(effect.function)); + console.log( + functionValues.length + + ' ' + + functionValues.map(v => v.kind).join(', '), + ); + if ( + functionValues.length === 1 && + functionValues[0].kind === 'FunctionExpression' + ) { + /* + * We're calling a locally declared function, we already know it's effects! + * We just have to substitute in the args for the params + */ + const signature = buildSignatureFromFunctionExpression( + state.env, + functionValues[0], + ); + console.log( + `constructed alias signature:\n${printAliasingSignature(signature)}`, + ); + const signatureEffects = computeEffectsForSignature( + state.env, + signature, + effect.into, + effect.receiver, + effect.args, + ); + if (signatureEffects != null) { + if (DEBUG) { + console.log('apply function expression effects'); + } + for (const signatureEffect of signatureEffects) { + applyEffect( + state, + signatureEffect, + instruction, + effectInstructionValueCache, + aliased, + effects, + ); + } + break; + } + } const signatureEffects = effect.signature?.aliasing != null ? computeEffectsForSignature( + state.env, effect.signature.aliasing, effect.into, effect.receiver, @@ -577,6 +663,9 @@ function applyEffect( * every other argument. */ for (const arg of [effect.receiver, effect.function, ...effect.args]) { + if (arg.kind === 'Hole') { + continue; + } const operand = arg.kind === 'Identifier' ? arg : arg.place; if (operand !== effect.function || effect.mutatesFunction) { applyEffect( @@ -602,7 +691,7 @@ function applyEffect( */ applyEffect( state, - {kind: 'Capture', from: operand, into: effect.into}, + {kind: 'Alias', from: operand, into: effect.into}, instruction, effectInstructionValueCache, aliased, @@ -613,6 +702,9 @@ function applyEffect( effect.function, ...effect.args, ]) { + if (otherArg.kind === 'Hole') { + continue; + } const other = otherArg.kind === 'Identifier' ? otherArg : otherArg.place; if (other === arg) { @@ -679,7 +771,7 @@ function applyEffect( } } -const DEBUG = false; +const DEBUG = true; class InferenceState { env: Environment; @@ -1235,24 +1327,6 @@ function computeSignatureForInstruction( } case 'ObjectMethod': case 'FunctionExpression': { - /* - * We consider the function frozen if it has no potential mutations of its context - * variables. - * TODO: this may lose some precision relative to InferReferenceEffects, where we - * decide the valuekind of the function based on the actual state of the context - * vars. Maybe a `CreateFunction captures=[...] into=place` effect or similar to - * express the logic - */ - const kind = value.loweredFunc.func.context.some( - operand => operand.effect === Effect.Capture, - ) - ? ValueKind.Mutable - : ValueKind.Frozen; - effects.push({ - kind: 'Create', - into: lvalue, - value: kind, - }); /** * We've already analyzed the function expression in AnalyzeFunctions. There, we assign * a Capture effect to any context variable that appears (locally) to be aliased and/or @@ -1291,15 +1365,14 @@ function computeSignatureForInstruction( * Note that if the type of the context variables are frozen, global, or primitive, the * Capture will either get pruned or downgraded to an ImmutableCapture. */ - for (const operand of value.loweredFunc.func.context) { - if (operand.effect === Effect.Capture) { - effects.push({ - kind: 'Capture', - from: operand, - into: lvalue, - }); - } - } + effects.push({ + kind: 'CreateFunction', + into: lvalue, + function: value, + captures: value.loweredFunc.func.context.filter( + operand => operand.effect === Effect.Capture, + ), + }); break; } case 'GetIterator': { @@ -1493,7 +1566,7 @@ function computeEffectsForLegacySignature( signature: FunctionSignature, lvalue: Place, receiver: Place, - args: Array, + args: Array, ): Array { const effects: Array = []; effects.push({ @@ -1566,11 +1639,14 @@ function computeEffectsForLegacySignature( areArgumentsImmutableAndNonMutating(state, args) ) { effects.push({ - kind: 'Capture', + kind: 'Alias', from: receiver, into: lvalue, }); for (const arg of args) { + if (arg.kind === 'Hole') { + continue; + } const place = arg.kind === 'Identifier' ? arg : arg.place; effects.push({ kind: 'ImmutableCapture', @@ -1591,7 +1667,7 @@ function computeEffectsForLegacySignature( * the type doesn't actually need to be captured based on the input and return type. */ effects.push({ - kind: 'Capture', + kind: 'Alias', from: receiver, into: lvalue, }); @@ -1599,6 +1675,9 @@ function computeEffectsForLegacySignature( visit(receiver, signature.calleeEffect); for (let i = 0; i < args.length; i++) { const arg = args[i]; + if (arg.kind === 'Hole') { + continue; + } const place = arg.kind === 'Identifier' ? arg : arg.place; const effect = arg.kind === 'Identifier' && i < signature.positionalParams.length @@ -1610,7 +1689,7 @@ function computeEffectsForLegacySignature( if (stores.length === 0) { // If no stores, then capture into the return value for (const capture of captures) { - effects.push({kind: 'Capture', from: capture, into: lvalue}); + effects.push({kind: 'Alias', from: capture, into: lvalue}); } } else { // Else capture into the stores @@ -1632,9 +1711,12 @@ function computeEffectsForLegacySignature( */ function areArgumentsImmutableAndNonMutating( state: InferenceState, - args: Array, + args: Array, ): boolean { for (const arg of args) { + if (arg.kind === 'Hole') { + continue; + } if (arg.kind === 'Identifier' && arg.identifier.type.kind === 'Function') { const fnShape = state.env.getFunctionSignature(arg.identifier.type); if (fnShape != null) { @@ -1685,10 +1767,11 @@ function areArgumentsImmutableAndNonMutating( } function computeEffectsForSignature( + env: Environment, signature: AliasingSignature, lvalue: Place, receiver: Place, - args: Array, + args: Array, ): Array | null { if ( // Not enough args @@ -1696,6 +1779,17 @@ 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 @@ -1705,8 +1799,13 @@ function computeEffectsForSignature( const params = signature.params; for (let i = 0; i < args.length; i++) { const arg = args[i]; - if (params == null || i >= params.length || arg.kind === 'Spread') { + if (arg.kind === 'Hole') { + 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; @@ -1717,8 +1816,13 @@ function computeEffectsForSignature( } } - // Apply substitutions const effects: Array = []; + for (const signatureTemporary of signature.temporaries) { + const temp = createTemporaryPlace(env, receiver.loc); + substitutions.set(signatureTemporary.identifier.id, [temp]); + } + + // Apply substitutions for (const effect of signature.effects) { switch (effect.kind) { case 'ImmutableCapture': @@ -1763,9 +1867,66 @@ function computeEffectsForSignature( break; } 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 = []; + for (const arg of effect.args) { + if (arg.kind === 'Hole') { + applyArgs.push(arg); + } 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]}); + } + } + effects.push({ + kind: 'Apply', + mutatesFunction: effect.mutatesFunction, + receiver: applyReceiver[0], + args: applyArgs, + function: applyFunction[0], + into: applyInto[0], + signature: effect.signature, + }); + break; + } + case 'CreateFunction': { CompilerError.throwTodo({ - reason: `Handle ${effect.kind} effects for function declarations`, - loc: lvalue.loc, + reason: `Support CreateFrom effects in signatures`, + loc: receiver.loc, }); } default: { @@ -1779,6 +1940,29 @@ function computeEffectsForSignature( return effects; } +function buildSignatureFromFunctionExpression( + env: Environment, + fn: FunctionExpression, +): AliasingSignature { + let rest: IdentifierId | null = null; + const params: Array = []; + for (const param of fn.loweredFunc.func.params) { + if (param.kind === 'Identifier') { + params.push(param.identifier.id); + } else { + rest = param.place.identifier.id; + } + } + return { + receiver: makeIdentifierId(0), + params, + rest: rest ?? createTemporaryPlace(env, fn.loc).identifier.id, + returns: fn.loweredFunc.func.returns.identifier.id, + effects: fn.loweredFunc.func.aliasingEffects ?? [], + temporaries: [], + }; +} + /* * array.map(cb) * t3 = t0 .t1 ( t2 ) @@ -1917,17 +2101,30 @@ export type AliasingEffect = receiver: Place; function: Place; mutatesFunction: boolean; - args: Array; + args: Array; into: Place; signature: FunctionSignature | null; + } + /** + * Constructs a function value with the given captures. The mutability of the function + * will be determined by the mutability of the capture values when evaluated. + */ + | { + kind: 'CreateFunction'; + captures: Array; + function: FunctionExpression | ObjectMethod; + into: Place; }; +export type AliasingSignatureEffect = AliasingEffect; + export type AliasingSignature = { receiver: IdentifierId; params: Array; rest: IdentifierId | null; returns: IdentifierId; - effects: Array; + effects: Array; + temporaries: Array; }; export type AbstractValue = { 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 f891dd6d2953d..86aaf7bda7bdb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -174,6 +174,7 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { operandEffects.set(effect.from.identifier.id, Effect.Read); break; } + case 'CreateFunction': case 'Create': { break; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-captures-receiver-noAlias.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-captures-receiver-noAlias.expect.md new file mode 100644 index 0000000000000..e687c995d077f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-captures-receiver-noAlias.expect.md @@ -0,0 +1,71 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +function Component(props) { + // This item is part of the receiver, should be memoized + const item = {a: props.a}; + const items = [item]; + const mapped = items.map(item => item); + // mapped[0].a = null; + return mapped; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {id: 42}}], + isComponent: false, +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +function Component(props) { + const $ = _c(6); + let t0; + if ($[0] !== props.a) { + t0 = { a: props.a }; + $[0] = props.a; + $[1] = t0; + } else { + t0 = $[1]; + } + const item = t0; + let t1; + if ($[2] !== item) { + t1 = [item]; + $[2] = item; + $[3] = t1; + } else { + t1 = $[3]; + } + const items = t1; + let t2; + if ($[4] !== items) { + t2 = items.map(_temp); + $[4] = items; + $[5] = t2; + } else { + t2 = $[5]; + } + const mapped = t2; + return mapped; +} +function _temp(item_0) { + return item_0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: { id: 42 } }], + isComponent: false, +}; + +``` + +### Eval output +(kind: ok) [{"a":{"id":42}}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-captures-receiver-noAlias.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-captures-receiver-noAlias.js new file mode 100644 index 0000000000000..42e32b3e38b3f --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/array-map-captures-receiver-noAlias.js @@ -0,0 +1,15 @@ +// @enableNewMutationAliasingModel +function Component(props) { + // This item is part of the receiver, should be memoized + const item = {a: props.a}; + const items = [item]; + const mapped = items.map(item => item); + // mapped[0].a = null; + return mapped; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {id: 42}}], + isComponent: false, +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/reactive-ref.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/reactive-ref.expect.md new file mode 100644 index 0000000000000..7939a03da7293 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/reactive-ref.expect.md @@ -0,0 +1,48 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +function ReactiveRefInEffect(props) { + const ref1 = useRef('initial value'); + const ref2 = useRef('initial value'); + let ref; + if (props.foo) { + ref = ref1; + } else { + ref = ref2; + } + useEffect(() => print(ref)); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +function ReactiveRefInEffect(props) { + const $ = _c(2); + const ref1 = useRef("initial value"); + const ref2 = useRef("initial value"); + let ref; + if (props.foo) { + ref = ref1; + } else { + ref = ref2; + } + let t0; + if ($[0] !== ref) { + t0 = () => print(ref); + $[0] = ref; + $[1] = t0; + } else { + t0 = $[1]; + } + useEffect(t0); +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/reactive-ref.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/reactive-ref.js new file mode 100644 index 0000000000000..3ae653c962034 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/reactive-ref.js @@ -0,0 +1,12 @@ +// @enableNewMutationAliasingModel +function ReactiveRefInEffect(props) { + const ref1 = useRef('initial value'); + const ref2 = useRef('initial value'); + let ref; + if (props.foo) { + ref = ref1; + } else { + ref = ref2; + } + useEffect(() => print(ref)); +}