[compiler][optim] more shapes for mixedreadonly

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

@@ -1126,9 +1126,32 @@ export class Environment {
     );
   }
 
+  getFallthroughPropertyType(
+    receiver: Type,
+    _property: Type,
+  ): BuiltInType | PolyType | null {
+    let shapeId = null;
+    if (receiver.kind === 'Object' || receiver.kind === 'Function') {
+      shapeId = receiver.shapeId;
+    }
+
+    if (shapeId !== null) {
+      const shape = this.#shapes.get(shapeId);
+
+      CompilerError.invariant(shape !== undefined, {
+        reason: `[HIR] Forget internal error: cannot resolve shape ${shapeId}`,
+        description: null,
+        loc: null,
+        suggestions: null,
+      });
+      return shape.properties.get('*') ?? null;
+    }
+    return null;
+  }
+
   getPropertyType(
     receiver: Type,
-    property: string,
+    property: string | number,
   ): BuiltInType | PolyType | null {
     let shapeId = null;
     if (receiver.kind === 'Object' || receiver.kind === 'Function') {
@@ -1146,17 +1169,19 @@ export class Environment {
         loc: null,
         suggestions: null,
       });
-      let value =
-        shape.properties.get(property) ?? shape.properties.get('*') ?? null;
-      if (value === null && isHookName(property)) {
-        value = this.#getCustomHookType();
+      if (typeof property === 'string') {
+        return (
+          shape.properties.get(property) ??
+          shape.properties.get('*') ??
+          (isHookName(property) ? this.#getCustomHookType() : null)
+        );
+      } else {
+        return shape.properties.get('*') ?? null;
       }
-      return value;
-    } else if (isHookName(property)) {
+    } else if (typeof property === 'string' && isHookName(property)) {
       return this.#getCustomHookType();
-    } else {
-      return null;
     }
+    return null;
   }
 
   getFunctionSignature(type: FunctionType): FunctionSignature | null {

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

@@ -119,15 +119,29 @@ const TYPED_GLOBALS: Array<[string, BuiltInType]> = [
       ],
       /*
        * https://tc39.es/ecma262/multipage/indexed-collections.html#sec-array.from
-       * Array.from(arrayLike, optionalFn, optionalThis) not added because
-       * the Effect of `arrayLike` is polymorphic i.e.
+       * Array.from(arrayLike, optionalFn, optionalThis)
+       * Note that the Effect of `arrayLike` is polymorphic i.e.
        *  - Effect.read if
        *     - it does not have an @iterator property and is array-like
        *       (i.e. has a length property)
        *    - it is an iterable object whose iterator does not mutate itself
        *  - Effect.mutate if it is a self-mutative iterator (e.g. a generator
        *    function)
        */
+      [
+        'from',
+        addFunction(DEFAULT_SHAPES, [], {
+          positionalParams: [
+            Effect.ConditionallyMutate,
+            Effect.ConditionallyMutate,
+            Effect.ConditionallyMutate,
+          ],
+          restParam: Effect.Read,
+          returnType: {kind: 'Object', shapeId: BuiltInArrayId},
+          calleeEffect: Effect.Read,
+          returnValueKind: ValueKind.Mutable,
+        }),
+      ],
       [
         'of',
         // Array.of(element0, ..., elementN)

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

@@ -535,6 +535,30 @@ addObject(BUILTIN_SHAPES, BuiltInRefValueId, [
   ['*', {kind: 'Object', shapeId: BuiltInRefValueId}],
 ]);
 
+/**
+ * MixedReadOnly =
+ *   | primitive
+ *   | simple objects (Record<string, MixedReadOnly>)
+ *   | Array<MixedReadOnly>
+ *
+ * APIs such as Relay — but also Flux and other data stores — often return a
+ * union of types with some interesting properties in terms of analysis.
+ *
+ * Given this constraint, if data came from Relay, then we should be able to
+ * infer things like `data.items.map(): Array`. That may seem like a leap at
+ * first but remember, we assume you're not patching builtins. Thus the only way
+ * data.items.map can exist and be a function, given the above set of data types
+ * and builtin JS methods, is if `data.items` was an Array, and `data.items.map`
+ * is therefore calling Array.prototype.map. Then we know that function returns
+ * an Array as well. This relies on the fact that map() is being called, so if
+ * data.items was some other type it would error at runtime - so it's sound.
+ *
+ * Note that this shape is currently only used for hook return values, which
+ * means that it's safe to type aliasing method-call return kinds as `Frozen`.
+ *
+ * Also note that all newly created arrays from method-calls (e.g. `.map`)
+ * have the appropriate mutable `BuiltInArray` shape
+ */
 addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
   [
     'toString',
@@ -546,6 +570,36 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
       returnValueKind: ValueKind.Primitive,
     }),
   ],
+  [
+    'indexOf',
+    addFunction(BUILTIN_SHAPES, [], {
+      positionalParams: [],
+      restParam: Effect.Read,
+      returnType: {kind: 'Primitive'},
+      calleeEffect: Effect.Read,
+      returnValueKind: ValueKind.Primitive,
+    }),
+  ],
+  [
+    'includes',
+    addFunction(BUILTIN_SHAPES, [], {
+      positionalParams: [],
+      restParam: Effect.Read,
+      returnType: {kind: 'Primitive'},
+      calleeEffect: Effect.Read,
+      returnValueKind: ValueKind.Primitive,
+    }),
+  ],
+  [
+    'at',
+    addFunction(BUILTIN_SHAPES, [], {
+      positionalParams: [Effect.Read],
+      restParam: null,
+      returnType: {kind: 'Object', shapeId: BuiltInMixedReadonlyId},
+      calleeEffect: Effect.Capture,
+      returnValueKind: ValueKind.Frozen,
+    }),
+  ],
   [
     'map',
     addFunction(BUILTIN_SHAPES, [], {
@@ -642,9 +696,9 @@ addObject(BUILTIN_SHAPES, BuiltInMixedReadonlyId, [
     addFunction(BUILTIN_SHAPES, [], {
       positionalParams: [],
       restParam: Effect.ConditionallyMutate,
-      returnType: {kind: 'Poly'},
+      returnType: {kind: 'Object', shapeId: BuiltInMixedReadonlyId},
       calleeEffect: Effect.ConditionallyMutate,
-      returnValueKind: ValueKind.Mutable,
+      returnValueKind: ValueKind.Frozen,
       noAlias: true,
       mutableOnlyIfOperandsAreMutable: true,
     }),

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

@@ -60,7 +60,15 @@ export type PropType = {
   kind: 'Property';
   objectType: Type;
   objectName: string;
-  propertyName: PropertyLiteral;
+  propertyName:
+    | {
+        kind: 'literal';
+        value: PropertyLiteral;
+      }
+    | {
+        kind: 'computed';
+        value: Type;
+      };
 };
 
 export type ObjectMethod = {

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;
+  }
+}