[compiler][newinference] Impure/Render effects

Adds two new effects:
- `Impure` is for builtin functions that are known to be impure and not safe to call during render. This is already implemented via an optional `impure` flag on function declarations and ValidateNoImpureFunctionsInRender. That validation still exists, but we can avoid the need for the special validation pass by relying on the function effect.
- `Render` is an effect that indicates a value is assumed to be called at render time, even if it would not normally appear to be called. Examples are JSX tags and children — if these values are functions, they are definitely called at render (tag) or almost certainly called in render (children). Any function that transitively flows into a Render effect must not have MutateGlobal or Impure effects. We can also extend this to include other APIs that should be pure, for example we could consider adding it to `useReducer()` callbacks or `useState()` initializers.

ghstack-source-id: 8367f46
Pull Request resolved: #33488

Author: josephsavona

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

@@ -416,6 +416,7 @@ addObject(BUILTIN_SHAPES, BuiltInArrayId, [
             into: signatureArgument(4),
             signature: null,
             mutatesFunction: false,
+            loc: GeneratedSource,
           },
           // captures the result of the callback into the return array
           {

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

@@ -1015,6 +1015,12 @@ export function printAliasingEffect(effect: AliasingEffect): string {
     case 'MutateGlobal': {
       return `MutateGlobal ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
     }
+    case 'Impure': {
+      return `Impure ${printPlaceForAliasEffect(effect.place)} reason=${JSON.stringify(effect.error.reason)}`;
+    }
+    case 'Render': {
+      return `Render ${printPlaceForAliasEffect(effect.place)}`;
+    }
     default: {
       assertExhaustive(effect, `Unexpected kind '${(effect as any).kind}'`);
     }

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

@@ -100,6 +100,8 @@ function lowerWithMutationAliasing(fn: HIRFunction): void {
         capturedOrMutated.add(effect.value.identifier.id);
         break;
       }
+      case 'Impure':
+      case 'Render':
       case 'MutateFrozen':
       case 'MutateGlobal':
       case 'CreateFunction':

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

@@ -10,6 +10,7 @@ import {
   CompilerErrorDetailOptions,
   Effect,
   ErrorSeverity,
+  SourceLocation,
   ValueKind,
 } from '..';
 import {
@@ -380,10 +381,7 @@ function applySignature(
               context: new Set(),
             });
             effects.push({
-              kind:
-                value.kind === ValueKind.Frozen
-                  ? 'MutateFrozen'
-                  : 'MutateGlobal',
+              kind: 'MutateFrozen',
               place: effect.value,
               error: {
                 severity: ErrorSeverity.InvalidReact,
@@ -546,7 +544,10 @@ function applyEffect(
       const hasTrackedSideEffects =
         effect.function.loweredFunc.func.aliasingEffects?.some(
           effect =>
-            effect.kind === 'MutateFrozen' || effect.kind === 'MutateGlobal',
+            // TODO; include "render" here?
+            effect.kind === 'MutateFrozen' ||
+            effect.kind === 'MutateGlobal' ||
+            effect.kind === 'Impure',
         );
       // For legacy compatibility
       const capturesRef = effect.function.loweredFunc.func.context.some(
@@ -721,6 +722,7 @@ function applyEffect(
           effect.receiver,
           effect.args,
           functionValues[0].loweredFunc.func.context,
+          effect.loc,
         );
         if (signatureEffects != null) {
           if (DEBUG) {
@@ -747,6 +749,8 @@ function applyEffect(
               effect.into,
               effect.receiver,
               effect.args,
+              [],
+              effect.loc,
             )
           : null;
       if (signatureEffects != null) {
@@ -766,6 +770,7 @@ function applyEffect(
           effect.into,
           effect.receiver,
           effect.args,
+          effect.loc,
         );
         for (const legacyEffect of legacyEffects) {
           applyEffect(context, state, legacyEffect, aliased, effects);
@@ -899,6 +904,8 @@ function applyEffect(
       }
       break;
     }
+    case 'Impure':
+    case 'Render':
     case 'MutateFrozen':
     case 'MutateGlobal': {
       effects.push(effect);
@@ -1452,6 +1459,7 @@ function computeSignatureForInstruction(
         args: value.args,
         into: lvalue,
         signature,
+        loc: value.loc,
       });
       break;
     }
@@ -1631,6 +1639,24 @@ function computeSignatureForInstruction(
           into: lvalue,
         });
       }
+      if (value.kind === 'JsxExpression') {
+        if (value.tag.kind === 'Identifier') {
+          // Tags are render function, by definition they're called during render
+          effects.push({
+            kind: 'Render',
+            place: value.tag,
+          });
+        }
+        if (value.children != null) {
+          // Children are typically called during render, not used as an event/effect callback
+          for (const child of value.children) {
+            effects.push({
+              kind: 'Render',
+              place: child,
+            });
+          }
+        }
+      }
       break;
     }
     case 'DeclareLocal': {
@@ -1861,6 +1887,7 @@ function computeEffectsForLegacySignature(
   lvalue: Place,
   receiver: Place,
   args: Array<Place | SpreadPattern | Hole>,
+  loc: SourceLocation,
 ): Array<AliasingEffect> {
   const returnValueReason = signature.returnValueReason ?? ValueReason.Other;
   const effects: Array<AliasingEffect> = [];
@@ -1870,6 +1897,23 @@ function computeEffectsForLegacySignature(
     value: signature.returnValueKind,
     reason: returnValueReason,
   });
+  if (signature.impure && state.env.config.validateNoImpureFunctionsInRender) {
+    effects.push({
+      kind: 'Impure',
+      place: receiver,
+      error: {
+        reason:
+          'Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent)',
+        description:
+          signature.canonicalName != null
+            ? `\`${signature.canonicalName}\` is an impure function whose results may change on every call`
+            : null,
+        severity: ErrorSeverity.InvalidReact,
+        loc,
+        suggestions: null,
+      },
+    });
+  }
   const stores: Array<Place> = [];
   const captures: Array<Place> = [];
   function visit(place: Place, effect: Effect): void {
@@ -2083,6 +2127,7 @@ function computeEffectsForSignature(
   args: Array<Place | SpreadPattern | Hole>,
   // Used for signatures constructed dynamically which reference context variables
   context: Array<Place> = [],
+  loc: SourceLocation,
 ): Array<AliasingEffect> | null {
   if (
     // Not enough args
@@ -2163,6 +2208,7 @@ function computeEffectsForSignature(
         }
         break;
       }
+      case 'Impure':
       case 'MutateFrozen':
       case 'MutateGlobal': {
         const values = substitutions.get(effect.place.identifier.id) ?? [];
@@ -2171,6 +2217,13 @@ function computeEffectsForSignature(
         }
         break;
       }
+      case 'Render': {
+        const values = substitutions.get(effect.place.identifier.id) ?? [];
+        for (const value of values) {
+          effects.push({kind: effect.kind, place: value});
+        }
+        break;
+      }
       case 'Mutate':
       case 'MutateTransitive':
       case 'MutateTransitiveConditionally':
@@ -2254,6 +2307,7 @@ function computeEffectsForSignature(
           function: applyFunction[0],
           into: applyInto[0],
           signature: effect.signature,
+          loc,
         });
         break;
       }
