diff --git a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts index 7285140de0a..e4a9b0e8a64 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/CompilerError.ts @@ -115,6 +115,14 @@ export class CompilerErrorDetail { export class CompilerError extends Error { details: Array = []; + static from(details: Array): CompilerError { + const error = new CompilerError(); + for (const detail of details) { + error.push(detail); + } + return error; + } + static invariant( condition: unknown, options: Omit, 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 f10ea22a57d..ffa70467a56 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -1003,6 +1003,12 @@ export function printAliasingEffect(effect: AliasingEffect): string { case 'MutateTransitiveConditionally': { return `${effect.kind} ${printPlaceForAliasEffect(effect.value)}`; } + case 'MutateFrozen': { + return `MutateFrozen ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`; + } + case 'MutateGlobal': { + return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`; + } default: { assertExhaustive(effect, `Unexpected kind '${(effect as any).kind}'`); } 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 71bc2b810e7..73e687c5558 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -15,6 +15,7 @@ import { Place, isRefOrRefValue, makeInstructionId, + printFunction, } from '../HIR'; import {deadCodeElimination} from '../Optimization'; import {inferReactiveScopeVariables} from '../ReactiveScopes'; @@ -72,7 +73,10 @@ function lowerWithMutationAliasing(fn: HIRFunction): void { value: fn, }); const effects = inferMutationAliasingFunctionEffects(fn); - fn.aliasingEffects = effects; + if (effects != null) { + fn.aliasingEffects ??= []; + fn.aliasingEffects?.push(...effects); + } const capturedOrMutated = new Set(); for (const effect of effects ?? []) { @@ -97,6 +101,8 @@ function lowerWithMutationAliasing(fn: HIRFunction): void { capturedOrMutated.add(effect.value.identifier.id); break; } + case 'MutateFrozen': + case 'MutateGlobal': case 'CreateFunction': case 'Create': case 'Freeze': @@ -114,7 +120,10 @@ function lowerWithMutationAliasing(fn: HIRFunction): void { } for (const operand of fn.context) { - if (capturedOrMutated.has(operand.identifier.id)) { + if ( + capturedOrMutated.has(operand.identifier.id) || + operand.effect === Effect.Capture + ) { operand.effect = Effect.Capture; } else { operand.effect = Effect.Read; 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 0fd3a31c5eb..f58d5982497 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -5,7 +5,13 @@ * LICENSE file in the root directory of this source tree. */ -import {CompilerError, Effect, ValueKind} from '..'; +import { + CompilerError, + CompilerErrorDetailOptions, + Effect, + ErrorSeverity, + ValueKind, +} from '..'; import { BasicBlock, BlockId, @@ -214,11 +220,6 @@ function inferBlock( instructionSignature = computeSignatureForInstruction(state.env, instr); instructionSignatureCache.set(instr, instructionSignature); } - /* - * console.log( - * printInstruction({...instr, effects: [...instructionSignature.effects]}), - * ); - */ const effects = applySignature( state, instructionSignature, @@ -244,6 +245,7 @@ function applySignature( instruction: Instruction, effectInstructionValueCache: Map, ): Array | null { + const effects: Array = []; /** * For function instructions, eagerly validate that they aren't mutating * a known-frozen value. @@ -260,27 +262,33 @@ function applySignature( for (const effect of aliasingEffects) { if (effect.kind === 'Mutate' || effect.kind === 'MutateTransitive') { const value = state.kind(effect.value); - if (value.kind === ValueKind.Frozen) { - if (DEBUG) { - console.log(printInstruction(instruction)); - console.log(printAliasingEffect(effect)); - console.log(prettyFormat(state.debugAbstractValue(value))); + switch (value.kind) { + case ValueKind.Global: + case ValueKind.Frozen: { + const reason = getWriteErrorReason({ + kind: value.kind, + reason: value.reason, + context: new Set(), + }); + effects.push({ + kind: + value.kind === ValueKind.Frozen + ? 'MutateFrozen' + : 'MutateGlobal', + place: effect.value, + error: { + severity: ErrorSeverity.InvalidReact, + reason, + description: + effect.value.identifier.name !== null && + effect.value.identifier.name.kind === 'named' + ? `Found mutation of \`${effect.value.identifier.name}\`` + : null, + loc: effect.value.loc, + suggestions: null, + }, + }); } - const reason = getWriteErrorReason({ - kind: value.kind, - reason: value.reason, - context: new Set(), - }); - CompilerError.throwInvalidReact({ - reason, - description: - effect.value.identifier.name !== null && - effect.value.identifier.name.kind === 'named' - ? `Found mutation of \`${effect.value.identifier.name}\`` - : null, - loc: effect.value.loc, - suggestions: null, - }); } } } @@ -296,7 +304,6 @@ function applySignature( console.log(printInstruction(instruction)); } - const effects: Array = []; for (const effect of signature.effects) { applyEffect( state, @@ -429,6 +436,19 @@ function applyEffect( } } }); + for (const operand of effect.function.loweredFunc.func.context) { + if (operand.effect !== Effect.Capture) { + continue; + } + const kind = state.kind(operand).kind; + if ( + kind === ValueKind.Primitive || + kind == ValueKind.Frozen || + kind == ValueKind.Global + ) { + operand.effect = Effect.Read; + } + } state.initialize(effect.function, { kind: isMutable ? ValueKind.Mutable : ValueKind.Frozen, reason: new Set([]), @@ -742,24 +762,36 @@ function applyEffect( console.log(printAliasingEffect(effect)); console.log(prettyFormat(state.debugAbstractValue(value))); } + const reason = getWriteErrorReason({ kind: value.kind, reason: value.reason, context: new Set(), }); - CompilerError.throwInvalidReact({ - reason, - description: - effect.value.identifier.name !== null && - effect.value.identifier.name.kind === 'named' - ? `Found mutation of \`${effect.value.identifier.name}\`` - : null, - loc: effect.value.loc, - suggestions: null, + effects.push({ + kind: + value.kind === ValueKind.Frozen ? 'MutateFrozen' : 'MutateGlobal', + place: effect.value, + error: { + severity: ErrorSeverity.InvalidReact, + reason, + description: + effect.value.identifier.name !== null && + effect.value.identifier.name.kind === 'named' + ? `Found mutation of \`${effect.value.identifier.name}\`` + : null, + loc: effect.value.loc, + suggestions: null, + }, }); } break; } + case 'MutateFrozen': + case 'MutateGlobal': { + effects.push(effect); + break; + } default: { assertExhaustive( effect, @@ -933,7 +965,11 @@ class InferenceState { kind: ValueKind.Frozen, reason: new Set([reason]), }); - if (value.kind === 'FunctionExpression') { + if ( + value.kind === 'FunctionExpression' && + (this.env.config.enablePreserveExistingMemoizationGuarantees || + this.env.config.enableTransitivelyFreezeFunctionExpressions) + ) { for (const place of value.loweredFunc.func.context) { this.freeze(place, reason); } @@ -1552,15 +1588,31 @@ function computeSignatureForInstruction( }); break; } + case 'StartMemoize': + case 'FinishMemoize': { + if (env.config.enablePreserveExistingMemoizationGuarantees) { + for (const operand of eachInstructionValueOperand(value)) { + effects.push({ + kind: 'Freeze', + value: operand, + reason: ValueReason.Other, + }); + } + } + effects.push({ + kind: 'Create', + into: lvalue, + value: ValueKind.Primitive, + }); + break; + } case 'TaggedTemplateExpression': case 'BinaryExpression': case 'Debugger': - case 'FinishMemoize': case 'JSXText': case 'MetaProperty': case 'Primitive': case 'RegExpLiteral': - case 'StartMemoize': case 'TemplateLiteral': case 'UnaryExpression': case 'UnsupportedNode': { @@ -1879,6 +1931,14 @@ function computeEffectsForSignature( } break; } + case 'MutateFrozen': + case 'MutateGlobal': { + const values = substitutions.get(effect.place.identifier.id) ?? []; + for (const value of values) { + effects.push({kind: effect.kind, place: value, error: effect.error}); + } + break; + } case 'Mutate': case 'MutateTransitive': case 'MutateTransitiveConditionally': @@ -2165,6 +2225,18 @@ export type AliasingEffect = captures: Array; function: FunctionExpression | ObjectMethod; into: Place; + } + /** + * Mutation of a value known to be immutable + */ + | {kind: 'MutateFrozen'; place: Place; error: CompilerErrorDetailOptions} + /** + * Mutation of a global + */ + | { + kind: 'MutateGlobal'; + place: Place; + error: CompilerErrorDetailOptions; }; export type AliasingSignatureEffect = AliasingEffect; 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 666896dac19..a62eb424914 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingFunctionEffects.ts @@ -5,14 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import { - HIRFunction, - IdentifierId, - isPrimitiveType, - Place, - ScopeId, - ValueKind, -} from '../HIR'; +import {HIRFunction, IdentifierId, Place, ScopeId, ValueKind} from '../HIR'; import {getOrInsertDefault} from '../Utils/utils'; import {AliasingEffect} from './InferMutationAliasingEffects'; @@ -28,35 +21,6 @@ export function inferMutationAliasingFunctionEffects( const tracked = new Map(); tracked.set(fn.returns.identifier.id, fn.returns); - /** - * For each reactive scope we track whether there are known/conditional local and transitive - * mutations. This is used to recover precise mutation effects for each of the params and - * context variables. - */ - const trackedScopes = new Map< - ScopeId, - {local: MutationKind; transitive: MutationKind} - >(); - for (const operand of fn.context) { - tracked.set(operand.identifier.id, operand); - if (operand.identifier.scope != null) { - getOrInsertDefault(trackedScopes, operand.identifier.scope.id, { - local: MutationKind.None, - transitive: MutationKind.None, - }); - } - } - for (const param of fn.params) { - const place = param.kind === 'Identifier' ? param : param.place; - tracked.set(place.identifier.id, place); - if (place.identifier.scope != null) { - getOrInsertDefault(trackedScopes, place.identifier.scope.id, { - local: MutationKind.None, - transitive: MutationKind.None, - }); - } - } - /** * Track capturing/aliasing of context vars and params into each other and into the return. * We don't need to track locals and intermediate values, since we're only concerned with effects @@ -132,36 +96,10 @@ export function inferMutationAliasingFunctionEffects( } } } else if ( - effect.kind === 'Mutate' && - effect.value.identifier.scope != null && - trackedScopes.has(effect.value.identifier.scope.id) - ) { - const scope = trackedScopes.get(effect.value.identifier.scope.id)!; - scope.local = MutationKind.Definite; - } else if ( - effect.kind === 'MutateConditionally' && - effect.value.identifier.scope != null && - trackedScopes.has(effect.value.identifier.scope.id) + effect.kind === 'MutateFrozen' || + effect.kind === 'MutateGlobal' ) { - const scope = trackedScopes.get(effect.value.identifier.scope.id)!; - scope.local = Math.max(MutationKind.Conditional, scope.local); - } else if ( - effect.kind === 'MutateTransitive' && - effect.value.identifier.scope != null && - trackedScopes.has(effect.value.identifier.scope.id) - ) { - const scope = trackedScopes.get(effect.value.identifier.scope.id)!; - scope.transitive = MutationKind.Definite; - } else if ( - effect.kind === 'MutateTransitiveConditionally' && - effect.value.identifier.scope != null && - trackedScopes.has(effect.value.identifier.scope.id) - ) { - const scope = trackedScopes.get(effect.value.identifier.scope.id)!; - scope.transitive = Math.max( - MutationKind.Conditional, - scope.transitive, - ); + effects.push(effect); } } } @@ -175,26 +113,6 @@ export function inferMutationAliasingFunctionEffects( } } - // Create mutation effects based on observed mutation types - for (const value of tracked.values()) { - if ( - value.identifier.id === fn.returns.identifier.id || - value.identifier.scope == null - ) { - continue; - } - const scope = trackedScopes.get(value.identifier.scope.id)!; - if (scope.local === MutationKind.Definite) { - effects.push({kind: 'Mutate', value}); - } else if (scope.local === MutationKind.Conditional) { - effects.push({kind: 'MutateConditionally', value}); - } - if (scope.transitive === MutationKind.Definite) { - effects.push({kind: 'MutateTransitive', value}); - } else if (scope.transitive === MutationKind.Conditional) { - effects.push({kind: 'MutateTransitiveConditionally', value}); - } - } // Create aliasing effects based on observed data flow let hasReturn = false; for (const [into, from] of dataFlow) { @@ -218,16 +136,17 @@ export function inferMutationAliasingFunctionEffects( effects.push({ kind: 'Create', into: fn.returns, - value: isPrimitiveType(fn.returns.identifier) - ? ValueKind.Primitive - : ValueKind.Mutable, + value: + fn.returnType.kind === 'Primitive' + ? ValueKind.Primitive + : ValueKind.Mutable, }); } return effects; } -enum MutationKind { +export enum MutationKind { None = 0, Conditional = 1, Definite = 2, 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 651ea451633..e42b71a0722 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -6,8 +6,9 @@ */ import prettyFormat from 'pretty-format'; -import {CompilerError, SourceLocation} from '..'; +import {CompilerError} from '..'; import { + BlockId, Effect, HIRFunction, Identifier, @@ -23,7 +24,8 @@ import { } from '../HIR/visitors'; import {assertExhaustive, getOrInsertWith} from '../Utils/utils'; import {printFunction} from '../HIR'; -import {printInstruction} from '../HIR/PrintHIR'; +import {printIdentifier, printPlace} from '../HIR/PrintHIR'; +import {MutationKind} from './InferMutationAliasingFunctionEffects'; const DEBUG = false; @@ -33,79 +35,180 @@ const DEBUG = false; export function inferMutationAliasingRanges(fn: HIRFunction): void { if (DEBUG) { console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(); + console.log(printFunction(fn)); } /** - * Part 1: Infer mutable ranges for values. We build an abstract model - * of the effect types, distinguishing values which can capture references - * to other values and variables, which can point to one or more values. - * - * Transitive mutation marks value identifiers as mutated and also walks - * into the identifiers captured by that abstract value: local mutation - * only marks the top-level identifiers as mutated. + * Part 1: Infer mutable ranges for values. We build an abstract model of + * values, the alias/capture edges between them, and the set of mutations. + * Edges and mutations are ordered, with mutations processed against the + * abstract model only after it is fully constructed by visiting all blocks + * _and_ connecting phis. Phis are considered ordered at the time of the + * phi node. * - * TODO: add a fixpoint. + * This should (may?) mean that mutations are able to see the full state + * of the graph and mark all the appropriate identifiers as mutated at + * the correct point, accounting for both backward and forward edges. + * Ie a mutation of x accounts for both values that flowed into x, + * and values that x flowed into. */ const state = new AliasingState(); - for (const param of fn.context) { - state.create(param); + type PendingPhiOperand = {from: Place; into: Place; index: number}; + const pendingPhis = new Map>(); + const mutations: Array<{ + index: number; + id: InstructionId; + transitive: boolean; + kind: MutationKind; + place: Place; + }> = []; + + let index = 0; + + for (const param of [...fn.params, ...fn.context, fn.returns]) { + const place = param.kind === 'Identifier' ? param : param.place; + state.create(place); } + const seenBlocks = new Set(); for (const block of fn.body.blocks.values()) { for (const phi of block.phis) { state.create(phi.place); - for (const operand of phi.operands.values()) { - state.alias(operand, phi.place); + for (const [pred, operand] of phi.operands) { + if (!seenBlocks.has(pred)) { + // NOTE: annotation required to actually typecheck and not silently infer `any` + const blockPhis = getOrInsertWith>( + pendingPhis, + pred, + () => [], + ); + blockPhis.push({from: operand, into: phi.place, index: index++}); + } else { + state.assign(index++, operand, phi.place); + } } } + seenBlocks.add(block.id); for (const instr of block.instructions) { for (const lvalue of eachInstructionLValue(instr)) { state.create(lvalue); - if (lvalue.identifier.mutableRange.start === 0) { - lvalue.identifier.mutableRange.start = instr.id; - } - lvalue.identifier.mutableRange.end = makeInstructionId( - Math.max(instr.id + 1, lvalue.identifier.mutableRange.end), - ); } if (instr.effects == null) continue; for (const effect of instr.effects) { if (effect.kind === 'Create' || effect.kind === 'CreateFunction') { state.create(effect.into); - } else if (effect.kind === 'Assign') { - state.assign(effect.from, effect.into); - } else if (effect.kind === 'Alias' || effect.kind === 'CreateFrom') { - state.alias(effect.from, effect.into); + } else if ( + effect.kind === 'Assign' || + effect.kind === 'Alias' || + effect.kind === 'CreateFrom' + ) { + state.assign(index++, effect.from, effect.into); } else if (effect.kind === 'Capture') { - state.capture(effect.from, effect.into); + state.capture(index++, effect.from, effect.into); } else if ( effect.kind === 'MutateTransitive' || effect.kind === 'MutateTransitiveConditionally' ) { - state.mutateTransitive( - effect.value.identifier, - makeInstructionId(instr.id + 1), - effect.value.loc, - ); + mutations.push({ + index: index++, + id: instr.id, + transitive: true, + kind: + effect.kind === 'MutateTransitive' + ? MutationKind.Definite + : MutationKind.Conditional, + place: effect.value, + }); } else if ( effect.kind === 'Mutate' || effect.kind === 'MutateConditionally' ) { - state.mutate( - effect.value.identifier, - makeInstructionId(instr.id + 1), - effect.value.loc, - ); + mutations.push({ + index: index++, + id: instr.id, + transitive: false, + kind: + effect.kind === 'Mutate' + ? MutationKind.Definite + : MutationKind.Conditional, + place: effect.value, + }); } } - if (DEBUG) { - console.log(printInstruction(instr)); - console.log(state.debug()); + } + const blockPhis = pendingPhis.get(block.id); + if (blockPhis != null) { + for (const {from, into, index} of blockPhis) { + state.assign(index, from, into); } } + if (block.terminal.kind === 'return') { + state.assign(index++, block.terminal.value, fn.returns); + } } + if (DEBUG) { console.log(state.debug()); + console.log(pretty(mutations)); + } + for (const mutation of mutations) { + state.mutate( + mutation.index, + mutation.place, + makeInstructionId(mutation.id + 1), + mutation.transitive, + mutation.kind, + ); + } + if (DEBUG) { + console.log(state.debug()); + } + fn.aliasingEffects ??= []; + for (const param of fn.context) { + const node = state.nodes.get(param.identifier); + if (node == null) { + continue; + } + let mutated = false; + if (node.local === MutationKind.Conditional) { + mutated = true; + fn.aliasingEffects.push({ + kind: 'MutateConditionally', + value: param, + }); + } else if (node.local === MutationKind.Definite) { + mutated = true; + fn.aliasingEffects.push({ + kind: 'Mutate', + value: param, + }); + } + if (node.transitive === MutationKind.Conditional) { + mutated = true; + fn.aliasingEffects.push({ + kind: 'MutateTransitiveConditionally', + value: param, + }); + } else if (node.transitive === MutationKind.Definite) { + mutated = true; + fn.aliasingEffects.push({ + kind: 'MutateTransitive', + value: param, + }); + } + if (mutated) { + param.effect = Effect.Capture; + } } /** @@ -129,13 +232,21 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { } } for (const instr of block.instructions) { - if (instr.effects == null) { - for (const lvalue of eachInstructionLValue(instr)) { - lvalue.effect = Effect.ConditionallyMutate; + for (const lvalue of eachInstructionLValue(instr)) { + lvalue.effect = Effect.ConditionallyMutate; + if (lvalue.identifier.mutableRange.start === 0) { + lvalue.identifier.mutableRange.start = instr.id; } - for (const operand of eachInstructionValueOperand(instr.value)) { - operand.effect = Effect.Read; + if (lvalue.identifier.mutableRange.end === 0) { + lvalue.identifier.mutableRange.end = makeInstructionId( + Math.max(instr.id + 1, lvalue.identifier.mutableRange.end), + ); } + } + for (const operand of eachInstructionValueOperand(instr.value)) { + operand.effect = Effect.Read; + } + if (instr.effects == null) { continue; } const operandEffects = new Map(); @@ -187,6 +298,11 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { operandEffects.set(effect.value.identifier.id, Effect.Freeze); break; } + case 'MutateFrozen': + case 'MutateGlobal': { + // no-op + break; + } default: { assertExhaustive( effect, @@ -222,132 +338,156 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void { } } -/** - * TODO: similar to alias effect inference state: - * - values represent the conceptual values (context vars, things that got Create*-ed) - * into which other values are captured - * - variables deals with aliases (Assign, Alias, CreateFrom) - * - * Ex: - * `Capture a -> b`: - * - find the values represented by `a` (aValues) by looking up a in .variables, then mapping to .values - * - find the values represented by `b` (bValues) by looking up b in .variables, then mapping to .values - * - add aValues into bValues - */ +type Node = { + id: Identifier; + captures: Map; + aliases: Map; + edges: Array<{index: number; node: Identifier; kind: 'capture' | 'alias'}>; + transitive: MutationKind; + local: MutationKind; +}; class AliasingState { - values: Map> = new Map(); - variables: Map> = new Map(); + nodes: Map = new Map(); create(place: Place): void { - this.values.set(place.identifier, new Set()); - this.variables.set(place.identifier.id, new Set([place.identifier])); + this.nodes.set(place.identifier, { + id: place.identifier, + captures: new Map(), + aliases: new Map(), + edges: [], + transitive: MutationKind.None, + local: MutationKind.None, + }); } - clear(lvalue: Place): void { - this.variables.set(lvalue.identifier.id, new Set()); - } - - assign(from: Place, into: Place): void { - const fromVariables = this.variables.get(from.identifier.id); - if (fromVariables == null) { + capture(index: number, from: Place, into: Place): void { + const fromNode = this.nodes.get(from.identifier); + const toNode = this.nodes.get(into.identifier); + if (fromNode == null || toNode == null) { + if (DEBUG) { + console.log( + `skip: capture ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, + ); + } return; } - this.variables.set( - into.identifier.id, - new Set([...fromVariables, from.identifier, into.identifier]), - // fromVariables, - ); - } - - alias(from: Place, into: Place): void { - const intoVariables = getOrInsertWith( - this.variables, - into.identifier.id, - () => new Set(), - ); - intoVariables.add(from.identifier); - } - - capture(from: Place, into: Place): void { - const intoVariables = this.variables.get(into.identifier.id)!; - for (const v of intoVariables) { - const values = this.values.get(v)!; - values.add(from.identifier); + fromNode.edges.push({index, node: into.identifier, kind: 'capture'}); + if (!toNode.captures.has(from.identifier)) { + toNode.captures.set(from.identifier, index); } } - mutateTransitive( - place: Identifier, - end: InstructionId, - loc: SourceLocation, - ): void { - const variables = this.variables.get(place.id); - if (variables == null) { - return; - } - for (const value of variables) { - value.mutableRange.end = makeInstructionId( - Math.max(value.mutableRange.end, end), - ); - const captured = this.values.get(value)!; - for (const capture of captured) { - this.mutateTransitive(capture, end, loc); + assign(index: number, from: Place, into: Place): void { + const fromNode = this.nodes.get(from.identifier); + const toNode = this.nodes.get(into.identifier); + if (fromNode == null || toNode == null) { + if (DEBUG) { + console.log( + `skip: assign ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, + ); } - } - } - - mutate(place: Identifier, end: InstructionId, _loc: SourceLocation): void { - const variables = this.variables.get(place.id); - if (variables == null) { return; } - place.mutableRange.end = makeInstructionId( - Math.max(place.mutableRange.end, end), - ); - for (const value of variables) { - // Unlike mutateTransitive, we don't traverse into captured values - value.mutableRange.end = makeInstructionId( - Math.max(value.mutableRange.end, end), - ); + fromNode.edges.push({index, node: into.identifier, kind: 'alias'}); + if (!toNode.captures.has(from.identifier)) { + toNode.aliases.set(from.identifier, index); } } - canonicalize(): Map { - const map = new Map(); - for (const value of this.values.keys()) { - map.set( - value.id, - `${value.mutableRange.start}:${value.mutableRange.end}`, + mutate( + index: number, + start: Place, + end: InstructionId, + transitive: boolean, + kind: MutationKind, + ): void { + const seen = new Set(); + const queue = [start.identifier]; + while (queue.length !== 0) { + const current = queue.pop()!; + if (seen.has(current)) { + continue; + } + seen.add(current); + const node = this.nodes.get(current); + if (node == null) { + if (DEBUG) { + console.log( + `no node! ${printPlace(start)} for identifier ${printIdentifier(current)}`, + ); + } + continue; + } + if (DEBUG) { + console.log( + `mutate $${node.id.id} via ${printPlace(start)} at [${end}]`, + ); + } + node.id.mutableRange.end = makeInstructionId( + Math.max(node.id.mutableRange.end, end), ); + if (transitive) { + node.transitive = Math.max(node.transitive, kind); + } else { + node.local = Math.max(node.local, kind); + } + /** + * all mutations affect "forward" edges by the rules: + * - Capture a -> b, mutate(a) => mutate(b) + * - Alias a -> b, mutate(a) => mutate(b) + */ + for (const edge of node.edges) { + if (edge.index >= index) { + break; + } + queue.push(edge.node); + } + /** + * all mutations affect backward alias edges by the rules: + * - Alias a -> b, mutate(b) => mutate(a) + * - Alias a -> b, mutateTransitive(b) => mutate(a) + */ + for (const [alias, when] of node.aliases) { + if (when >= index) { + continue; + } + queue.push(alias); + } + /** + * but only transitive mutations affect captures + */ + if (transitive) { + for (const [capture, when] of node.captures) { + if (when >= index) { + continue; + } + queue.push(capture); + } + } } - return map; } debug(): string { - const values = new Map(); - for (const [k, v] of this.values) { - values.set( - `${k.name?.value ?? ''}$${k.id}:${k.mutableRange.start}:${k.mutableRange.end}`, - [...v] - .map( - v => - `${v.name?.value ?? ''}$${v.id}:${v.mutableRange.start}:${v.mutableRange.end}`, - ) - .join(', '), - ); - } - const variables = new Map(); - for (const [k, v] of this.variables) { - variables.set( - `$${k}`, - [...v] - .map( - v => - `${v.name?.value ?? ''}$${v.id}:${v.mutableRange.start}:${v.mutableRange.end}`, - ) - .join(', '), - ); - } - return prettyFormat({values, variables}); + return pretty(this.nodes); } } + +function pretty(v: any): string { + return prettyFormat(v, { + plugins: [ + { + test: v => + v !== null && typeof v === 'object' && v.kind === 'Identifier', + serialize: v => printPlace(v), + }, + { + test: v => + v !== null && + typeof v === 'object' && + typeof v.declarationId === 'number', + serialize: v => + `${printIdentifier(v)}:${v.mutableRange.start}:${v.mutableRange.end}`, + }, + ], + }); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md new file mode 100644 index 00000000000..a09996febb0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.expect.md @@ -0,0 +1,87 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +import {identity, mutate} from 'shared-runtime'; + +/** + * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr + * with the mutation hoisted to a named variable instead of being directly + * inlined into the Object key. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * Forget: + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] + */ +function Component(props) { + const key = {}; + const tmp = (mutate(key), key); + const context = { + // Here, `tmp` is frozen (as it's inferred to be a primitive/string) + [tmp]: identity([props.value]), + }; + mutate(key); + return [context, key]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [{value: 42}, {value: 42}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +import { identity, mutate } from "shared-runtime"; + +/** + * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr + * with the mutation hoisted to a named variable instead of being directly + * inlined into the Object key. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * Forget: + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] + */ +function Component(props) { + const $ = _c(2); + let t0; + if ($[0] !== props.value) { + const key = {}; + const tmp = (mutate(key), key); + const context = { [tmp]: identity([props.value]) }; + + mutate(key); + t0 = [context, key]; + $[0] = props.value; + $[1] = t0; + } else { + t0 = $[1]; + } + return t0; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ value: 42 }], + sequentialRenders: [{ value: 42 }, { value: 42 }], +}; + +``` + +### Eval output +(kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] +[{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js new file mode 100644 index 00000000000..57b2d5db9f5 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr.js @@ -0,0 +1,32 @@ +// @enableNewMutationAliasingModel +import {identity, mutate} from 'shared-runtime'; + +/** + * Bug: copy of error.todo-object-expression-computed-key-modified-during-after-construction-sequence-expr + * with the mutation hoisted to a named variable instead of being directly + * inlined into the Object key. + * + * Found differences in evaluator results + * Non-forget (expected): + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * Forget: + * (kind: ok) [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe"}] + * [{"[object Object]":[42]},{"wat0":"joe","wat1":"joe","wat2":"joe"}] + */ +function Component(props) { + const key = {}; + const tmp = (mutate(key), key); + const context = { + // Here, `tmp` is frozen (as it's inferred to be a primitive/string) + [tmp]: identity([props.value]), + }; + mutate(key); + return [context, key]; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{value: 42}], + sequentialRenders: [{value: 42}, {value: 42}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/iife-return-modified-later-phi.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/iife-return-modified-later-phi.expect.md new file mode 100644 index 00000000000..22f967883b0 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/iife-return-modified-later-phi.expect.md @@ -0,0 +1,58 @@ + +## Input + +```javascript +function Component(props) { + const items = (() => { + if (props.cond) { + return []; + } else { + return null; + } + })(); + items?.push(props.a); + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {}}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function Component(props) { + const $ = _c(3); + let items; + if ($[0] !== props.a || $[1] !== props.cond) { + let t0; + if (props.cond) { + t0 = []; + } else { + t0 = null; + } + items = t0; + + items?.push(props.a); + $[0] = props.a; + $[1] = props.cond; + $[2] = items; + } else { + items = $[2]; + } + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{ a: {} }], +}; + +``` + +### Eval output +(kind: ok) null \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/iife-return-modified-later-phi.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/iife-return-modified-later-phi.js new file mode 100644 index 00000000000..f4f953d294e --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/iife-return-modified-later-phi.js @@ -0,0 +1,16 @@ +function Component(props) { + const items = (() => { + if (props.cond) { + return []; + } else { + return null; + } + })(); + items?.push(props.a); + return items; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{a: {}}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitive-mutation-before-capturing-value-created-earlier.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitive-mutation-before-capturing-value-created-earlier.expect.md new file mode 100644 index 00000000000..09c4e3eaf33 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitive-mutation-before-capturing-value-created-earlier.expect.md @@ -0,0 +1,50 @@ + +## Input + +```javascript +// @enableNewMutationAliasingModel +function Component({a, b}) { + const x = [a]; + const y = {b}; + mutate(y); + y.x = x; + return
{y}
; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel +function Component(t0) { + const $ = _c(5); + const { a, b } = t0; + let t1; + if ($[0] !== a) { + t1 = [a]; + $[0] = a; + $[1] = t1; + } else { + t1 = $[1]; + } + const x = t1; + let t2; + if ($[2] !== b || $[3] !== x) { + const y = { b }; + mutate(y); + y.x = x; + t2 =
{y}
; + $[2] = b; + $[3] = x; + $[4] = t2; + } else { + t2 = $[4]; + } + return t2; +} + +``` + +### 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/transitive-mutation-before-capturing-value-created-earlier.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitive-mutation-before-capturing-value-created-earlier.js new file mode 100644 index 00000000000..e6e2e17bc0b --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/transitive-mutation-before-capturing-value-created-earlier.js @@ -0,0 +1,8 @@ +// @enableNewMutationAliasingModel +function Component({a, b}) { + const x = [a]; + const y = {b}; + mutate(y); + y.x = x; + return
{y}
; +}