Repository: facebook/react
Pull request: [compiler] Cleanup debugging code


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

Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
patch: @@ -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<Place | SpreadPattern | Hole> = [];
@@ -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]});

filename: compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingRanges.ts
patch: @@ -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<void, CompilerError> {
-  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<Identifier>();
     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}`,
-      },
-    ],
-  });
 }