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

ImmutableCapture allows us to record information flow for escape analysis purposes, without impacting mutable range analysis. All of Alias, Capture, and CreateFrom now downgrade to ImmutableCapture if the `from` value is frozen. Globals and primitives are conceptually copy types and don't record any information flow at all. I guess we could add a `Copy` effect if we wanted but i haven't seen a need for it yet.

Related, previously CreateFrom was always paired with Capture when constructing effects. But I had also coded up the inference to sort of treat them the same. So this diff makes that official, and means CreateFrom is a valid third way to represent data flow and not just creation. When applied, CreateFrom will turn into: ImmutableCapture for frozen values, Capture for mutable values, and disappear for globals/primitives.

A final tweak is to change range inference to ensure that range.start is non-zero if the value is mutated.

ghstack-source-id: fef144d7fb
Pull Request resolved: https://github.com/facebook/react/pull/33367

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 @@ import {
   Place,
   isRefOrRefValue,
   makeInstructionId,
+  printFunction,
 } from '../HIR';
 import {deadCodeElimination} from '../Optimization';
 import {inferReactiveScopeVariables} from '../ReactiveScopes';
@@ -26,10 +27,7 @@ import {
   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 @@ export default function analyseFunctions(func: HIRFunction): void {
             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 |
 ```

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.expect.md

@@ -0,0 +1,58 @@
+
+## Input
+
+```javascript
+// @enableNewMutationAliasingModel
+function Component({a, b}) {
+  const x = {a};
+  const y = [b];
+  const f = () => {
+    y.x = x;
+    mutate(y);
+  };
+  return <div onClick={f}>{x}</div>;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel
+function Component(t0) {
+  const $ = _c(7);
+  const { a, b } = t0;
+  let t1;
+  let x;
+  if ($[0] !== a || $[1] !== b) {
+    x = { a };
+    const y = [b];
+    t1 = () => {
+      y.x = x;
+      mutate(y);
+    };
+    $[0] = a;
+    $[1] = b;
+    $[2] = t1;
+    $[3] = x;
+  } else {
+    t1 = $[2];
+    x = $[3];
+  }
+  const f = t1;
+  let t2;
+  if ($[4] !== f || $[5] !== x) {
+    t2 = <div onClick={f}>{x}</div>;
+    $[4] = f;
+    $[5] = x;
+    $[6] = t2;
+  } else {
+    t2 = $[6];
+  }
+  return t2;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation-via-function-expression.js

@@ -0,0 +1,11 @@
+// @enableNewMutationAliasingModel
+function Component({a, b}) {
+  const x = {a};
+  const y = [b];
+  const f = () => {
+    y.x = x;
+    mutate(y);
+  };
+  f();
+  return <div>{x}</div>;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.expect.md

@@ -0,0 +1,42 @@
+
+## Input
+
+```javascript
+// @enableNewMutationAliasingModel
+function Component({a, b}) {
+  const x = {a};
+  const y = [b];
+  y.x = x;
+  mutate(y);
+  return <div>{x}</div>;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel
+function Component(t0) {
+  const $ = _c(3);
+  const { a, b } = t0;
+  let t1;
+  if ($[0] !== a || $[1] !== b) {
+    const x = { a };
+    const y = [b];
+    y.x = x;
+    mutate(y);
+    t1 = <div>{x}</div>;
+    $[0] = a;
+    $[1] = b;
+    $[2] = t1;
+  } else {
+    t1 = $[2];
+  }
+  return t1;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/basic-mutation.js

@@ -0,0 +1,8 @@
+// @enableNewMutationAliasingModel
+function Component({a, b}) {
+  const x = {a};
+  const y = [b];
+  y.x = x;
+  mutate(y);
+  return <div>{x}</div>;
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.expect.md

@@ -0,0 +1,59 @@
+
+## Input
+
+```javascript
+// @enableNewMutationAliasingModel
+function Component({a, b, c}) {
+  const x = [a, b];
+  const f = () => {
+    maybeMutate(x);
+    // different dependency to force this not to merge with x's scope
+    console.log(c);
+  };
+  return <Foo onClick={f} value={x} />;
+}
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @enableNewMutationAliasingModel
+function Component(t0) {
+  const $ = _c(8);
+  const { a, b, c } = t0;
+  let t1;
+  let x;
+  if ($[0] !== a || $[1] !== b || $[2] !== c) {
+    x = [a, b];
+    t1 = () => {
+      maybeMutate(x);
+
+      console.log(c);
+    };
+    $[0] = a;
+    $[1] = b;
+    $[2] = c;
+    $[3] = t1;
+    $[4] = x;
+  } else {
+    t1 = $[3];
+    x = $[4];
+  }
+  const f = t1;
+  let t2;
+  if ($[5] !== f || $[6] !== x) {
+    t2 = <Foo onClick={f} value={x} />;
+    $[5] = f;
+    $[6] = x;
+    $[7] = t2;
+  } else {
+    t2 = $[7];
+  }
+  return t2;
+}
+
+```
+      
+### Eval output
+(kind: exception) Fixture not implemented

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/new-mutability/potential-mutation-in-function-expression.js

@@ -0,0 +1,10 @@
+// @enableNewMutationAliasingModel
+function Component({a, b, c}) {
+  const x = [a, b];
+  const f = () => {
+    maybeMutate(x);
+    // different dependency to force this not to merge with x's scope
+    console.log(c);
+  };
+  return <Foo onClick={f} value={x} />;
+}