[compiler] Add ImmutableCapture effect, CreateFrom no longer needs Capture

compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts

@@ -949,10 +949,13 @@ function getFunctionName(
 export function printAliasingEffect(effect: AliasingEffect): string {
   switch (effect.kind) {
     case 'Alias': {
-      return `Alias ${printPlaceForAliasEffect(effect.from)} -> ${printPlaceForAliasEffect(effect.into)}`;
+      return `Alias ${printPlaceForAliasEffect(effect.into)} = ${printPlaceForAliasEffect(effect.from)}`;
     }
     case 'Capture': {
-      return `Capture ${printPlaceForAliasEffect(effect.from)} -> ${printPlaceForAliasEffect(effect.into)}`;
+      return `Capture ${printPlaceForAliasEffect(effect.into)} <- ${printPlaceForAliasEffect(effect.from)}`;
+    }
+    case 'ImmutableCapture': {
+      return `ImmutableCapture ${printPlaceForAliasEffect(effect.into)} <- ${printPlaceForAliasEffect(effect.from)}`;
     }
     case 'Create': {
       return `Create ${printPlaceForAliasEffect(effect.into)} = ${effect.value}`;

compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts

@@ -14,6 +14,7 @@
   Place,
   isRefOrRefValue,
   makeInstructionId,
+  printFunction,
 } from '../HIR';
 import {deadCodeElimination} from '../Optimization';
 import {inferReactiveScopeVariables} from '../ReactiveScopes';
@@ -26,10 +27,7 @@
   eachInstructionValueOperand,
 } from '../HIR/visitors';
 import {Iterable_some} from '../Utils/utils';
-import {
-  AliasingEffect,
-  inferMutationAliasingEffects,
-} from './InferMutationAliasingEffects';
+import {inferMutationAliasingEffects} from './InferMutationAliasingEffects';
 import {inferMutationAliasingFunctionEffects} from './InferMutationAliasingFunctionEffects';
 import {inferMutationAliasingRanges} from './InferMutationAliasingRanges';
 
@@ -44,7 +42,6 @@
             infer(instr.value.loweredFunc, aliases);
           } else {
             lowerWithMutationAliasing(instr.value.loweredFunc.func);
-            infer(instr.value.loweredFunc, new DisjointSet());
           }
 
           /**

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

@@ -38,7 +38,9 @@ import {
   Set_isSuperset,
 } from '../Utils/utils';
 import {
+  printAliasingEffect,
   printIdentifier,
+  printInstruction,
   printInstructionValue,
   printPlace,
   printSourceLocation,
@@ -258,6 +260,23 @@ function applySignature(
         state.define(effect.into, value);
         break;
       }
+      case 'ImmutableCapture': {
+        let value = effectInstructionValueCache.get(effect);
+        if (value == null) {
+          value = {
+            kind: 'ObjectExpression',
+            properties: [],
+            loc: effect.into.loc,
+          };
+          effectInstructionValueCache.set(effect, value);
+        }
+        state.initialize(value, {
+          kind: ValueKind.Frozen,
+          reason: new Set([ValueReason.Other]),
+        });
+        state.define(effect.into, value);
+        break;
+      }
       case 'CreateFrom': {
         const kind = state.kind(effect.from).kind;
         let value = effectInstructionValueCache.get(effect);
@@ -274,6 +293,29 @@ function applySignature(
           reason: new Set([ValueReason.Other]),
         });
         state.define(effect.into, value);
+        switch (kind) {
+          case ValueKind.Primitive:
+          case ValueKind.Global: {
+            // no need to track this data flow
+            break;
+          }
+          case ValueKind.Frozen: {
+            effects.push({
+              kind: 'ImmutableCapture',
+              from: effect.from,
+              into: effect.into,
+            });
+            break;
+          }
+          default: {
+            // TODO: should this be Alias??
+            effects.push({
+              kind: 'Capture',
+              from: effect.from,
+              into: effect.into,
+            });
+          }
+        }
         break;
       }
       case 'Capture': {
@@ -297,27 +339,28 @@ function applySignature(
           }
         }
         const fromKind = state.kind(effect.from).kind;
-        let isCopyByReferenceValue: boolean;
+        let isMutableReferenceType: boolean;
         switch (fromKind) {
           case ValueKind.Global:
           case ValueKind.Primitive: {
-            isCopyByReferenceValue = false;
+            isMutableReferenceType = false;
             break;
           }
           case ValueKind.Frozen: {
-            /*
-             * TODO: add a separate "ImmutableAlias" effect to downgrade to, that doesn't impact mutable ranges
-             * We want to remember that the data flow occurred for PruneNonEscapingScopes
-             */
-            isCopyByReferenceValue = false;
+            isMutableReferenceType = false;
+            effects.push({
+              kind: 'ImmutableCapture',
+              from: effect.from,
+              into: effect.into,
+            });
             break;
           }
           default: {
-            isCopyByReferenceValue = true;
+            isMutableReferenceType = true;
             break;
           }
         }
