[compiler] Propagate CreateFunction effects for functions that return functions #33642

josephsavona · 2025-06-25T17:56:19Z

If you have a local helper function that itself returns a function (() => () => { ... }), we currently infer the return effect of the outer function as Create mutable. We correctly track the aliasing, but we lose some precision because we don't understand that a function specifically is being returned.

Here, we do some extra analysis of which values are returned in InferMutationAliasingRanges, and if the sole return value is a function we infer a CreateFunction effect. We also infer an Assign (instead of a Create) if the sole return value was one of the context variables or parameters.

Stack created with Sapling. Best reviewed with ReviewStack.

josephsavona · 2025-06-25T17:58:23Z

...compiler/src/__tests__/fixtures/compiler/repro-returned-inner-fn-reassigns-context.expect.md

+    t1 = $[1];
+  }
+  const arr = t1;
+  fn(arr);


Before, all we knew is that fn (created by fnFactory) is a mutable value, and therefore could be mutating arr. Now we know that fn is a function, with a specific signature, and that it doesn't mutate it's argument. That means we can memoize arr independently.

josephsavona · 2025-06-25T17:58:45Z

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

+  const returned: Set<Node> = new Set();
+  const queue: Array<Node> = [state.nodes.get(fn.returns.identifier)!];
+  const seen: Set<Node> = new Set();
+  while (queue.length !== 0) {
+    const node = queue.pop()!;
+    if (seen.has(node)) {
+      continue;
+    }
+    seen.add(node);
+    for (const id of node.aliases.keys()) {
+      queue.push(state.nodes.get(id)!);
+    }
+    for (const id of node.createdFrom.keys()) {
+      queue.push(state.nodes.get(id)!);
+    }
+    if (node.id.id === fn.returns.identifier.id) {
+      continue;
+    }
+    switch (node.value.kind) {
+      case 'Assign':
+      case 'CreateFrom': {
+        break;
+      }
+      case 'Phi':
+      case 'Object':
+      case 'Function': {
+        returned.add(node);
+        break;
+      }
+      default: {
+        assertExhaustive(
+          node.value,
+          `Unexpected node value kind '${(node.value as any).kind}'`,
+        );
+      }
+    }
+  }


i want to clean this logic up a bit before landing

…33624) Closes #33577, a bug with ExtractScopeDeclarationsFromDestructuring and codegen when a function param is reassigned. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33624). * #33643 * #33642 * #33640 * #33625 * __->__ #33624

Small cosmetic win, found this when i was looking at some code internally with lots of cases that all share the same logic. Previously, all the but last one would have an empty block. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33625). * #33643 * #33642 * #33640 * __->__ #33625 * #33624

We now have `HIRFunction.returns: Place` as well as `returnType: Type`. I want to add additional return information, so as a first step i'm consolidating everything under an object at `HIRFunction.returns: {place: Place}`. We use the type of this place as the return type. Next step is to add more properties to this object to represent things like the return kind. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33640). * #33643 * #33642 * __->__ #33640 * #33625 * #33624

Small cosmetic win, found this when i was looking at some code internally with lots of cases that all share the same logic. Previously, all the but last one would have an empty block. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33625). * #33643 * #33642 * #33640 * __->__ #33625 * #33624 DiffTrain build for [e130c08](e130c08)

We now have `HIRFunction.returns: Place` as well as `returnType: Type`. I want to add additional return information, so as a first step i'm consolidating everything under an object at `HIRFunction.returns: {place: Place}`. We use the type of this place as the return type. Next step is to add more properties to this object to represent things like the return kind. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33640). * #33643 * #33642 * __->__ #33640 * #33625 * #33624 DiffTrain build for [123ff13](123ff13)

…33624) Closes #33577, a bug with ExtractScopeDeclarationsFromDestructuring and codegen when a function param is reassigned. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33624). * #33643 * #33642 * #33640 * #33625 * __->__ #33624 DiffTrain build for [9894c48](9894c48)

… functions If you have a local helper function that itself returns a function (`() => () => { ... }`), we currently infer the return effect of the outer function as `Create mutable`. We correctly track the aliasing, but we lose some precision because we don't understand that a function specifically is being returned. Here, we do some extra analysis of which values are returned in InferMutationAliasingRanges, and if the sole return value is a function we infer a `CreateFunction` effect. We also infer an `Assign` (instead of a Create) if the sole return value was one of the context variables or parameters.

This was referenced Jun 25, 2025

[compiler] Consolidate HIRFunction return information #33640

Merged

[compiler] Fix bug with reassigning function param in destructuring #33624

Merged

[compiler] Avoid empty switch cases #33625

Merged

facebook-github-bot added the CLA Signed label Jun 25, 2025

josephsavona marked this pull request as draft June 25, 2025 17:56

github-actions bot added the React Core Team Opened by a member of the React Core Team label Jun 25, 2025

josephsavona commented Jun 25, 2025

View reviewed changes

josephsavona mentioned this pull request Jun 25, 2025

[compiler] Repro for missed case of global mutation #33643

Draft

This was referenced Jun 25, 2025

[compiler] Infer render helpers for additional validation #33647

Open

[compiler] InferEffects uses effects as value keys #33650

Open

josephsavona force-pushed the pr33642 branch from 87dc463 to 5415e4e Compare June 27, 2025 00:07

kushxg mentioned this pull request Jul 1, 2025

Mirror of upstream PR #33642 kushxg/react-mirror#183

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[compiler] Propagate CreateFunction effects for functions that return functions #33642

[compiler] Propagate CreateFunction effects for functions that return functions #33642

josephsavona commented Jun 25, 2025 •

edited

Loading

Uh oh!

josephsavona Jun 25, 2025

Uh oh!

josephsavona Jun 25, 2025

Uh oh!

Uh oh!

[compiler] Propagate CreateFunction effects for functions that return functions #33642

Are you sure you want to change the base?

[compiler] Propagate CreateFunction effects for functions that return functions #33642

Conversation

josephsavona commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

josephsavona Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

josephsavona Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

josephsavona commented Jun 25, 2025 •

edited

Loading