[compiler] comments and todos

ghstack-source-id: 851743f0d6
Pull Request resolved: https://github.com/facebook/react/pull/33376

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<Identifier> {
   // Infer mutable ranges for non fields
@@ -105,7 +105,8 @@ export function debugAliases(aliases: DisjointSet<Identifier>): void {
         .buildSets()
         .map(set =>
           [...set].map(
-            ident => printIdentifier(ident) + printMutableRange(ident),
+            ident =>
+              `${printIdentifier(ident)}:${ident.mutableRange.start}:${ident.mutableRange.end}`,
           ),
         ),
     ),

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(

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<Identifier>();
   for (const block of fn.body.blocks.values()) {