[compiler] Clean up deadcode: ReactiveFunctionValue

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

@@ -108,16 +108,7 @@ export type ReactiveValue =
   | ReactiveLogicalValue
   | ReactiveSequenceValue
   | ReactiveTernaryValue
-  | ReactiveOptionalCallValue
-  | ReactiveFunctionValue;
-
-export type ReactiveFunctionValue = {
-  kind: 'ReactiveFunctionValue';
-  fn: ReactiveFunction;
-  dependencies: Array<Place>;
-  returnType: t.FlowType | t.TSType | null;
-  loc: SourceLocation;
-};
+  | ReactiveOptionalCallValue;
 
 export type ReactiveLogicalValue = {
   kind: 'LogicalExpression';

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

@@ -6,7 +6,6 @@
  */
 
 import generate from '@babel/generator';
-import {printReactiveFunction} from '..';
 import {CompilerError} from '../CompilerError';
 import {printReactiveScopeSummary} from '../ReactiveScopes/PrintReactiveFunction';
 import DisjointSet from '../Utils/DisjointSet';
@@ -701,10 +700,6 @@ export function printInstructionValue(instrValue: ReactiveValue): string {
       value = `FinishMemoize decl=${printPlace(instrValue.decl)}`;
       break;
     }
-    case 'ReactiveFunctionValue': {
-      value = `FunctionValue ${printReactiveFunction(instrValue.fn)}`;
-      break;
-    }
     default: {
       assertExhaustive(
         instrValue,

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

@@ -10,35 +10,16 @@ import {
   Effect,
   HIRFunction,
   Identifier,
-  IdentifierId,
   LoweredFunction,
   isRefOrRefValue,
   makeInstructionId,
 } from '../HIR';
 import {deadCodeElimination} from '../Optimization';
 import {inferReactiveScopeVariables} from '../ReactiveScopes';
 import {rewriteInstructionKindsBasedOnReassignment} from '../SSA';
-import {inferMutableContextVariables} from './InferMutableContextVariables';
 import {inferMutableRanges} from './InferMutableRanges';
 import inferReferenceEffects from './InferReferenceEffects';
 
-// Helper class to track indirections such as LoadLocal and PropertyLoad.
-export class IdentifierState {
-  properties: Map<IdentifierId, Identifier> = new Map();
-
-  resolve(identifier: Identifier): Identifier {
-    const resolved = this.properties.get(identifier.id);
-    if (resolved !== undefined) {
-      return resolved;
-    }
-    return identifier;
-  }
-
-  alias(lvalue: Identifier, value: Identifier): void {
-    this.properties.set(lvalue.id, this.properties.get(value.id) ?? value);
-  }
-}
-
 export default function analyseFunctions(func: HIRFunction): void {
   for (const [_, block] of func.body.blocks) {
     for (const instr of block.instructions) {
@@ -78,7 +59,6 @@ function lower(func: HIRFunction): void {
 }
 
 function infer(loweredFunc: LoweredFunction): void {
-  const knownMutated = inferMutableContextVariables(loweredFunc.func);
   for (const operand of loweredFunc.func.context) {
     const identifier = operand.identifier;
     CompilerError.invariant(operand.effect === Effect.Unknown, {
@@ -95,10 +75,11 @@ function infer(loweredFunc: LoweredFunction): void {
        * render
        */
       operand.effect = Effect.Capture;
-    } else if (knownMutated.has(operand)) {
-      operand.effect = Effect.Mutate;
     } else if (isMutatedOrReassigned(identifier)) {
-      // Note that this also reflects if identifier is ConditionallyMutated
+      /**
+       * Reflects direct reassignments, PropertyStores, and ConditionallyMutate
+       * (directly or through maybe-aliases)
+       */
       operand.effect = Effect.Capture;
     } else {
       operand.effect = Effect.Read;

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

@@ -2231,7 +2231,6 @@ function codegenInstructionValue(
       );
       break;
     }
-    case 'ReactiveFunctionValue':
     case 'StartMemoize':
     case 'FinishMemoize':
     case 'Debugger':

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

@@ -772,14 +772,6 @@ function computeMemoizationInputs(
         rvalues: operands,
       };
     }
-    case 'ReactiveFunctionValue': {
-      CompilerError.invariant(false, {
-        reason: `Unexpected ReactiveFunctionValue node`,
-        description: null,
-        loc: value.loc,
-        suggestions: null,
-      });
-    }
     case 'UnsupportedNode': {
       CompilerError.invariant(false, {
         reason: `Unexpected unsupported node`,

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

@@ -73,15 +73,6 @@ export class ReactiveFunctionVisitor<TState = void> {
         this.visitValue(value.id, value.value, state);
         break;
       }
-      case 'ReactiveFunctionValue': {
-        this.visitReactiveFunctionValue(
-          id,
-          value.dependencies,
-          value.fn,
-          state,
-        );
-        break;
-      }
       default: {
         for (const place of eachInstructionValueOperand(value)) {
           this.visitPlace(id, place, state);
@@ -434,18 +425,6 @@ export class ReactiveFunctionTransform<
         }
         break;
       }
-      case 'ReactiveFunctionValue': {
-        const nextValue = this.transformReactiveFunctionValue(
-          id,
-          value.dependencies,
-          value.fn,
-          state,
-        );
-        if (nextValue.kind === 'replace') {
-          value.fn = nextValue.value;
-        }
-        break;
-      }
       default: {
         for (const place of eachInstructionValueOperand(value)) {
           this.visitPlace(id, place, state);
@@ -619,10 +598,6 @@ export function* eachReactiveValueOperand(
       yield* eachReactiveValueOperand(instrValue.alternate);
       break;
     }
-    case 'ReactiveFunctionValue': {
-      yield* instrValue.dependencies;
-      break;
-    }
     default: {
       yield* eachInstructionValueOperand(instrValue);
     }

compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts

@@ -327,13 +327,6 @@ class Visitor extends ReactiveFunctionVisitor<VisitorState> {
       case 'OptionalExpression': {
         return this.recordDepsInValue(value.value, state);
       }
-      case 'ReactiveFunctionValue': {
-        CompilerError.throwTodo({
-          reason:
-            'Handle ReactiveFunctionValue in ValidatePreserveManualMemoization',
-          loc: value.loc,
-        });
-      }
       case 'ConditionalExpression': {
         this.recordDepsInValue(value.test, state);
         this.recordDepsInValue(value.consequent, state);

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/allow-mutate-global-in-effect-fixpoint.expect.md

@@ -19,9 +19,14 @@ function Component() {
 
   // capture into a separate variable that is not a context variable.
   const y = x;
+  /**
+   * Note that this fixture currently produces a stale effect closure if `y = x
+   * = someGlobal` changes between renders. Under current compiler assumptions,
+   * that would be a rule of react violation.
+   */
   useEffect(() => {
     y.value = 'hello';
-  }, []);
+  });
 
   useEffect(() => {
     setState(someGlobal.value);
@@ -46,57 +51,50 @@ import { useEffect, useState } from "react";
 let someGlobal = { value: null };
 
 function Component() {
-  const $ = _c(7);
+  const $ = _c(5);
   const [state, setState] = useState(someGlobal);
+
+  let x = someGlobal;
+  while (x == null) {
+    x = someGlobal;
+  }
+
+  const y = x;
   let t0;
-  let t1;
-  let t2;
   if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
-    let x = someGlobal;
-    while (x == null) {
-      x = someGlobal;
-    }
-
-    const y = x;
-    t0 = useEffect;
-    t1 = () => {
+    t0 = () => {
       y.value = "hello";
     };
-    t2 = [];
     $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  useEffect(t0);
+  let t1;
+  let t2;
+  if ($[1] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = () => {
+      setState(someGlobal.value);
+    };
+    t2 = [someGlobal];
     $[1] = t1;
     $[2] = t2;
   } else {
-    t0 = $[0];
     t1 = $[1];
     t2 = $[2];
   }
-  t0(t1, t2);
-  let t3;
+  useEffect(t1, t2);
+
+  const t3 = String(state);
   let t4;
-  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
-    t3 = () => {
-      setState(someGlobal.value);
-    };
-    t4 = [someGlobal];
+  if ($[3] !== t3) {
+    t4 = <div>{t3}</div>;
     $[3] = t3;
     $[4] = t4;
   } else {
-    t3 = $[3];
     t4 = $[4];
   }
-  useEffect(t3, t4);
-
-  const t5 = String(state);
-  let t6;
-  if ($[5] !== t5) {
-    t6 = <div>{t5}</div>;
-    $[5] = t5;
-    $[6] = t6;
-  } else {
-    t6 = $[6];
-  }
-  return t6;
+  return t4;
 }
 
 export const FIXTURE_ENTRYPOINT = {