[compiler] Fix for false positive mutation of destructured spread object (#33786)

When destructuring, spread creates a new mutable object that _captures_
part of the original rvalue. This new value is safe to modify.

When making this change I realized that we weren't inferring array
pattern spread as creating an array (in type inference) so I also added
that here.

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

@@ -345,6 +345,51 @@ export function* eachPatternOperand(pattern: Pattern): Iterable<Place> {
   }
 }
 
+export function* eachPatternItem(
+  pattern: Pattern,
+): Iterable<Place | SpreadPattern> {
+  switch (pattern.kind) {
+    case 'ArrayPattern': {
+      for (const item of pattern.items) {
+        if (item.kind === 'Identifier') {
+          yield item;
+        } else if (item.kind === 'Spread') {
+          yield item;
+        } else if (item.kind === 'Hole') {
+          continue;
+        } else {
+          assertExhaustive(
+            item,
+            `Unexpected item kind \`${(item as any).kind}\``,
+          );
+        }
+      }
+      break;
+    }
+    case 'ObjectPattern': {
+      for (const property of pattern.properties) {
+        if (property.kind === 'ObjectProperty') {
+          yield property.place;
+        } else if (property.kind === 'Spread') {
+          yield property;
+        } else {
+          assertExhaustive(
+            property,
+            `Unexpected item kind \`${(property as any).kind}\``,
+          );
+        }
+      }
+      break;
+    }
+    default: {
+      assertExhaustive(
+        pattern,
+        `Unexpected pattern kind \`${(pattern as any).kind}\``,
+      );
+    }
+  }
+}
+
 export function mapInstructionLValues(
   instr: Instruction,
   fn: (place: Place) => Place,

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

@@ -36,8 +36,8 @@ import {
   ValueReason,
 } from '../HIR';
 import {
-  eachInstructionValueLValue,
   eachInstructionValueOperand,
+  eachPatternItem,
   eachTerminalOperand,
   eachTerminalSuccessor,
 } from '../HIR/visitors';
@@ -1864,19 +1864,34 @@ function computeSignatureForInstruction(
       break;
     }
     case 'Destructure': {
-      for (const patternLValue of eachInstructionValueLValue(value)) {
+      for (const patternItem of eachPatternItem(value.lvalue.pattern)) {
-        if (isPrimitiveType(patternLValue.identifier)) {
+        const place =
+          patternItem.kind === 'Identifier' ? patternItem : patternItem.place;
+        if (isPrimitiveType(place.identifier)) {
           effects.push({
             kind: 'Create',
-            into: patternLValue,
+            into: place,
             value: ValueKind.Primitive,
             reason: ValueReason.Other,
           });
-        } else {
+        } else if (patternItem.kind === 'Identifier') {
           effects.push({
             kind: 'CreateFrom',
             from: value.value,
-            into: patternLValue,
+            into: place,
+          });
+        } else {
+          // Spread creates a new object/array that captures from the RValue
+          effects.push({
+            kind: 'Create',
+            into: place,
+            reason: ValueReason.Other,
+            value: ValueKind.Mutable,
+          });
+          effects.push({
+            kind: 'Capture',
+            from: value.value,
+            into: place,
           });
         }
       }

compiler/packages/babel-plugin-react-compiler/src/TypeInference/InferTypes.ts

@@ -360,6 +360,12 @@ function* generateInstructionTypes(
                 value: makePropertyLiteral(propertyName),
               },
             });
+          } else if (item.kind === 'Spread') {
+            // Array pattern spread always creates an array
+            yield equation(item.place.identifier.type, {
+              kind: 'Object',
+              shapeId: BuiltInArrayId,
+            });
           } else {
             break;
           }

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.expect.md

@@ -0,0 +1,104 @@
+
+## Input
+
+```javascript
+// @validatePreserveExistingMemoizationGuarantees
+import {useMemo} from 'react';
+import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
+
+function Component(props) {
+  // Should memoize independently
+  const x = useMemo(() => makeObject_Primitives(), []);
+
+  const rest = useMemo(() => {
+    const [_, ...rest] = props.array;
+
+    // Should be inferred as Array.proto.push which doesn't mutate input
+    rest.push(x);
+    return rest;
+  });
+
+  return (
+    <>
+      <ValidateMemoization inputs={[]} output={x} />
+      <ValidateMemoization inputs={[props.array]} output={rest} />
+    </>
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{array: [0, 1, 2]}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime"; // @validatePreserveExistingMemoizationGuarantees
+import { useMemo } from "react";
+import { makeObject_Primitives, ValidateMemoization } from "shared-runtime";
+
+function Component(props) {
+  const $ = _c(9);
+  let t0;
+  if ($[0] === Symbol.for("react.memo_cache_sentinel")) {
+    t0 = makeObject_Primitives();
+    $[0] = t0;
+  } else {
+    t0 = $[0];
+  }
+  const x = t0;
+  let rest;
+  if ($[1] !== props.array) {
+    [, ...rest] = props.array;
+
+    rest.push(x);
+    $[1] = props.array;
+    $[2] = rest;
+  } else {
+    rest = $[2];
+  }
+  const rest_0 = rest;
+  let t1;
+  if ($[3] === Symbol.for("react.memo_cache_sentinel")) {
+    t1 = <ValidateMemoization inputs={[]} output={x} />;
+    $[3] = t1;
+  } else {
+    t1 = $[3];
+  }
+  let t2;
+  if ($[4] !== props.array) {
+    t2 = [props.array];
+    $[4] = props.array;
+    $[5] = t2;
+  } else {
+    t2 = $[5];
+  }
+  let t3;
+  if ($[6] !== rest_0 || $[7] !== t2) {
+    t3 = (
+      <>
+        {t1}
+        <ValidateMemoization inputs={t2} output={rest_0} />
+      </>
+    );
+    $[6] = rest_0;
+    $[7] = t2;
+    $[8] = t3;
+  } else {
+    t3 = $[8];
+  }
+  return t3;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ array: [0, 1, 2] }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"inputs":[],"output":{"a":0,"b":"value1","c":true}}</div><div>{"inputs":[[0,1,2]],"output":[1,2,{"a":0,"b":"value1","c":true}]}</div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/array-pattern-spread-creates-array.js

@@ -0,0 +1,28 @@
+// @validatePreserveExistingMemoizationGuarantees
+import {useMemo} from 'react';
+import {makeObject_Primitives, ValidateMemoization} from 'shared-runtime';
+
+function Component(props) {
+  // Should memoize independently
+  const x = useMemo(() => makeObject_Primitives(), []);
+
+  const rest = useMemo(() => {
+    const [_, ...rest] = props.array;
+
+    // Should be inferred as Array.proto.push which doesn't mutate input
+    rest.push(x);
+    return rest;
+  });
+
+  return (
+    <>
+      <ValidateMemoization inputs={[]} output={x} />
+      <ValidateMemoization inputs={[props.array]} output={rest} />
+    </>
+  );
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{array: [0, 1, 2]}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-of-new-object-from-destructured-prop.expect.md

@@ -0,0 +1,62 @@
+
+## Input
+
+```javascript
+import {Stringify} from 'shared-runtime';
+
+function Component(props) {
+  const {a} = props;
+  const {b, ...rest} = a;
+  // Local mutation of `rest` is allowed since it is a newly allocated object
+  rest.value = props.value;
+  return <Stringify rest={rest} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{a: {b: 0, other: 'other'}, value: 42}],
+};
+
+```
+
+## Code
+
+```javascript
+import { c as _c } from "react/compiler-runtime";
+import { Stringify } from "shared-runtime";
+
+function Component(props) {
+  const $ = _c(5);
+  const { a } = props;
+  let rest;
+  if ($[0] !== a || $[1] !== props.value) {
+    const { b, ...t0 } = a;
+    rest = t0;
+
+    rest.value = props.value;
+    $[0] = a;
+    $[1] = props.value;
+    $[2] = rest;
+  } else {
+    rest = $[2];
+  }
+  let t0;
+  if ($[3] !== rest) {
+    t0 = <Stringify rest={rest} />;
+    $[3] = rest;
+    $[4] = t0;
+  } else {
+    t0 = $[4];
+  }
+  return t0;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{ a: { b: 0, other: "other" }, value: 42 }],
+};
+
+```
+      
+### Eval output
+(kind: ok) <div>{"rest":{"other":"other","value":42}}</div>

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-local-mutation-of-new-object-from-destructured-prop.js

@@ -0,0 +1,14 @@
+import {Stringify} from 'shared-runtime';
+
+function Component(props) {
+  const {a} = props;
+  const {b, ...rest} = a;
+  // Local mutation of `rest` is allowed since it is a newly allocated object
+  rest.value = props.value;
+  return <Stringify rest={rest} />;
+}
+
+export const FIXTURE_ENTRYPOINT = {
+  fn: Component,
+  params: [{a: {b: 0, other: 'other'}, value: 42}],
+};

compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-undefined-expression-of-jsxexpressioncontainer.expect.md

@@ -48,32 +48,26 @@ import { c as _c } from "react/compiler-runtime";
 import { StaticText1, Stringify, Text } from "shared-runtime";
 
 function Component(props) {
-  const $ = _c(6);
+  const $ = _c(4);
   const { buttons } = props;
-  let nonPrimaryButtons;
-  if ($[0] !== buttons) {
-    [, ...nonPrimaryButtons] = buttons;
-    $[0] = buttons;
-    $[1] = nonPrimaryButtons;
-  } else {
-    nonPrimaryButtons = $[1];
-  }
   let t0;
-  if ($[2] !== nonPrimaryButtons) {
+  if ($[0] !== buttons) {
+    const [, ...nonPrimaryButtons] = buttons;
+
     t0 = nonPrimaryButtons.map(_temp);
-    $[2] = nonPrimaryButtons;
+    $[0] = buttons;
-    $[3] = t0;
+    $[1] = t0;
   } else {
-    t0 = $[3];
+    t0 = $[1];
   }
   const renderedNonPrimaryButtons = t0;
   let t1;
-  if ($[4] !== renderedNonPrimaryButtons) {
+  if ($[2] !== renderedNonPrimaryButtons) {
     t1 = <StaticText1>{renderedNonPrimaryButtons}</StaticText1>;
-    $[4] = renderedNonPrimaryButtons;
+    $[2] = renderedNonPrimaryButtons;
-    $[5] = t1;
+    $[3] = t1;
   } else {
-    t1 = $[5];
+    t1 = $[3];
   }
   return t1;
 }