Repository: facebook/react
Pull request: [compiler] Preserve Create effects, guarantee effects initialize once


Ensures that effects are well-formed with respect to the rules:
* For a given instruction, each place is only initialized once (w one of Create, CreateFrom, Assign)
* Ensures that Alias targets are already initialized within the same instruction (should have a Create before them)
* Preserves Create and similar instructions
* Avoids duplicate instructions when inferring effects of function expressions

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33558).
* #33571
* __->__ #33558
* #33547

Changed files:
filename: compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts
patch: @@ -20,10 +20,11 @@ import {inferReactiveScopeVariables} from '../ReactiveScopes';
 import {rewriteInstructionKindsBasedOnReassignment} from '../SSA';
 import {inferMutableRanges} from './InferMutableRanges';
 import inferReferenceEffects from './InferReferenceEffects';
-import {assertExhaustive} from '../Utils/utils';
+import {assertExhaustive, retainWhere} from '../Utils/utils';
 import {inferMutationAliasingEffects} from './InferMutationAliasingEffects';
 import {inferFunctionExpressionAliasingEffectsSignature} from './InferFunctionExpressionAliasingEffectsSignature';
 import {inferMutationAliasingRanges} from './InferMutationAliasingRanges';
+import {hashEffect} from './AliasingEffects';
 
 export default function analyseFunctions(func: HIRFunction): void {
   for (const [_, block] of func.body.blocks) {
@@ -81,6 +82,17 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
     fn.aliasingEffects ??= [];
     fn.aliasingEffects?.push(...effects);
   }
+  if (fn.aliasingEffects != null) {
+    const seen = new Set<string>();
+    retainWhere(fn.aliasingEffects, effect => {
+      const hash = hashEffect(effect);
+      if (seen.has(hash)) {
+        return false;
+      }
+      seen.add(hash);
+      return true;
+    });
+  }
 
   /**
    * Phase 2: populate the Effect of each context variable to use in inferring

filename: compiler/packages/babel-plugin-react-compiler/src/Inference/InferMutationAliasingEffects.ts
patch: @@ -362,6 +362,11 @@ function inferBlock(
   } else if (terminal.kind === 'maybe-throw') {
     const handlerParam = context.catchHandlers.get(terminal.handler);
     if (handlerParam != null) {
+      CompilerError.invariant(state.kind(handlerParam) != null, {
+        reason:
+          'Expected catch binding to be intialized with a DeclareLocal Catch instruction',
+        loc: terminal.loc,
+      });
       const effects: Array<AliasingEffect> = [];
       for (const instr of block.instructions) {
         if (
@@ -476,14 +481,14 @@ function applySignature(
    * Track which values we've already aliased once, so that we can switch to
    * appendAlias() for subsequent aliases into the same value
    */
-  const aliased = new Set<IdentifierId>();
+  const initialized = new Set<IdentifierId>();
 
   if (DEBUG) {
     console.log(printInstruction(instruction));
   }
 
   for (const effect of signature.effects) {
-    applyEffect(context, state, effect, aliased, effects);
+    applyEffect(context, state, effect, initialized, effects);
   }
   if (DEBUG) {
     console.log(
@@ -508,7 +513,7 @@ function applyEffect(
   context: Context,
   state: InferenceState,
   _effect: AliasingEffect,
-  aliased: Set<IdentifierId>,
+  initialized: Set<IdentifierId>,
   effects: Array<AliasingEffect>,
 ): void {
   const effect = context.internEffect(_effect);
@@ -524,6 +529,13 @@ function applyEffect(
       break;
     }
     case 'Create': {
+      CompilerError.invariant(!initialized.has(effect.into.identifier.id), {
+        reason: `Cannot re-initialize variable within an instruction`,
+        description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`,
+        loc: effect.into.loc,
+      });
+      initialized.add(effect.into.identifier.id);
+
       let value = context.effectInstructionValueCache.get(effect);
       if (value == null) {
         value = {
@@ -538,6 +550,7 @@ function applyEffect(
         reason: new Set([effect.reason]),
       });
       state.define(effect.into, value);
+      effects.push(effect);
       break;
     }
     case 'ImmutableCapture': {
@@ -555,6 +568,13 @@ function applyEffect(
       break;
     }
     case 'CreateFrom': {
+      CompilerError.invariant(!initialized.has(effect.into.identifier.id), {
+        reason: `Cannot re-initialize variable within an instruction`,
+        description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`,
+        loc: effect.into.loc,
+      });
+      initialized.add(effect.into.identifier.id);
+
       const fromValue = state.kind(effect.from);
       let value = context.effectInstructionValueCache.get(effect);
       if (value == null) {
@@ -573,10 +593,21 @@ function applyEffect(
       switch (fromValue.kind) {
         case ValueKind.Primitive:
         case ValueKind.Global: {
-          // no need to track this data flow
+          effects.push({
+            kind: 'Create',
+            value: fromValue.kind,
+            into: effect.into,
+            reason: [...fromValue.reason][0] ?? ValueReason.Other,
+          });
           break;
         }
         case ValueKind.Frozen: {
+          effects.push({
+            kind: 'Create',
+            value: fromValue.kind,
+            into: effect.into,
+            reason: [...fromValue.reason][0] ?? ValueReason.Other,
+          });
           applyEffect(
             context,
             state,
@@ -585,7 +616,7 @@ function applyEffect(
               from: effect.from,
               into: effect.into,
             },
-            aliased,
+            initialized,
             effects,
           );
           break;
@@ -597,6 +628,13 @@ function applyEffect(
       break;
     }
     case 'CreateFunction': {
+      CompilerError.invariant(!initialized.has(effect.into.identifier.id), {
+        reason: `Cannot re-initialize variable within an instruction`,
+        description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`,
+        loc: effect.into.loc,
+      });
+      initialized.add(effect.into.identifier.id);
+
       effects.push(effect);
       /**
        * We consider the function mutable if it has any mutable context variables or
@@ -653,14 +691,22 @@ function applyEffect(
             from: capture,
             into: effect.into,
           },
-          aliased,
+          initialized,
           effects,
         );
       }
       break;
     }
     case 'Alias':
     case 'Capture': {
+      CompilerError.invariant(
+        effect.kind === 'Capture' || initialized.has(effect.into.identifier.id),
+        {
+          reason: `Expected destination value to already be initialized within this instruction for Alias effect`,
+          description: `Destination ${printPlace(effect.into)} is not initialized in this instruction`,
+          loc: effect.into.loc,
+        },
+      );
       /*
        * Capture describes potential information flow: storing a pointer to one value
        * within another. If the destination is not mutable, or the source value has
@@ -698,7 +744,7 @@ function applyEffect(
               from: effect.from,
               into: effect.into,
             },
-            aliased,
+            initialized,
             effects,
           );
           break;
@@ -714,6 +760,13 @@ function applyEffect(
       break;
     }
     case 'Assign': {
+      CompilerError.invariant(!initialized.has(effect.into.identifier.id), {
+        reason: `Cannot re-initialize variable within an instruction`,
+        description: `Re-initialized ${printPlace(effect.into)} in ${printAliasingEffect(effect)}`,
+        loc: effect.into.loc,
+      });
+      initialized.add(effect.into.identifier.id);
+
       /*
        * Alias represents potential pointer aliasing. If the type is a global,
        * a primitive (copy-on-write semantics) then we can prune the effect
@@ -730,7 +783,7 @@ function applyEffect(
               from: effect.from,
               into: effect.into,
             },
-            aliased,
+            initialized,
             effects,
           );
           let value = context.effectInstructionValueCache.get(effect);
@@ -768,12 +821,7 @@ function applyEffect(
           break;
         }
         default: {
-          if (aliased.has(effect.into.identifier.id)) {
-            state.appendAlias(effect.into, effect.from);
-          } else {
-            aliased.add(effect.into.identifier.id);
-            state.alias(effect.into, effect.from);
-          }
+          state.assign(effect.into, effect.from);
           effects.push(effect);
           break;
         }
@@ -826,11 +874,11 @@ function applyEffect(
             context,
             state,
             {kind: 'MutateTransitiveConditionally', value: effect.function},
-            aliased,
+            initialized,
             effects,
           );
           for (const signatureEffect of signatureEffects) {
-            applyEffect(context, state, signatureEffect, aliased, effects);
+            applyEffect(context, state, signatureEffect, initialized, effects);
           }
           break;
         }
@@ -858,7 +906,7 @@ function applyEffect(
           console.log('apply aliasing signature effects');
         }
         for (const signatureEffect of signatureEffects) {
-          applyEffect(context, state, signatureEffect, aliased, effects);
+          applyEffect(context, state, signatureEffect, initialized, effects);
         }
       } else if (effect.signature != null) {
         if (DEBUG) {
@@ -873,7 +921,7 @@ function applyEffect(
           effect.loc,
         );
         for (const legacyEffect of legacyEffects) {
-          applyEffect(context, state, legacyEffect, aliased, effects);
+          applyEffect(context, state, legacyEffect, initialized, effects);
         }
       } else {
         if (DEBUG) {
@@ -888,7 +936,7 @@ function applyEffect(
             value: ValueKind.Mutable,
             reason: ValueReason.Other,
           },
-          aliased,
+          initialized,
           effects,
         );
         /*
@@ -911,21 +959,21 @@ function applyEffect(
                 kind: 'MutateTransitiveConditionally',
                 value: operand,
               },
-              aliased,
+              initialized,
               effects,
             );
           }
           const mutateIterator =
             arg.kind === 'Spread' ? conditionallyMutateIterator(operand) : null;
           if (mutateIterator) {
-            applyEffect(context, state, mutateIterator, aliased, effects);
+            applyEffect(context, state, mutateIterator, initialized, effects);
           }
           applyEffect(
             context,
             state,
             // OK: recording information flow
             {kind: 'Alias', from: operand, into: effect.into},
-            aliased,
+            initialized,
             effects,
           );
           for (const otherArg of [
@@ -953,7 +1001,7 @@ function applyEffect(
                 from: operand,
                 into: other,
               },
-              aliased,
+              initialized,
               effects,
             );
           }
@@ -1009,7 +1057,7 @@ function applyEffect(
                   suggestions: null,
                 },
               },
-              aliased,
+              initialized,
               effects,
             );
           }
@@ -1028,7 +1076,7 @@ function applyEffect(
                 suggestions: null,
               },
             },
-            aliased,
+            initialized,
             effects,
           );
         } else {
@@ -1059,7 +1107,7 @@ function applyEffect(
                 suggestions: null,
               },
             },
-            aliased,
+            initialized,
             effects,
           );
         }
@@ -1166,7 +1214,7 @@ class InferenceState {
   }
 
   // Updates the value at @param place to point to the same value as @param value.
-  alias(place: Place, value: Place): void {
+  assign(place: Place, value: Place): void {
     const values = this.#variables.get(value.identifier.id);
     CompilerError.invariant(values != null, {
       reason: `[InferMutationAliasingEffects] Expected value for identifier to be initialized`,