-        if (isMutableDesination && isCopyByReferenceValue) {
+        if (isMutableDesination && isMutableReferenceType) {
           effects.push(effect);
         }
         break;
@@ -329,12 +372,25 @@ function applySignature(
          */
         const fromKind = state.kind(effect.from).kind;
         switch (fromKind) {
-          /*
-           * TODO: add a separate "ImmutableAlias" effect to downgrade to, that doesn't impact mutable ranges
-           * We want to remember that the data flow occurred for PruneNonEscapingScopes
-           * (use this to replace the ValueKind.Frozen case)
-           */
-          case ValueKind.Frozen:
+          case ValueKind.Frozen: {
+            effects.push({
+              kind: 'ImmutableCapture',
+              from: effect.from,
+              into: effect.into,
+            });
+            let value = effectInstructionValueCache.get(effect);
+            if (value == null) {
+              value = {
+                kind: 'Primitive',
+                value: undefined,
+                loc: effect.from.loc,
+              };
+              effectInstructionValueCache.set(effect, value);
+            }
+            state.initialize(value, {kind: fromKind, reason: new Set([])});
+            state.define(effect.into, value);
+            break;
+          }
           case ValueKind.Global:
           case ValueKind.Primitive: {
             let value = effectInstructionValueCache.get(effect);
@@ -385,24 +441,7 @@ function applySignature(
       case 'MutateTransitiveConditionally': {
         const didMutate = state.mutate(effect.kind, effect.value);
         if (didMutate) {
-          switch (effect.kind) {
-            case 'Mutate': {
-              effects.push(effect);
-              break;
-            }
-            case 'MutateConditionally': {
-              effects.push({kind: 'Mutate', value: effect.value});
-              break;
-            }
-            case 'MutateTransitive': {
-              effects.push(effect);
-              break;
-            }
-            case 'MutateTransitiveConditionally': {
-              effects.push({kind: 'MutateTransitive', value: effect.value});
-              break;
-            }
-          }
+          effects.push(effect);
         }
         break;
       }
@@ -414,13 +453,19 @@ function applySignature(
       }
     }
   }
-  CompilerError.invariant(
-    state.isDefined(instruction.lvalue) && state.kind(instruction.lvalue),
-    {
+  if (
+    !(state.isDefined(instruction.lvalue) && state.kind(instruction.lvalue))
+  ) {
+    console.log(printInstruction(instruction));
+    console.log('input effects');
+    console.log(signature.effects.map(printAliasingEffect).join('\n'));
+    console.log('refined effects');
+    console.log(effects.map(printAliasingEffect).join('\n'));
+    CompilerError.invariant(false, {
       reason: `Expected instruction lvalue to be initialized`,
       loc: instruction.loc,
-    },
-  );
+    });
+  }
   return effects.length !== 0 ? effects : null;
 }
 
@@ -961,11 +1006,6 @@ function computeSignatureForInstruction(
         from: value.object,
         into: lvalue,
       });
-      effects.push({
-        kind: 'Capture',
-        from: value.object,
-        into: lvalue,
-      });
       break;
     }
     case 'PropertyStore':
@@ -1106,11 +1146,6 @@ function computeSignatureForInstruction(
           from: value.value,
           into: patternLValue,
         });
-        effects.push({
-          kind: 'Capture',
-          from: value.value,
-          into: patternLValue,
-        });
       }
       effects.push({kind: 'Alias', from: value.value, into: lvalue});
       break;
