[compiler] Represent array accesses with PropertyLoad

(draft, needs cleanup) Prior to this PR, our HIR represented property access with numeric literals (e.g. `myVar[0]`) as ComputedLoads. This means that they were subject to some deopts (most notably, not being easily dedupable / hoistable as dependencies). Now, `PropertyLoad`, `PropertyStore`, etc reference numeric and string literals (although not yet string literals that aren't valid babel identifiers). The difference between PropertyLoad and ComputedLoad is fuzzy now (maybe we should rename these). - PropertyLoad: property keys are string and numeric literals, only when the string literals are valid babel identifiers - ComputedLoad: non-valid babel identifier string literals (rare) and other non-literal expressions The biggest feature from this PR is that it trivially enables array-indicing expressions as dependencies. The compiler can also specify global and imported types for arrays (e.g. return value of `useState`) I'm happy to close this if it complicates more than it helps -- alternative options are to entirely rely on instruction reordering-based approaches like ReactiveGraphIR or make dependency-specific parsing + hoisting logic more robust.
facebook · Feb 1, 2025 · 096b717 · 096b717
1 parent 06adef6
commit 096b717
Show file tree

Hide file tree

Showing 26 changed files with 328 additions and 238 deletions.
diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
@@ -36,12 +36,14 @@ import {
   ObjectProperty,
   ObjectPropertyKey,
   Place,
+  PropertyLiteral,
   ReturnTerminal,
   SourceLocation,
   SpreadPattern,
   ThrowTerminal,
   Type,
   makeInstructionId,
+  makePropertyLiteral,
   makeType,
   promoteTemporary,
 } from './HIR';
@@ -2019,11 +2021,11 @@ function lowerExpression(
           });
 
           // Save the result back to the property
