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 0ef421ad6b..b6c9812918 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts @@ -57,7 +57,6 @@ import { import { printAliasingEffect, printAliasingSignature, - printFunction, printIdentifier, printInstruction, printInstructionValue, @@ -194,19 +193,15 @@ export function inferMutationAliasingEffects( hoistedContextDeclarations, ); - let count = 0; + let iterationCount = 0; while (queuedStates.size !== 0) { - count++; - if (count > 100) { - console.log( - 'oops infinite loop', - fn.id, - typeof fn.loc !== 'symbol' ? fn.loc?.filename : null, - ); - if (DEBUG) { - console.log(printFunction(fn)); - } - throw new Error('infinite loop'); + iterationCount++; + if (iterationCount > 100) { + CompilerError.invariant(false, { + reason: `[InferMutationAliasingEffects] Potential infinite loop`, + description: `A value, temporary place, or effect was not cached properly`, + loc: fn.loc, + }); } for (const [blockId, block] of fn.body.blocks) { const incomingState = queuedStates.get(blockId); @@ -217,11 +212,6 @@ export function inferMutationAliasingEffects( statesByBlock.set(blockId, incomingState); const state = incomingState.clone(); - if (DEBUG) { - console.log('*************'); - console.log(`bb${block.id}`); - console.log('*************'); - } inferBlock(context, state, block); for (const nextBlockId of eachTerminalSuccessor(block.terminal)) { @@ -867,9 +857,6 @@ function applyEffect( ), ); if (signatureEffects != null) { - if (DEBUG) { - console.log('apply function expression effects'); - } applyEffect( context, state, @@ -902,16 +889,10 @@ function applyEffect( ); } if (signatureEffects != null) { - if (DEBUG) { - console.log('apply aliasing signature effects'); - } for (const signatureEffect of signatureEffects) { applyEffect(context, state, signatureEffect, initialized, effects); } } else if (effect.signature != null) { - if (DEBUG) { - console.log('apply legacy signature effects'); - } const legacyEffects = computeEffectsForLegacySignature( state, effect.signature, @@ -924,9 +905,6 @@ function applyEffect( applyEffect(context, state, legacyEffect, initialized, effects); } } else { - if (DEBUG) { - console.log('default effects'); - } applyEffect( context, state, @@ -1292,9 +1270,6 @@ class InferenceState { kind: ValueKind.Frozen, reason: new Set([reason]), }); - if (DEBUG) { - console.log(`freeze value: ${printInstructionValue(value)} ${reason}`); - } if ( value.kind === 'FunctionExpression' && (this.env.config.enablePreserveExistingMemoizationGuarantees || @@ -2334,17 +2309,6 @@ 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 @@ -2359,9 +2323,6 @@ function computeEffectsForSignature( 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; @@ -2469,23 +2430,14 @@ function computeEffectsForSignature( 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 = []; @@ -2495,18 +2447,12 @@ function computeEffectsForSignature( } 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]}); 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 e0fb84fe5a..864eb29d8e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import prettyFormat from 'pretty-format'; import {CompilerError, SourceLocation} from '..'; import { BlockId, @@ -23,14 +22,9 @@ import { eachTerminalOperand, } from '../HIR/visitors'; import {assertExhaustive, getOrInsertWith} from '../Utils/utils'; -import {printFunction} from '../HIR'; -import {printIdentifier, printPlace} from '../HIR/PrintHIR'; import {MutationKind} from './InferFunctionExpressionAliasingEffectsSignature'; import {Result} from '../Utils/Result'; -const DEBUG = false; -const VERBOSE = false; - /** * Infers mutable ranges for all values in the program, using previously inferred * mutation/aliasing effects. This pass builds a data flow graph using the effects, @@ -56,10 +50,6 @@ export function inferMutationAliasingRanges( fn: HIRFunction, {isFunctionExpression}: {isFunctionExpression: boolean}, ): Result { - if (VERBOSE) { - console.log(); - console.log(printFunction(fn)); - } /** * 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. @@ -115,20 +105,6 @@ export function inferMutationAliasingRanges( seenBlocks.add(block.id); for (const instr of block.instructions) { - if ( - instr.value.kind === 'FunctionExpression' || - instr.value.kind === 'ObjectMethod' - ) { - state.create(instr.lvalue, { - kind: 'Function', - function: instr.value.loweredFunc.func, - }); - } else { - for (const lvalue of eachInstructionLValue(instr)) { - state.create(lvalue, {kind: 'Object'}); - } - } - if (instr.effects == null) continue; for (const effect of instr.effects) { if (effect.kind === 'Create') { @@ -141,6 +117,15 @@ export function inferMutationAliasingRanges( } else if (effect.kind === 'CreateFrom') { state.createFrom(index++, effect.from, effect.into); } else if (effect.kind === 'Assign') { + /** + * TODO: Invariant that the node is not initialized yet + * + * InferFunctionExpressionAliasingEffectSignatures currently infers + * Assign effects in some places that should be Alias, leading to + * Assign effects that reinitialize a value. The end result appears to + * be fine, but we should fix that inference pass so that we add the + * invariant here. + */ if (!state.nodes.has(effect.into.identifier)) { state.create(effect.into, {kind: 'Object'}); } @@ -216,10 +201,6 @@ export function inferMutationAliasingRanges( } } - if (VERBOSE) { - console.log(state.debug()); - console.log(pretty(mutations)); - } for (const mutation of mutations) { state.mutate( mutation.index, @@ -234,9 +215,6 @@ export function inferMutationAliasingRanges( for (const render of renders) { state.render(render.index, render.place.identifier, errors); } - if (DEBUG) { - console.log(pretty([...state.nodes.keys()])); - } fn.aliasingEffects ??= []; for (const param of [...fn.context, ...fn.params]) { const place = param.kind === 'Identifier' ? param : param.place; @@ -458,9 +436,6 @@ export function inferMutationAliasingRanges( } } - if (VERBOSE) { - console.log(printFunction(fn)); - } return errors.asResult(); } @@ -511,11 +486,6 @@ class AliasingState { const fromNode = this.nodes.get(from.identifier); const toNode = this.nodes.get(into.identifier); if (fromNode == null || toNode == null) { - if (VERBOSE) { - console.log( - `skip: createFrom ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, - ); - } return; } fromNode.edges.push({index, node: into.identifier, kind: 'alias'}); @@ -528,11 +498,6 @@ class AliasingState { const fromNode = this.nodes.get(from.identifier); const toNode = this.nodes.get(into.identifier); if (fromNode == null || toNode == null) { - if (VERBOSE) { - console.log( - `skip: capture ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, - ); - } return; } fromNode.edges.push({index, node: into.identifier, kind: 'capture'}); @@ -545,11 +510,6 @@ class AliasingState { const fromNode = this.nodes.get(from.identifier); const toNode = this.nodes.get(into.identifier); if (fromNode == null || toNode == null) { - if (VERBOSE) { - console.log( - `skip: assign ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`, - ); - } return; } fromNode.edges.push({index, node: into.identifier, kind: 'alias'}); @@ -604,11 +564,6 @@ class AliasingState { loc: SourceLocation, errors: CompilerError, ): void { - if (DEBUG) { - console.log( - `mutate ix=${index} start=$${start.id} end=[${end}]${transitive ? ' transitive' : ''} kind=${kind}`, - ); - } const seen = new Set(); const queue: Array<{ place: Identifier; @@ -623,18 +578,8 @@ class AliasingState { seen.add(current); const node = this.nodes.get(current); if (node == null) { - if (DEBUG) { - console.log( - `no node! ${printIdentifier(start)} for identifier ${printIdentifier(current)}`, - ); - } continue; } - if (DEBUG) { - console.log( - ` mutate $${node.id.id} transitive=${transitive} direction=${direction}`, - ); - } node.id.mutableRange.end = makeInstructionId( Math.max(node.id.mutableRange.end, end), ); @@ -701,37 +646,5 @@ class AliasingState { } } } - if (DEBUG) { - const nodes = new Map(); - for (const id of seen) { - const node = this.nodes.get(id); - nodes.set(id.id, node); - } - console.log(pretty(nodes)); - } - } - - debug(): string { - return pretty(this.nodes); } } - -export 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}`, - }, - ], - }); -}