@@ -1267,6 +1302,7 @@ function computeEffectsForSignature(
         }
         break;
       }
+      case 'ImmutableCapture':
       case 'CreateFrom':
       case 'Apply':
       case 'Mutate':
@@ -1413,6 +1449,10 @@ export type AliasingEffect =
    * Creates a new value with the same kind as the starting value.
    */
   | {kind: 'CreateFrom'; from: Place; into: Place}
+  /**
+   * Immutable data flow, used for escape analysis. Does not influence mutable range analysis:
+   */
+  | {kind: 'ImmutableCapture'; from: Place; into: Place}
   /**
    * Calls the function at the given place with the given arguments either captured or aliased,
    * and captures/aliases the result into the given place.

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

@@ -54,7 +54,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
           effect.kind === 'Capture'
         ) {
           captures.union([effect.from.identifier, effect.into.identifier]);
-        } else if (effect.kind === 'MutateTransitive') {
+        } else if (
+          effect.kind === 'MutateTransitive' ||
+          effect.kind === 'MutateTransitiveConditionally'
+        ) {
           const value = effect.value;
           value.identifier.mutableRange.end = makeInstructionId(instr.id + 1);
         }
@@ -83,7 +86,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
       for (const effect of instr.effects) {
         if (effect.kind === 'Alias' || effect.kind === 'CreateFrom') {
           aliases.union([effect.from.identifier, effect.into.identifier]);
-        } else if (effect.kind === 'Mutate') {
+        } else if (
+          effect.kind === 'Mutate' ||
+          effect.kind === 'MutateConditionally'
+        ) {
           const value = effect.value;
           value.identifier.mutableRange.end = makeInstructionId(instr.id + 1);
         }
@@ -94,7 +100,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
 
   /**
    * Part 3
-   * Add legacy operand-specific effects based on instruction effects and mutable ranges
+   * Add legacy operand-specific effects based on instruction effects and mutable ranges.
+   * Also fixes up operand mutable ranges, making sure that start is non-zero if the value
+   * is mutated (depended on by later passes like InferReactiveScopeVariables which uses this
+   * to filter spurious mutations of globals, which we now guard against more precisely)
    */
   for (const block of fn.body.blocks.values()) {
     for (const phi of block.phis) {
@@ -136,6 +145,10 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
             }
             break;
           }
+          case 'ImmutableCapture': {
+            operandEffects.set(effect.from.identifier.id, Effect.Read);
+            break;
+          }
           case 'Create': {
             break;
           }
@@ -178,6 +191,12 @@ export function inferMutationAliasingRanges(fn: HIRFunction): void {
         lvalue.effect = effect;
       }
       for (const operand of eachInstructionValueOperand(instr.value)) {
+        if (
+          operand.identifier.mutableRange.end > instr.id &&
+          operand.identifier.mutableRange.start === 0
+        ) {
+          operand.identifier.mutableRange.start = instr.id;
+        }
         const effect = operandEffects.get(operand.identifier.id) ?? Effect.Read;
         operand.effect = effect;
       }

compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts

@@ -376,12 +376,12 @@ export function findDisjointMutableValues(
       } else {
         for (const operand of eachInstructionOperand(instr)) {
           if (
-            isMutable(instr, operand)
+            isMutable(instr, operand) &&
             /*
              * exclude global variables from being added to scopes, we can't recreate them!
              * TODO: improve handling of module-scoped variables and globals
              */
-            // && operand.identifier.mutableRange.start > 0
+            operand.identifier.mutableRange.start > 0
           ) {
             if (
               instr.value.kind === 'FunctionExpression' ||

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-repro-named-function-with-shadowed-local-same-name.expect.md

@@ -22,7 +22,7 @@ function Component(props) {
    7 |     return hasErrors;
    8 |   }
 >  9 |   return hasErrors();
-     |          ^^^^^^^^^ Invariant: [hoisting] Expected value for identifier to be initialized. hasErrors_0$14 (9:9)
+     |          ^^^^^^^^^ Invariant: [hoisting] Expected value for identifier to be initialized. hasErrors_0$15 (9:9)
   10 | }
   11 |
 ```