@@ -2468,6 +2522,7 @@ export type AliasingEffect =
       args: Array<Place | SpreadPattern | Hole>;
       into: Place;
       signature: FunctionSignature | null;
+      loc: SourceLocation;
     }
   /**
    * Constructs a function value with the given captures. The mutability of the function
@@ -2490,6 +2545,20 @@ export type AliasingEffect =
       kind: 'MutateGlobal';
       place: Place;
       error: CompilerErrorDetailOptions;
+    }
+  /**
+   * Indicates a side-effect that is not safe during render
+   */
+  | {kind: 'Impure'; place: Place; error: CompilerErrorDetailOptions}
+  /**
+   * Indicates that a given place is accessed during render. Used to distingush
+   * hook arguments that are known to be called immediately vs those used for
+   * event handlers/effects, and for JSX values known to be called during render
+   * (tags, children) vs those that may be events/effect (other props).
+   */
+  | {
+      kind: 'Render';
+      place: Place;
     };
 
 function hashEffect(effect: AliasingEffect): string {
@@ -2536,6 +2605,8 @@ function hashEffect(effect: AliasingEffect): string {
     case 'Freeze': {
       return [effect.kind, effect.value.identifier.id, effect.reason].join(':');
     }
+    case 'Impure':
+    case 'Render':
     case 'MutateFrozen':
     case 'MutateGlobal': {
       return [effect.kind, effect.place.identifier.id].join(':');

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

@@ -108,7 +108,9 @@ export function inferMutationAliasingFunctionEffects(
           ]);
         } else if (
           effect.kind === 'MutateFrozen' ||
-          effect.kind === 'MutateGlobal'
+          effect.kind === 'MutateGlobal' ||
+          effect.kind === 'Impure' ||
+          effect.kind === 'Render'
         ) {
           effects.push(effect);
         }

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

@@ -66,6 +66,7 @@ export function inferMutationAliasingRanges(
     kind: MutationKind;
     place: Place;
   }> = [];
+  const renders: Array<{index: number; place: Place}> = [];
 
   let index = 0;
 
@@ -160,9 +161,12 @@ export function inferMutationAliasingRanges(
           });
         } else if (
           effect.kind === 'MutateFrozen' ||
-          effect.kind === 'MutateGlobal'
+          effect.kind === 'MutateGlobal' ||
+          effect.kind === 'Impure'
         ) {
           errors.push(effect.error);
+        } else if (effect.kind === 'Render') {
+          renders.push({index: index++, place: effect.place});
         }
       }
     }
