[compiler] Patch array and argument spread mutability

Array and argument spreads may mutate stateful iterables. Spread sites should have `ConditionallyMutate` effects (e.g. mutate if the ValueKind is mutable, otherwise read).

See
- [ecma spec (13.2.4.1 Runtime Semantics: ArrayAccumulation. SpreadElement : ... AssignmentExpression)](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-runtime-semantics-arrayaccumulation).
- [ecma spec 13.3.8.1 Runtime Semantics: ArgumentListEvaluation](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-runtime-semantics-argumentlistevaluation)

Note that
- Object and JSX Attribute spreads do not evaluate iterables (srcs [mozilla](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#description), [ecma](https://tc39.es/ecma262/multipage/ecmascript-language-expressions.html#sec-runtime-semantics-propertydefinitionevaluation))
- An ideal mutability inference system could model known collections (i.e. Arrays or Sets) as a "mutated collection of non-mutable objects" (see `todo-granular-iterator-semantics`), but this is not what we do today. As such, an array / argument spread will always extend the range of built-in arrays, sets, etc
- Due to HIR limitations, call expressions with argument spreads may cause unnecessary bailouts and/or scope merging when we know the call itself has `freeze`, `capture`, or `read` semantics (e.g. `useHook(...mutableValue)`)
  We can deal with this by rewriting these call instructions to (1) create an intermediate array to consume the iterator and (2) capture and spread the array at the callsite

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',
+          kind: 'funeffects',
-          valueKind,
-          effect: {kind: Effect.Capture, reason: ValueReason.Other},
-          lvalueEffect: Effect.Store,
         };
         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(
-            state.referenceAndRecordEffects(
+            freezeActions,
-              freezeActions,
+            place,
-              place,
+            getArgumentEffect(effects != null ? effects[i] : null, arg),
-              effects[i],
+            ValueReason.Other,
-              ValueReason.Other,
+          );
-            );
-          } else {
-            state.referenceAndRecordEffects(
-              freezeActions,
-              place,
-              Effect.ConditionallyMutate,
-              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
-             * If effects are inferred for an argument, we should fail invalid
+           * mutating effects
-             * mutating effects
+           */
-             */
+          state.referenceAndRecordEffects(
-            state.referenceAndRecordEffects(
+            freezeActions,
-              freezeActions,
+            place,
-              place,
+            getArgumentEffect(effects != null ? effects[i] : null, arg),
-              effects[i],
+            ValueReason.Other,
-              ValueReason.Other,
+          );
-            );
-          } else {
-            state.referenceAndRecordEffects(
-              freezeActions,
-              place,
-              Effect.ConditionallyMutate,
-              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/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();
+    const mutableIterator = s.values();
-    $[0] = t1;
+
+    t1 = [arg, ...mutableIterator];
+    $[0] = arg;
+    $[1] = t1;
   } else {
-    t1 = $[0];
+    t1 = $[1];
   }
-  const mutableIterator = t1;
+  return t1;
-  let t2;
-  if ($[1] !== arg) {
-    t2 = [arg, ...mutableIterator];
-    $[1] = arg;
-    $[2] = t2;
-  } else {
-    t2 = $[2];
-  }
-  return t2;
 }
 
 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: [{}, {}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.expect.md

@@ -4,9 +4,10 @@
 ```javascript
 import {makeArray} from 'shared-runtime';
 
-function Component(props) {
+const other = [0, 1];
+function Component({}) {
   const items = makeArray(0, 1, 2, null, 4, false, 6);
-  const max = Math.max(...items.filter(Boolean));
+  const max = Math.max(2, items.push(5), ...other);
   return max;
 }
 
@@ -21,13 +22,13 @@ export const FIXTURE_ENTRYPOINT = {
 ## Error
 
 ```
-  3 | function Component(props) {
+  4 | function Component({}) {
-  4 |   const items = makeArray(0, 1, 2, null, 4, false, 6);
+  5 |   const items = makeArray(0, 1, 2, null, 4, false, 6);
-> 5 |   const max = Math.max(...items.filter(Boolean));
+> 6 |   const max = Math.max(2, items.push(5), ...other);
-    |               ^^^^^^^^ Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` (5:5)
+    |               ^^^^^^^^ Invariant: [Codegen] Internal error: MethodCall::property must be an unpromoted + unmemoized MemberExpression. Got a `Identifier` (6:6)
-  6 |   return max;
+  7 |   return max;
-  7 | }
+  8 | }
-  8 |
+  9 |
 ```
           
       

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/error.todo-nested-method-calls-lower-property-load-into-temporary.js

@@ -1,8 +1,9 @@
 import {makeArray} from 'shared-runtime';
 
-function Component(props) {
+const other = [0, 1];
+function Component({}) {
   const items = makeArray(0, 1, 2, null, 4, false, 6);
-  const max = Math.max(...items.filter(Boolean));
+  const max = Math.max(2, items.push(5), ...other);
   return max;
 }
 

compiler/packages/snap/src/SproutTodoFilter.ts

@@ -462,7 +462,6 @@ const skipFilter = new Set([
 
   // bugs
   'bug-object-expression-computed-key-modified-during-after-construction-hoisted-sequence-expr',
-  'bug-array-spread-mutable-iterator',
   `bug-capturing-func-maybealias-captured-mutate`,
   'bug-aliased-capture-aliased-mutate',
   'bug-aliased-capture-mutate',