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 67f1e75a23683..1842b8fe36456 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -189,6 +189,7 @@ export function inferMutationAliasingEffects( } class Context { + internedEffects: Map = new Map(); instructionSignatureCache: Map = new Map(); effectInstructionValueCache: Map = new Map(); @@ -200,6 +201,17 @@ class Context { this.isFuctionExpression = isFunctionExpression; this.fn = fn; } + + internEffect(effect: AliasingEffect): AliasingEffect { + const hash = hashEffect(effect); + let interned = this.internedEffects.get(hash); + if (interned == null) { + console.log(`intern: ${hash}`); + this.internedEffects.set(hash, effect); + interned = effect; + } + return interned; + } } function inferParam( @@ -388,10 +400,11 @@ function applySignature( function applyEffect( context: Context, state: InferenceState, - effect: AliasingEffect, + _effect: AliasingEffect, aliased: Set, effects: Array, ): void { + const effect = context.internEffect(_effect); if (DEBUG) { console.log(printAliasingEffect(effect)); } @@ -465,15 +478,12 @@ function applyEffect( break; } default: { - // Even if the source is non-primitive, the destination may be. skip primitives. - if (!isPrimitiveType(effect.into.identifier)) { - effects.push({ - // OK: recording information flow - kind: 'CreateFrom', // prev Alias - from: effect.from, - into: effect.into, - }); - } + effects.push({ + // OK: recording information flow + kind: 'CreateFrom', // prev Alias + from: effect.from, + into: effect.into, + }); } } break; @@ -1363,11 +1373,19 @@ function computeSignatureForInstruction( } case 'PropertyLoad': case 'ComputedLoad': { - effects.push({ - kind: 'CreateFrom', - from: value.object, - into: lvalue, - }); + if (isPrimitiveType(lvalue.identifier)) { + effects.push({ + kind: 'Create', + into: lvalue, + value: ValueKind.Primitive, + }); + } else { + effects.push({ + kind: 'CreateFrom', + from: value.object, + into: lvalue, + }); + } break; } case 'PropertyStore': @@ -1378,8 +1396,11 @@ function computeSignatureForInstruction( from: value.value, into: value.object, }); - // OK: lvalues are assigned to - effects.push({kind: 'Assign', from: value.value, into: lvalue}); + effects.push({ + kind: 'Create', + into: lvalue, + value: ValueKind.Primitive, + }); break; } case 'ObjectMethod': @@ -2207,7 +2228,23 @@ export type AliasedPlace = {place: Place; kind: 'alias' | 'capture'}; export type AliasingEffect = /** - * Marks the given value, its aliases, and indirect captures, as frozen. + * Marks the given value and its direct aliases as frozen. + * + * Captured values are *not* considered frozen, because we cannot be sure that a previously + * captured value will still be captured at the point of the freeze. + * + * For example: + * const x = {}; + * const y = [x]; + * y.pop(); // y dosn't contain x anymore! + * freeze(y); + * mutate(x); // safe to mutate! + * + * The exception to this is FunctionExpressions - since it is impossible to change which + * value a function closes over[1] we can transitively freeze functions and their captures. + * + * [1] Except for `let` values that are reassigned and closed over by a function, but we + * handle this explicitly with StoreContext/LoadContext. */ | {kind: 'Freeze'; value: Place; reason: ValueReason} /** @@ -2305,6 +2342,67 @@ export type AliasingEffect = error: CompilerErrorDetailOptions; }; +function hashEffect(effect: AliasingEffect): string { + switch (effect.kind) { + case 'Apply': { + return [ + effect.kind, + effect.receiver.identifier.id, + effect.function.identifier.id, + effect.mutatesFunction, + effect.args + .map(a => { + if (a.kind === 'Hole') { + return ''; + } else if (a.kind === 'Identifier') { + return a.identifier.id; + } else { + return `...${a.place.identifier.id}`; + } + }) + .join(','), + effect.into.identifier.id, + ].join(':'); + } + case 'CreateFrom': + case 'ImmutableCapture': + case 'Assign': + case 'Alias': + case 'Capture': { + return [ + effect.kind, + effect.from.identifier.id, + effect.into.identifier.id, + ].join(':'); + } + case 'Create': { + return [effect.kind, effect.into.identifier.id].join(':'); + } + case 'Freeze': { + return [effect.kind, effect.value.identifier.id, effect.reason].join(':'); + } + case 'MutateFrozen': + case 'MutateGlobal': { + return [effect.kind, effect.place.identifier.id].join(':'); + } + case 'Mutate': + case 'MutateConditionally': + case 'MutateTransitive': + case 'MutateTransitiveConditionally': { + return [effect.kind, effect.value.identifier.id].join(':'); + } + case 'CreateFunction': { + return [ + effect.kind, + effect.into.identifier.id, + // return places are a unique way to identify functions themselves + effect.function.loweredFunc.func.returns.identifier.id, + effect.captures.map(p => p.identifier.id).join(','), + ].join(':'); + } + } +} + export type AliasingSignatureEffect = AliasingEffect; export type AliasingSignature = { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts index ca620528dfcb5..f80c2053a423c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts @@ -160,9 +160,6 @@ export function inferMutationAliasingFunctionEffects( }); } - // console.log(pretty(tracked)); - // console.log(pretty(dataFlow)); - // console.log('FunctionEffects', effects.map(printAliasingEffect).join('\n')); return effects; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/capture-backedge-phi-with-later-mutation.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/capture-backedge-phi-with-later-mutation.expect.md new file mode 100644 index 0000000000000..e310d0d71e0c9 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/capture-backedge-phi-with-later-mutation.expect.md @@ -0,0 +1,105 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +import {arrayPush, Stringify} from 'shared-runtime'; + +function Component({prop1, prop2}) { + 'use memo'; + + // we'll ultimately extract the item from this array as z, and mutate later + let x = [{value: prop1}]; + let z; + while (x.length < 2) { + // there's a phi here for x (value before the loop and the reassignment later) + + // this mutation occurs before the reassigned value + arrayPush(x, {value: prop2}); + + // this condition will never be true, so x doesn't get reassigned + if (x[0].value === null) { + x = [{value: prop2}]; + const y = x; + z = y[0]; + } + } + // the code is set up so that z will always be the value from the original x + z.other = true; + return ; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{prop1: 0, prop2: 0}], + sequentialRenders: [ + {prop1: 0, prop2: 0}, + {prop1: 1, prop2: 0}, + {prop1: 1, prop2: 1}, + {prop1: 0, prop2: 1}, + {prop1: 0, prop2: 0}, + ], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +import { arrayPush, Stringify } from "shared-runtime"; + +function Component(t0) { + "use memo"; + const $ = _c(5); + const { prop1, prop2 } = t0; + let z; + if ($[0] !== prop1 || $[1] !== prop2) { + let x = [{ value: prop1 }]; + while (x.length < 2) { + arrayPush(x, { value: prop2 }); + if (x[0].value === null) { + x = [{ value: prop2 }]; + const y = x; + z = y[0]; + } + } + + z.other = true; + $[0] = prop1; + $[1] = prop2; + $[2] = z; + } else { + z = $[2]; + } + let t1; + if ($[3] !== z) { + t1 = ; + $[3] = z; + $[4] = t1; + } else { + t1 = $[4]; + } + return t1; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ prop1: 0, prop2: 0 }], + sequentialRenders: [ + { prop1: 0, prop2: 0 }, + { prop1: 1, prop2: 0 }, + { prop1: 1, prop2: 1 }, + { prop1: 0, prop2: 1 }, + { prop1: 0, prop2: 0 }, + ], +}; + +``` + +### Eval output +(kind: ok) [[ (exception in render) TypeError: Cannot set properties of undefined (setting 'other') ]] +[[ (exception in render) TypeError: Cannot set properties of undefined (setting 'other') ]] +[[ (exception in render) TypeError: Cannot set properties of undefined (setting 'other') ]] +[[ (exception in render) TypeError: Cannot set properties of undefined (setting 'other') ]] +[[ (exception in render) TypeError: Cannot set properties of undefined (setting 'other') ]] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/capture-backedge-phi-with-later-mutation.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/capture-backedge-phi-with-later-mutation.js new file mode 100644 index 0000000000000..bda11b500191a --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/capture-backedge-phi-with-later-mutation.js @@ -0,0 +1,35 @@ +// @enableNewMutationAliasingModel +import {arrayPush, Stringify} from 'shared-runtime'; + +function Component({prop1, prop2}) { + 'use memo'; + + let x = [{value: prop1}]; + let z; + while (x.length < 2) { + // there's a phi here for x (value before the loop and the reassignment later) + + // this mutation occurs before the reassigned value + arrayPush(x, {value: prop2}); + + if (x[0].value === prop1) { + x = [{value: prop2}]; + const y = x; + z = y[0]; + } + } + z.other = true; + return ; +} + +// export const FIXTURE_ENTRYPOINT = { +// fn: Component, +// params: [{prop1: 0, prop2: 0}], +// sequentialRenders: [ +// {prop1: 0, prop2: 0}, +// {prop1: 1, prop2: 0}, +// {prop1: 1, prop2: 1}, +// {prop1: 0, prop2: 1}, +// {prop1: 0, prop2: 0}, +// ], +// };