@@ -209,6 +213,9 @@ export function inferMutationAliasingRanges(
       errors,
     );
   }
+  for (const render of renders) {
+    state.render(render.index, render.place.identifier, errors);
+  }
   if (DEBUG) {
     console.log(pretty([...state.nodes.keys()]));
   }
@@ -357,6 +364,8 @@ export function inferMutationAliasingRanges(
             // no-op, Read is the default
             break;
           }
+          case 'Impure':
+          case 'Render':
           case 'MutateFrozen':
           case 'MutateGlobal': {
             // no-op
@@ -440,6 +449,7 @@ export function inferMutationAliasingRanges(
 function appendFunctionErrors(errors: CompilerError, fn: HIRFunction): void {
   for (const effect of fn.aliasingEffects ?? []) {
     switch (effect.kind) {
+      case 'Impure':
       case 'MutateFrozen':
       case 'MutateGlobal': {
         errors.push(effect.error);
@@ -530,6 +540,43 @@ class AliasingState {
     }
   }
 
+  render(index: number, start: Identifier, errors: CompilerError): void {
+    const seen = new Set<Identifier>();
+    const queue: Array<Identifier> = [start];
+    while (queue.length !== 0) {
+      const current = queue.pop()!;
+      if (seen.has(current)) {
+        continue;
+      }
+      seen.add(current);
+      const node = this.nodes.get(current);
+      if (node == null || node.transitive != null || node.local != null) {
+        continue;
+      }
+      if (node.value.kind === 'Function') {
+        appendFunctionErrors(errors, node.value.function);
+      }
+      for (const [alias, when] of node.createdFrom) {
+        if (when >= index) {
+          continue;
+        }
+        queue.push(alias);
+      }
+      for (const [alias, when] of node.aliases) {
+        if (when >= index) {
+          continue;
+        }
+        queue.push(alias);
+      }
+      for (const [capture, when] of node.captures) {
+        if (when >= index) {
+          continue;
+        }
+        queue.push(capture);
+      }
+    }
+  }
+
   mutate(
     index: number,
     start: Identifier,

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.expect.md

@@ -2,6 +2,7 @@
 ## Input
 
 ```javascript
+// @enableNewMutationAliasingModel:false
 function Component() {
   const foo = () => {
     someGlobal = true;
@@ -15,13 +16,13 @@ function Component() {
 ## Error
 
 ```
-  1 | function Component() {
-  2 |   const foo = () => {
-> 3 |     someGlobal = true;
-    |     ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (3:3)
-  4 |   };
-  5 |   return <div {...foo} />;
-  6 | }
+  2 | function Component() {
+  3 |   const foo = () => {
+> 4 |     someGlobal = true;
+    |     ^^^^^^^^^^ InvalidReact: Unexpected reassignment of a variable which was defined outside of the component. Components and hooks should be pure and side-effect free, but variable reassignment is a form of side-effect. If this variable is used in rendering, use useState instead. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#side-effects-must-run-outside-of-render) (4:4)
+  5 |   };
+  6 |   return <div {...foo} />;
+  7 | }
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.assign-global-in-jsx-spread-attribute.js

@@ -1,3 +1,4 @@
+// @enableNewMutationAliasingModel:false
 function Component() {
   const foo = () => {
     someGlobal = true;

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.invalid-impure-functions-in-render.expect.md

@@ -20,7 +20,7 @@ function Component() {
   2 |
   3 | function Component() {
 > 4 |   const date = Date.now();
-    |                ^^^^^^^^ InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Date.now` is an impure function whose results may change on every call (4:4)
+    |                ^^^^^^^^^^ InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `Date.now` is an impure function whose results may change on every call (4:4)
 
 InvalidReact: Calling an impure function can produce unstable results. (https://react.dev/reference/rules/components-and-hooks-must-be-pure#components-and-hooks-must-be-idempotent). `performance.now` is an impure function whose results may change on every call (5:5)