-          if (typeof property === 'string') {
+          if (typeof property === 'string' || typeof property === 'number') {
             return {
               kind: 'PropertyStore',
               object: {...object},
-              property,
+              property: makePropertyLiteral(property),
               value: {...newValuePlace},
               loc: leftExpr.node.loc ?? GeneratedSource,
             };
@@ -2318,11 +2320,11 @@ function lowerExpression(
         const argument = expr.get('argument');
         if (argument.isMemberExpression()) {
           const {object, property} = lowerMemberExpression(builder, argument);
-          if (typeof property === 'string') {
+          if (typeof property === 'string' || typeof property === 'number') {
             return {
               kind: 'PropertyDelete',
               object,
-              property,
+              property: makePropertyLiteral(property),
               loc: exprLoc,
             };
           } else {
@@ -2429,11 +2431,11 @@ function lowerExpression(
 
         // Save the result back to the property
         let newValuePlace;
-        if (typeof property === 'string') {
+        if (typeof property === 'string' || typeof property === 'number') {
           newValuePlace = lowerValueToTemporary(builder, {
             kind: 'PropertyStore',
             object: {...object},
-            property,
+            property: makePropertyLiteral(property),
             value: {...updatedValue},
             loc: leftExpr.node.loc ?? GeneratedSource,
           });
@@ -3058,7 +3060,7 @@ function lowerArguments(
 
 type LoweredMemberExpression = {
   object: Place;
-  property: Place | string;
+  property: Place | string | number;
   value: InstructionValue;
 };
 function lowerMemberExpression(
@@ -3073,8 +3075,13 @@ function lowerMemberExpression(
   const object =
     loweredObject ?? lowerExpressionToTemporary(builder, objectNode);
 
-  if (!expr.node.computed) {
-    if (!propertyNode.isIdentifier()) {
+  if (!expr.node.computed || expr.node.property.type === 'NumericLiteral') {
+    let property: PropertyLiteral;
+    if (propertyNode.isIdentifier()) {
+      property = makePropertyLiteral(propertyNode.node.name);
+    } else if (propertyNode.isNumericLiteral()) {
+      property = makePropertyLiteral(propertyNode.node.value);
+    } else {
       builder.errors.push({
         reason: `(BuildHIR::lowerMemberExpression) Handle ${propertyNode.type} property`,
         severity: ErrorSeverity.Todo,
@@ -3090,10 +3097,10 @@ function lowerMemberExpression(
     const value: InstructionValue = {
       kind: 'PropertyLoad',
       object: {...object},
-      property: propertyNode.node.name,
+      property,
       loc: exprLoc,
     };
-    return {object, property: propertyNode.node.name, value};
+    return {object, property, value};
   } else {
     if (!propertyNode.isExpression()) {
       builder.errors.push({
@@ -3211,7 +3218,7 @@ function lowerJsxMemberExpression(
   return lowerValueToTemporary(builder, {
     kind: 'PropertyLoad',
     object: objectPlace,
-    property,
+    property: makePropertyLiteral(property),
     loc,
   });
 }
@@ -3624,8 +3631,25 @@ function lowerAssignment(
       const lvalue = lvaluePath as NodePath<t.MemberExpression>;
       const property = lvalue.get('property');
       const object = lowerExpressionToTemporary(builder, lvalue.get('object'));
-      if (!lvalue.node.computed) {
-        if (!property.isIdentifier()) {
+      if (!lvalue.node.computed || lvalue.get('property').isNumericLiteral()) {
+        let temporary;
+        if (property.isIdentifier()) {
+          temporary = lowerValueToTemporary(builder, {
+            kind: 'PropertyStore',
+            object,
+            property: makePropertyLiteral(property.node.name),
+            value,
+            loc,
+          });
+        } else if (property.isNumericLiteral()) {
+          temporary = lowerValueToTemporary(builder, {
+            kind: 'PropertyStore',
+            object,
+            property: makePropertyLiteral(property.node.value),
+            value,
+            loc,
+          });
+        } else {
           builder.errors.push({
             reason: `(BuildHIR::lowerAssignment) Handle ${property.type} properties in MemberExpression`,
             severity: ErrorSeverity.Todo,
@@ -3634,13 +3658,6 @@ function lowerAssignment(
           });
           return {kind: 'UnsupportedNode', node: lvalueNode, loc};
         }
-        const temporary = lowerValueToTemporary(builder, {
-          kind: 'PropertyStore',
-          object,
-          property: property.node.name,
-          value,
-          loc,
-        });
         return {kind: 'LoadLocal', place: temporary, loc: temporary.loc};
       } else {
         if (!property.isExpression()) {

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
@@ -18,6 +18,7 @@ import {
   IdentifierId,
   InstructionId,
   InstructionValue,
+  PropertyLiteral,
   ReactiveScopeDependency,
   ScopeId,
 } from './HIR';
@@ -172,8 +173,8 @@ export type BlockInfo = {
  * and make computing sets intersections simpler.
  */
 type RootNode = {
-  properties: Map<string, PropertyPathNode>;
-  optionalProperties: Map<string, PropertyPathNode>;
+  properties: Map<PropertyLiteral, PropertyPathNode>;
+  optionalProperties: Map<PropertyLiteral, PropertyPathNode>;
   parent: null;
   // Recorded to make later computations simpler
   fullPath: ReactiveScopeDependency;
@@ -183,8 +184,8 @@ type RootNode = {
 
 type PropertyPathNode =
   | {
-      properties: Map<string, PropertyPathNode>;
-      optionalProperties: Map<string, PropertyPathNode>;
+      properties: Map<PropertyLiteral, PropertyPathNode>;
+      optionalProperties: Map<PropertyLiteral, PropertyPathNode>;
       parent: PropertyPathNode;
       fullPath: ReactiveScopeDependency;
       hasOptional: boolean;

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts
@@ -16,6 +16,7 @@ import {
   DependencyPathEntry,
   Instruction,
   Terminal,
+  PropertyLiteral,
 } from './HIR';
 import {printIdentifier} from './PrintHIR';
 
@@ -157,7 +158,7 @@ function matchOptionalTestBlock(
   blocks: ReadonlyMap<BlockId, BasicBlock>,
 ): {
   consequentId: IdentifierId;
-  property: string;
+  property: PropertyLiteral;
   propertyId: IdentifierId;
   storeLocalInstr: Instruction;
   consequentGoto: BlockId;

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts
@@ -10,6 +10,7 @@ import {
   DependencyPathEntry,
   GeneratedSource,
   Identifier,
+  PropertyLiteral,
   ReactiveScopeDependency,
 } from '../HIR';
 import {printIdentifier} from '../HIR/PrintHIR';
@@ -303,7 +304,7 @@ function merge(
 }
 
 type TreeNode<T extends string> = {
-  properties: Map<string, TreeNode<T>>;
+  properties: Map<PropertyLiteral, TreeNode<T>>;
   accessType: T;
 };
 type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
@@ -359,7 +360,7 @@ function printSubtree(
 
 function makeOrMergeProperty(
   node: DependencyNode,
-  property: string,
+  property: PropertyLiteral,
   accessType: PropertyAccessType,
 ): DependencyNode {
   let child = node.properties.get(property);

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
@@ -941,7 +941,7 @@ export type InstructionValue =
   | {
       kind: 'PropertyStore';
       object: Place;
-      property: string;
+      property: PropertyLiteral;
       value: Place;
       loc: SourceLocation;
     }
@@ -951,7 +951,7 @@ export type InstructionValue =
   | {
       kind: 'PropertyDelete';
       object: Place;
-      property: string;
+      property: PropertyLiteral;
       loc: SourceLocation;
     }
 
@@ -1125,7 +1125,7 @@ export type StoreLocal = {
 export type PropertyLoad = {
   kind: 'PropertyLoad';
   object: Place;
-  property: string;
+  property: PropertyLiteral;
   loc: SourceLocation;
 };
 
@@ -1507,7 +1507,17 @@ export type ReactiveScopeDeclaration = {
   scope: ReactiveScope; // the scope in which the variable was originally declared
 };
 
-export type DependencyPathEntry = {property: string; optional: boolean};
+const opaquePropertyLiteral = Symbol();
+export type PropertyLiteral = (string | number) & {
+  [opaquePropertyLiteral]: 'PropertyLiteral';
+};
+export function makePropertyLiteral(value: string | number): PropertyLiteral {
+  return value as PropertyLiteral;
+}
+export type DependencyPathEntry = {
+  property: PropertyLiteral;
+  optional: boolean;
+};
 export type DependencyPath = Array<DependencyPathEntry>;
 export type ReactiveScopeDependency = {
   identifier: Identifier;

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
@@ -34,6 +34,7 @@ import {
   ReactiveValue,
   ReactiveScopeBlock,
   PrunedReactiveScopeBlock,
+  PropertyLiteral,
 } from './HIR';
 import {
   collectHoistablePropertyLoads,
@@ -947,7 +948,7 @@ function collectTemporariesSidemapImpl(
 
 function getProperty(
   object: Place,
-  propertyName: string,
+  propertyName: PropertyLiteral,
   optional: boolean,
   temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
 ): ReactiveScopeDependency {
@@ -1148,7 +1149,11 @@ class Context {
     );
   }
 
-  visitProperty(object: Place, property: string, optional: boolean): void {
+  visitProperty(
+    object: Place,
+    property: PropertyLiteral,
+    optional: boolean,
+  ): void {
     const nextDependency = getProperty(
       object,
       property,

diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts
@@ -6,6 +6,7 @@
  */
 
 import {CompilerError} from '../CompilerError';
+import {PropertyLiteral} from './HIR';
 
 export type BuiltInType = PrimitiveType | FunctionType | ObjectType;
 
@@ -59,7 +60,7 @@ export type PropType = {
   kind: 'Property';
   objectType: Type;
   objectName: string;
-  propertyName: string;
+  propertyName: PropertyLiteral;
 };
 
 export type ObjectMethod = {

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
@@ -145,9 +145,10 @@ function collectTemporaries(
     }
     case 'PropertyLoad': {
       if (sidemap.react.has(value.object.identifier.id)) {
-        if (value.property === 'useMemo' || value.property === 'useCallback') {
+        const property = value.property;
+        if (property === 'useMemo' || property === 'useCallback') {
           sidemap.manualMemos.set(instr.lvalue.identifier.id, {
-            kind: value.property,
+            kind: property as 'useMemo' | 'useCallback',
             loadInstr: instr as TInstruction<PropertyLoad>,
           });
         }

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts
@@ -19,6 +19,7 @@ import {
   Primitive,
   assertConsistentIdentifiers,
   assertTerminalSuccessorsExist,
+  makePropertyLiteral,
   markInstructionIds,
   markPredecessors,
   mergeConsecutiveBlocks,
@@ -238,13 +239,14 @@ function evaluateInstruction(
       if (
         property !== null &&
         property.kind === 'Primitive' &&
-        typeof property.value === 'string' &&
-        isValidIdentifier(property.value)
+        ((typeof property.value === 'string' &&
+          isValidIdentifier(property.value)) ||
+          typeof property.value === 'number')
       ) {
         const nextValue: InstructionValue = {
           kind: 'PropertyLoad',
           loc: value.loc,
-          property: property.value,
+          property: makePropertyLiteral(property.value),
           object: value.object,
         };
         instr.value = nextValue;
@@ -256,13 +258,14 @@ function evaluateInstruction(
       if (
         property !== null &&
         property.kind === 'Primitive' &&
-        typeof property.value === 'string' &&
-        isValidIdentifier(property.value)
+        ((typeof property.value === 'string' &&
+          isValidIdentifier(property.value)) ||
+          typeof property.value === 'number')
       ) {
         const nextValue: InstructionValue = {
           kind: 'PropertyStore',
           loc: value.loc,
-          property: property.value,
+          property: makePropertyLiteral(property.value),
           object: value.object,
           value: value.value,
         };

diff --git a/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts b/compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts
@@ -21,6 +21,7 @@ import {
   InstructionKind,
   JsxAttribute,
   makeInstructionId,
+  makePropertyLiteral,
   ObjectProperty,
   Phi,
   Place,
@@ -444,7 +445,7 @@ function createSymbolProperty(
     value: {
       kind: 'PropertyLoad',
       object: {...symbolInstruction.lvalue},
-      property: 'for',
+      property: makePropertyLiteral('for'),
       loc: instr.value.loc,
     },
     loc: instr.loc,