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 f5501980a16cb..3f107f3682fbb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutableRanges.ts @@ -17,7 +17,7 @@ import {inferMutableLifetimes} from './InferMutableLifetimes'; import {inferMutableRangesForAlias} from './InferMutableRangesForAlias'; import {inferMutableRangesForComutation} from './InferMutableRangesForComutation'; import {inferTryCatchAliases} from './InferTryCatchAliases'; -import {printIdentifier, printMutableRange} from '../HIR/PrintHIR'; +import {printIdentifier} from '../HIR/PrintHIR'; export function inferMutableRanges(ir: HIRFunction): DisjointSet { // Infer mutable ranges for non fields @@ -105,7 +105,8 @@ export function debugAliases(aliases: DisjointSet): void { .buildSets() .map(set => [...set].map( - ident => printIdentifier(ident) + printMutableRange(ident), + ident => + `${printIdentifier(ident)}:${ident.mutableRange.start}:${ident.mutableRange.end}`, ), ), ), 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 82f8e3529febd..b33722426c71e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -986,7 +986,7 @@ function computeSignatureForInstruction( value.args, ) : null; - if (signatureEffects != null && signature?.aliasing != null) { + if (signatureEffects != null) { effects.push(...signatureEffects); } else if (signature != null) { effects.push( 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 e7224be8066b1..a3e839bb298c1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -21,6 +21,9 @@ import DisjointSet from '../Utils/DisjointSet'; import {assertExhaustive} from '../Utils/utils'; import {inferMutableRangesForAlias} from './InferMutableRangesForAlias'; +/** + * Infers mutable ranges for all values. + */ export function inferMutationAliasingRanges(fn: HIRFunction): void { /** * Part 1 @@ -64,6 +67,24 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { } } } + /** + * TODO: this will incorrectly mark values as mutated in the following case: + * 1. Create value x + * 2. Create value y + * 3. Transitively mutate y + * 4. Capture x -> y + * + * We will put these all into one alias set in captures, and then InferMutableRangesForAlias + * will look at all identifiers in the set that start before the mutation. Thus it will see + * that x is created before the mutation, and consider it mutated. But the capture doesn't + * occur until after the mutation! + * + * The fix is to use a graph to precisely describe what is captured where at each instruction, + * so that on a transitive mutation we can iterate all the captured values and mark them. + * + * This then needs a fixpoint: although we would track captures for phi operands, the operands' + * own capture values won't be populated until we do a fixpoint. + */ inferMutableRangesForAlias(fn, captures); /** @@ -71,6 +92,9 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { * Infer ranges for local (non-transitive) mutations. We build a disjoint set * that only tracks direct value aliasing, and look only at local mutations * to extend ranges + * + * TODO: similar design to the above todo for captures, use a directed graph instead of disjoint union, + * with fixpoint. */ const aliases = new DisjointSet(); for (const block of fn.body.blocks.values()) {