Skip to content

Commit 2bee348

Browse files
josephsavonajosephsavona
authored
[compiler] Cleanup debugging code (#33571)
Removes unnecessary debugging code in the new inference passes now that they've stabilized more. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33571). * __->__ #33571 * #33558 * #33547
1 parent d37faa0 commit 2bee348

File tree

2 files changed

+17
-158
lines changed

2 files changed

+17
-158
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts

Lines changed: 8 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ import {
5757
import {
5858
printAliasingEffect,
5959
printAliasingSignature,
60-
printFunction,
6160
printIdentifier,
6261
printInstruction,
6362
printInstructionValue,
@@ -194,19 +193,15 @@ export function inferMutationAliasingEffects(
194193
hoistedContextDeclarations,
195194
);
196195

197-
let count = 0;
196+
let iterationCount = 0;
198197
while (queuedStates.size !== 0) {
199-
count++;
200-
if (count > 100) {
201-
console.log(
202-
'oops infinite loop',
203-
fn.id,
204-
typeof fn.loc !== 'symbol' ? fn.loc?.filename : null,
205-
);
206-
if (DEBUG) {
207-
console.log(printFunction(fn));
208-
}
209-
throw new Error('infinite loop');
198+
iterationCount++;
199+
if (iterationCount > 100) {
200+
CompilerError.invariant(false, {
201+
reason: `[InferMutationAliasingEffects] Potential infinite loop`,
202+
description: `A value, temporary place, or effect was not cached properly`,
203+
loc: fn.loc,
204+
});
210205
}
211206
for (const [blockId, block] of fn.body.blocks) {
212207
const incomingState = queuedStates.get(blockId);
@@ -217,11 +212,6 @@ export function inferMutationAliasingEffects(
217212

218213
statesByBlock.set(blockId, incomingState);
219214
const state = incomingState.clone();
220-
if (DEBUG) {
221-
console.log('*************');
222-
console.log(`bb${block.id}`);
223-
console.log('*************');
224-
}
225215
inferBlock(context, state, block);
226216

227217
for (const nextBlockId of eachTerminalSuccessor(block.terminal)) {
@@ -867,9 +857,6 @@ function applyEffect(
867857
),
868858
);
869859
if (signatureEffects != null) {
870-
if (DEBUG) {
871-
console.log('apply function expression effects');
872-
}
873860
applyEffect(
874861
context,
875862
state,
@@ -902,16 +889,10 @@ function applyEffect(
902889
);
903890
}
904891
if (signatureEffects != null) {
905-
if (DEBUG) {
906-
console.log('apply aliasing signature effects');
907-
}
908892
for (const signatureEffect of signatureEffects) {
909893
applyEffect(context, state, signatureEffect, initialized, effects);
910894
}
911895
} else if (effect.signature != null) {
912-
if (DEBUG) {
913-
console.log('apply legacy signature effects');
914-
}
915896
const legacyEffects = computeEffectsForLegacySignature(
916897
state,
917898
effect.signature,
@@ -924,9 +905,6 @@ function applyEffect(
924905
applyEffect(context, state, legacyEffect, initialized, effects);
925906
}
926907
} else {
927-
if (DEBUG) {
928-
console.log('default effects');
929-
}
930908
applyEffect(
931909
context,
932910
state,
@@ -1292,9 +1270,6 @@ class InferenceState {
12921270
kind: ValueKind.Frozen,
12931271
reason: new Set([reason]),
12941272
});
1295-
if (DEBUG) {
1296-
console.log(`freeze value: ${printInstructionValue(value)} ${reason}`);
1297-
}
12981273
if (
12991274
value.kind === 'FunctionExpression' &&
13001275
(this.env.config.enablePreserveExistingMemoizationGuarantees ||
@@ -2334,17 +2309,6 @@ function computeEffectsForSignature(
23342309
// Too many args and there is no rest param to hold them
23352310
(args.length > signature.params.length && signature.rest == null)
23362311
) {
2337-
if (DEBUG) {
2338-
if (signature.params.length > args.length) {
2339-
console.log(
2340-
`not enough args: ${args.length} args for ${signature.params.length} params`,
2341-
);
2342-
} else {
2343-
console.log(
2344-
`too many args: ${args.length} args for ${signature.params.length} params, with no rest param`,
2345-
);
2346-
}
2347-
}
23482312
return null;
23492313
}
23502314
// Build substitutions
@@ -2359,9 +2323,6 @@ function computeEffectsForSignature(
23592323
continue;
23602324
} else if (params == null || i >= params.length || arg.kind === 'Spread') {
23612325
if (signature.rest == null) {
2362-
if (DEBUG) {
2363-
console.log(`no rest value to hold param`);
2364-
}
23652326
return null;
23662327
}
23672328
const place = arg.kind === 'Identifier' ? arg : arg.place;
@@ -2469,23 +2430,14 @@ function computeEffectsForSignature(
24692430
case 'Apply': {
24702431
const applyReceiver = substitutions.get(effect.receiver.identifier.id);
24712432
if (applyReceiver == null || applyReceiver.length !== 1) {
2472-
if (DEBUG) {
2473-
console.log(`too many substitutions for receiver`);
2474-
}
24752433
return null;
24762434
}
24772435
const applyFunction = substitutions.get(effect.function.identifier.id);
24782436
if (applyFunction == null || applyFunction.length !== 1) {
2479-
if (DEBUG) {
2480-
console.log(`too many substitutions for function`);
2481-
}
24822437
return null;
24832438
}
24842439
const applyInto = substitutions.get(effect.into.identifier.id);
24852440
if (applyInto == null || applyInto.length !== 1) {
2486-
if (DEBUG) {
2487-
console.log(`too many substitutions for into`);
2488-
}
24892441
return null;
24902442
}
24912443
const applyArgs: Array<Place | SpreadPattern | Hole> = [];
@@ -2495,18 +2447,12 @@ function computeEffectsForSignature(
24952447
} else if (arg.kind === 'Identifier') {
24962448
const applyArg = substitutions.get(arg.identifier.id);
24972449
if (applyArg == null || applyArg.length !== 1) {
2498-
if (DEBUG) {
2499-
console.log(`too many substitutions for arg`);
2500-
}
25012450
return null;
25022451
}
25032452
applyArgs.push(applyArg[0]);
25042453
} else {
25052454
const applyArg = substitutions.get(arg.place.identifier.id);
25062455
if (applyArg == null || applyArg.length !== 1) {
2507-
if (DEBUG) {
2508-
console.log(`too many substitutions for arg`);
2509-
}
25102456
return null;
25112457
}
25122458
applyArgs.push({kind: 'Spread', place: applyArg[0]});

‎compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts

Lines changed: 9 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import prettyFormat from 'pretty-format';
98
import {CompilerError, SourceLocation} from '..';
109
import {
1110
BlockId,
@@ -23,14 +22,9 @@ import {
2322
eachTerminalOperand,
2423
} from '../HIR/visitors';
2524
import {assertExhaustive, getOrInsertWith} from '../Utils/utils';
26-
import {printFunction} from '../HIR';
27-
import {printIdentifier, printPlace} from '../HIR/PrintHIR';
2825
import {MutationKind} from './InferFunctionExpressionAliasingEffectsSignature';
2926
import {Result} from '../Utils/Result';
3027

31-
const DEBUG = false;
32-
const VERBOSE = false;
33-
3428
/**
3529
* Infers mutable ranges for all values in the program, using previously inferred
3630
* mutation/aliasing effects. This pass builds a data flow graph using the effects,
@@ -56,10 +50,6 @@ export function inferMutationAliasingRanges(
5650
fn: HIRFunction,
5751
{isFunctionExpression}: {isFunctionExpression: boolean},
5852
): Result<void, CompilerError> {
59-
if (VERBOSE) {
60-
console.log();
61-
console.log(printFunction(fn));
62-
}
6353
/**
6454
* Part 1: Infer mutable ranges for values. We build an abstract model of
6555
* values, the alias/capture edges between them, and the set of mutations.
@@ -115,20 +105,6 @@ export function inferMutationAliasingRanges(
115105
seenBlocks.add(block.id);
116106

117107
for (const instr of block.instructions) {
118-
if (
119-
instr.value.kind === 'FunctionExpression' ||
120-
instr.value.kind === 'ObjectMethod'
121-
) {
122-
state.create(instr.lvalue, {
123-
kind: 'Function',
124-
function: instr.value.loweredFunc.func,
125-
});
126-
} else {
127-
for (const lvalue of eachInstructionLValue(instr)) {
128-
state.create(lvalue, {kind: 'Object'});
129-
}
130-
}
131-
132108
if (instr.effects == null) continue;
133109
for (const effect of instr.effects) {
134110
if (effect.kind === 'Create') {
@@ -141,6 +117,15 @@ export function inferMutationAliasingRanges(
141117
} else if (effect.kind === 'CreateFrom') {
142118
state.createFrom(index++, effect.from, effect.into);
143119
} else if (effect.kind === 'Assign') {
120+
/**
121+
* TODO: Invariant that the node is not initialized yet
122+
*
123+
* InferFunctionExpressionAliasingEffectSignatures currently infers
124+
* Assign effects in some places that should be Alias, leading to
125+
* Assign effects that reinitialize a value. The end result appears to
126+
* be fine, but we should fix that inference pass so that we add the
127+
* invariant here.
128+
*/
144129
if (!state.nodes.has(effect.into.identifier)) {
145130
state.create(effect.into, {kind: 'Object'});
146131
}
@@ -216,10 +201,6 @@ export function inferMutationAliasingRanges(
216201
}
217202
}
218203

219-
if (VERBOSE) {
220-
console.log(state.debug());
221-
console.log(pretty(mutations));
222-
}
223204
for (const mutation of mutations) {
224205
state.mutate(
225206
mutation.index,
@@ -234,9 +215,6 @@ export function inferMutationAliasingRanges(
234215
for (const render of renders) {
235216
state.render(render.index, render.place.identifier, errors);
236217
}
237-
if (DEBUG) {
238-
console.log(pretty([...state.nodes.keys()]));
239-
}
240218
fn.aliasingEffects ??= [];
241219
for (const param of [...fn.context, ...fn.params]) {
242220
const place = param.kind === 'Identifier' ? param : param.place;
@@ -458,9 +436,6 @@ export function inferMutationAliasingRanges(
458436
}
459437
}
460438

461-
if (VERBOSE) {
462-
console.log(printFunction(fn));
463-
}
464439
return errors.asResult();
465440
}
466441

@@ -511,11 +486,6 @@ class AliasingState {
511486
const fromNode = this.nodes.get(from.identifier);
512487
const toNode = this.nodes.get(into.identifier);
513488
if (fromNode == null || toNode == null) {
514-
if (VERBOSE) {
515-
console.log(
516-
`skip: createFrom ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`,
517-
);
518-
}
519489
return;
520490
}
521491
fromNode.edges.push({index, node: into.identifier, kind: 'alias'});
@@ -528,11 +498,6 @@ class AliasingState {
528498
const fromNode = this.nodes.get(from.identifier);
529499
const toNode = this.nodes.get(into.identifier);
530500
if (fromNode == null || toNode == null) {
531-
if (VERBOSE) {
532-
console.log(
533-
`skip: capture ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`,
534-
);
535-
}
536501
return;
537502
}
538503
fromNode.edges.push({index, node: into.identifier, kind: 'capture'});
@@ -545,11 +510,6 @@ class AliasingState {
545510
const fromNode = this.nodes.get(from.identifier);
546511
const toNode = this.nodes.get(into.identifier);
547512
if (fromNode == null || toNode == null) {
548-
if (VERBOSE) {
549-
console.log(
550-
`skip: assign ${printPlace(from)}${!!fromNode} -> ${printPlace(into)}${!!toNode}`,
551-
);
552-
}
553513
return;
554514
}
555515
fromNode.edges.push({index, node: into.identifier, kind: 'alias'});
@@ -604,11 +564,6 @@ class AliasingState {
604564
loc: SourceLocation,
605565
errors: CompilerError,
606566
): void {
607-
if (DEBUG) {
608-
console.log(
609-
`mutate ix=${index} start=$${start.id} end=[${end}]${transitive ? ' transitive' : ''} kind=${kind}`,
610-
);
611-
}
612567
const seen = new Set<Identifier>();
613568
const queue: Array<{
614569
place: Identifier;
@@ -623,18 +578,8 @@ class AliasingState {
623578
seen.add(current);
624579
const node = this.nodes.get(current);
625580
if (node == null) {
626-
if (DEBUG) {
627-
console.log(
628-
`no node! ${printIdentifier(start)} for identifier ${printIdentifier(current)}`,
629-
);
630-
}
631581
continue;
632582
}
633-
if (DEBUG) {
634-
console.log(
635-
` mutate $${node.id.id} transitive=${transitive} direction=${direction}`,
636-
);
637-
}
638583
node.id.mutableRange.end = makeInstructionId(
639584
Math.max(node.id.mutableRange.end, end),
640585
);
@@ -701,37 +646,5 @@ class AliasingState {
701646
}
702647
}
703648
}
704-
if (DEBUG) {
705-
const nodes = new Map();
706-
for (const id of seen) {
707-
const node = this.nodes.get(id);
708-
nodes.set(id.id, node);
709-
}
710-
console.log(pretty(nodes));
711-
}
712649
}
713-
714-
debug(): string {
715-
return pretty(this.nodes);
716-
}
717-
}
718-
719-
export function pretty(v: any): string {
720-
return prettyFormat(v, {
721-
plugins: [
722-
{
723-
test: v =>
724-
v !== null && typeof v === 'object' && v.kind === 'Identifier',
725-
serialize: v => printPlace(v),
726-
},
727-
{
728-
test: v =>
729-
v !== null &&
730-
typeof v === 'object' &&
731-
typeof v.declarationId === 'number',
732-
serialize: v =>
733-
`${printIdentifier(v)}:${v.mutableRange.start}:${v.mutableRange.end}`,
734-
},
735-
],
736-
});
737650
}

0 commit comments

Comments
 (0)