[compiler] Patch array and argument spread mutability

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

@@ -872,11 +872,33 @@ function inferBlock(
                 reason: new Set([ValueReason.Other]),
                 context: new Set(),
               };
+
+        for (const element of instrValue.elements) {
+          if (element.kind === 'Spread') {
+            state.referenceAndRecordEffects(
+              freezeActions,
+              element.place,
+              isArrayType(element.place.identifier)
+                ? Effect.Capture
+                : Effect.ConditionallyMutate,
+              ValueReason.Other,
+            );
+          } else if (element.kind === 'Identifier') {
+            state.referenceAndRecordEffects(
+              freezeActions,
+              element,
+              Effect.Capture,
+              ValueReason.Other,
+            );
+          } else {
+            let _: 'Hole' = element.kind;
+          }
+        }
+        state.initialize(instrValue, valueKind);
+        state.define(instr.lvalue, instrValue);
+        instr.lvalue.effect = Effect.Store;
         continuation = {
-          kind: 'initialize',
-          valueKind,
-          effect: {kind: Effect.Capture, reason: ValueReason.Other},
-          lvalueEffect: Effect.Store,
+          kind: 'funeffects',
         };
         break;
       }
@@ -1241,21 +1263,12 @@ function inferBlock(
         for (let i = 0; i < instrValue.args.length; i++) {
           const arg = instrValue.args[i];
           const place = arg.kind === 'Identifier' ? arg : arg.place;
-          if (effects !== null) {
-            state.referenceAndRecordEffects(
-              freezeActions,
-              place,
-              effects[i],
-              ValueReason.Other,
-            );
-          } else {
-            state.referenceAndRecordEffects(
-              freezeActions,
-              place,
-              Effect.ConditionallyMutate,
-              ValueReason.Other,
-            );
-          }
+          state.referenceAndRecordEffects(
+            freezeActions,
+            place,
+            getArgumentEffect(effects != null ? effects[i] : null, arg),
+            ValueReason.Other,
+          );
           hasCaptureArgument ||= place.effect === Effect.Capture;
         }
         if (signature !== null) {
@@ -1307,7 +1320,10 @@ function inferBlock(
           signature !== null
             ? {
                 kind: signature.returnValueKind,
-                reason: new Set([ValueReason.Other]),
+                reason: new Set([
+                  signature.returnValueReason ??
+                    ValueReason.KnownReturnSignature,
+                ]),
                 context: new Set(),
               }
             : {
@@ -1356,25 +1372,16 @@ function inferBlock(
         for (let i = 0; i < instrValue.args.length; i++) {
           const arg = instrValue.args[i];
           const place = arg.kind === 'Identifier' ? arg : arg.place;
-          if (effects !== null) {
-            /*
-             * If effects are inferred for an argument, we should fail invalid
-             * mutating effects
-             */
-            state.referenceAndRecordEffects(
-              freezeActions,
-              place,
-              effects[i],
-              ValueReason.Other,
-            );
-          } else {
-            state.referenceAndRecordEffects(
-              freezeActions,
-              place,
-              Effect.ConditionallyMutate,
-              ValueReason.Other,
-            );
-          }
+          /*
+           * If effects are inferred for an argument, we should fail invalid
+           * mutating effects
+           */
+          state.referenceAndRecordEffects(
+            freezeActions,
+            place,
+            getArgumentEffect(effects != null ? effects[i] : null, arg),
+            ValueReason.Other,
+          );
           hasCaptureArgument ||= place.effect === Effect.Capture;
         }
         if (signature !== null) {
@@ -2049,3 +2056,31 @@ function areArgumentsImmutableAndNonMutating(
   }
   return true;
 }
+
+function getArgumentEffect(
+  signatureEffect: Effect | null,
+  arg: Place | SpreadPattern,
+): Effect {
+  if (signatureEffect != null) {
+    if (arg.kind === 'Identifier') {
+      return signatureEffect;
+    } else if (
+      signatureEffect === Effect.Mutate ||
+      signatureEffect === Effect.ConditionallyMutate
+    ) {
+      return signatureEffect;
+    } else {
+      // see call-spread-argument-mutable-iterator test fixture
+      if (signatureEffect === Effect.Freeze) {
+        CompilerError.throwTodo({
+          reason: 'Support spread syntax for hook arguments',
+          loc: arg.place.loc,
+        });
+      }
+      // effects[i] is Effect.Capture | Effect.Read | Effect.Store
+      return Effect.ConditionallyMutate;
+    }
+  } else {
+    return Effect.ConditionallyMutate;
+  }
+}

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.expect.md

@@ -0,0 +1,62 @@
+
+## Input
+
+```javascript
+function useBar({arg}) {
+  /**
+   * Note that mutableIterator is mutated by the later object spread. Therefore,
+   * `s.values()` should be memoized within the same block as the object spread.
+   * In terms of compiler internals, they should have the same reactive scope.
+   */
+  const obj = {};
+  const s = new Set([obj, 5, 4]);
+  const mutableIterator = s.values();
+  const arr = [...mutableIterator];
+
+  obj.x = arg;
+  return arr;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useBar,
+  params: [{arg: 3}],
+  sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+function useBar(t0) {
+  const $ = _c(2);
+  const { arg } = t0;
+  let arr;
+  if ($[0] !== arg) {
+    const obj = {};
+    const s = new Set([obj, 5, 4]);
+    const mutableIterator = s.values();
+    arr = [...mutableIterator];
+
+    obj.x = arg;
+    $[0] = arg;
+    $[1] = arr;
+  } else {
+    arr = $[1];
+  }
+  return arr;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useBar,
+  params: [{ arg: 3 }],
+  sequentialRenders: [{ arg: 3 }, { arg: 3 }, { arg: 4 }],
+};
+
+```
+
+### Eval output
+(kind: ok) [{"x":3},5,4]
+[{"x":3},5,4]
+[{"x":4},5,4]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-later-mutated.js

@@ -0,0 +1,20 @@
+function useBar({arg}) {
+  /**
+   * Note that mutableIterator is mutated by the later object spread. Therefore,
+   * `s.values()` should be memoized within the same block as the object spread.
+   * In terms of compiler internals, they should have the same reactive scope.
+   */
+  const obj = {};
+  const s = new Set([obj, 5, 4]);
+  const mutableIterator = s.values();
+  const arr = [...mutableIterator];
+
+  obj.x = arg;
+  return arr;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useBar,
+  params: [{arg: 3}],
+  sequentialRenders: [{arg: 3}, {arg: 3}, {arg: 4}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-array-spread-mutable-iterator.expect.md → compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-spread-mutable-iterator.expect.md

@@ -55,26 +55,20 @@ import { c as _c } from "react/compiler-runtime"; /**
 
 function useBar(t0) {
   "use memo";
-  const $ = _c(3);
+  const $ = _c(2);
   const { arg } = t0;
   let t1;
-  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+  if ($[0] !== arg) {
     const s = new Set([1, 5, 4]);
-    t1 = s.values();
-    $[0] = t1;
-  } else {
-    t1 = $[0];
-  }
-  const mutableIterator = t1;
-  let t2;
-  if ($[1] !== arg) {
-    t2 = [arg, ...mutableIterator];
-    $[1] = arg;
-    $[2] = t2;
+    const mutableIterator = s.values();
+
+    t1 = [arg, ...mutableIterator];
+    $[0] = arg;
+    $[1] = t1;
   } else {
-    t2 = $[2];
+    t1 = $[1];
   }
-  return t2;
+  return t1;
 }
 
 export const FIXTURE_ENTRYPOINT = {
@@ -84,4 +78,8 @@ export const FIXTURE_ENTRYPOINT = {
 };
 
 ```
-
+
+### Eval output
+(kind: ok) [3,1,5,4]
+[3,1,5,4]
+[4,1,5,4]

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.expect.md

@@ -0,0 +1,42 @@
+
+## Input
+
+```javascript
+import {useIdentity} from 'shared-runtime';
+
+function useFoo() {
+  const it = new Set([1, 2]).values();
+  useIdentity();
+  return Math.max(...it);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{}],
+  sequentialRenders: [{}, {}],
+};
+
+```
+
+## Code
+
+```javascript
+import { useIdentity } from "shared-runtime";
+
+function useFoo() {
+  const it = new Set([1, 2]).values();
+  useIdentity();
+  return Math.max(...it);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{}],
+  sequentialRenders: [{}, {}],
+};
+
+```
+
+### Eval output
+(kind: ok) 2
+2

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/call-spread-argument-mutable-iterator.js

@@ -0,0 +1,13 @@
+import {useIdentity} from 'shared-runtime';
+
+function useFoo() {
+  const it = new Set([1, 2]).values();
+  useIdentity();
+  return Math.max(...it);
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: useFoo,
+  params: [{}],
+  sequentialRenders: [{}, {}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.expect.md

@@ -0,0 +1,33 @@
+
+## Input
+
+```javascript
+import {useIdentity} from 'shared-runtime';
+
+function Component() {
+  const items = makeArray(0, 1, 2, null, 4, false, 6);
+  return useIdentity(...items.values());
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [],
+  sequentialRenders: [{}, {}],
+};
+
+```
+
+
+## Error
+
+```
+  3 | function Component() {
+  4 |   const items = makeArray(0, 1, 2, null, 4, false, 6);
+> 5 |   return useIdentity(...items.values());
+    |                         ^^^^^^^^^^^^^^ Todo: Support spread syntax for hook arguments (5:5)
+  6 | }
+  7 |
+  8 | export const FIXTURE_ENTRYPOINT = {
+```
+
+

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-hook-call-spreads-mutable-iterator.js

@@ -0,0 +1,12 @@
+import {useIdentity} from 'shared-runtime';
+
+function Component() {
+  const items = makeArray(0, 1, 2, null, 4, false, 6);
+  return useIdentity(...items.values());
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [],
+  sequentialRenders: [{}, {}],
+};