Skip to content

Commit a9575dc

Browse files
mofeiZmofeiZ
authored
[compiler] Represent array accesses with PropertyLoad (#32287)
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.
1 parent 19cc5af commit a9575dc

26 files changed

+322
-253
lines changed

‎compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts‎

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@ import {
3636
ObjectProperty,
3737
ObjectPropertyKey,
3838
Place,
39+
PropertyLiteral,
3940
ReturnTerminal,
4041
SourceLocation,
4142
SpreadPattern,
4243
ThrowTerminal,
4344
Type,
4445
makeInstructionId,
46+
makePropertyLiteral,
4547
makeType,
4648
promoteTemporary,
4749
} from './HIR';
@@ -2017,11 +2019,11 @@ function lowerExpression(
20172019
});
20182020

20192021
// Save the result back to the property
2020-
if (typeof property === 'string') {
2022+
if (typeof property === 'string' || typeof property === 'number') {
20212023
return {
20222024
kind: 'PropertyStore',
20232025
object: {...object},
2024-
property,
2026+
property: makePropertyLiteral(property),
20252027
value: {...newValuePlace},
20262028
loc: leftExpr.node.loc ?? GeneratedSource,
20272029
};
@@ -2316,11 +2318,11 @@ function lowerExpression(
23162318
const argument = expr.get('argument');
23172319
if (argument.isMemberExpression()) {
23182320
const {object, property} = lowerMemberExpression(builder, argument);
2319-
if (typeof property === 'string') {
2321+
if (typeof property === 'string' || typeof property === 'number') {
23202322
return {
23212323
kind: 'PropertyDelete',
23222324
object,
2323-
property,
2325+
property: makePropertyLiteral(property),
23242326
loc: exprLoc,
23252327
};
23262328
} else {
@@ -2427,11 +2429,11 @@ function lowerExpression(
24272429

24282430
// Save the result back to the property
24292431
let newValuePlace;
2430-
if (typeof property === 'string') {
2432+
if (typeof property === 'string' || typeof property === 'number') {
24312433
newValuePlace = lowerValueToTemporary(builder, {
24322434
kind: 'PropertyStore',
24332435
object: {...object},
2434-
property,
2436+
property: makePropertyLiteral(property),
24352437
value: {...updatedValue},
24362438
loc: leftExpr.node.loc ?? GeneratedSource,
24372439
});
@@ -3057,7 +3059,7 @@ function lowerArguments(
30573059

30583060
type LoweredMemberExpression = {
30593061
object: Place;
3060-
property: Place | string;
3062+
property: Place | string | number;
30613063
value: InstructionValue;
30623064
};
30633065
function lowerMemberExpression(
@@ -3072,8 +3074,13 @@ function lowerMemberExpression(
30723074
const object =
30733075
loweredObject ?? lowerExpressionToTemporary(builder, objectNode);
30743076

3075-
if (!expr.node.computed) {
3076-
if (!propertyNode.isIdentifier()) {
3077+
if (!expr.node.computed || expr.node.property.type === 'NumericLiteral') {
3078+
let property: PropertyLiteral;
3079+
if (propertyNode.isIdentifier()) {
3080+
property = makePropertyLiteral(propertyNode.node.name);
3081+
} else if (propertyNode.isNumericLiteral()) {
3082+
property = makePropertyLiteral(propertyNode.node.value);
3083+
} else {
30773084
builder.errors.push({
30783085
reason: `(BuildHIR::lowerMemberExpression) Handle ${propertyNode.type} property`,
30793086
severity: ErrorSeverity.Todo,
@@ -3089,10 +3096,10 @@ function lowerMemberExpression(
30893096
const value: InstructionValue = {
30903097
kind: 'PropertyLoad',
30913098
object: {...object},
3092-
property: propertyNode.node.name,
3099+
property,
30933100
loc: exprLoc,
30943101
};
3095-
return {object, property: propertyNode.node.name, value};
3102+
return {object, property, value};
30963103
} else {
30973104
if (!propertyNode.isExpression()) {
30983105
builder.errors.push({
@@ -3210,7 +3217,7 @@ function lowerJsxMemberExpression(
32103217
return lowerValueToTemporary(builder, {
32113218
kind: 'PropertyLoad',
32123219
object: objectPlace,
3213-
property,
3220+
property: makePropertyLiteral(property),
32143221
loc,
32153222
});
32163223
}
@@ -3626,8 +3633,25 @@ function lowerAssignment(
36263633
const lvalue = lvaluePath as NodePath<t.MemberExpression>;
36273634
const property = lvalue.get('property');
36283635
const object = lowerExpressionToTemporary(builder, lvalue.get('object'));
3629-
if (!lvalue.node.computed) {
3630-
if (!property.isIdentifier()) {
3636+
if (!lvalue.node.computed || lvalue.get('property').isNumericLiteral()) {
3637+
let temporary;
3638+
if (property.isIdentifier()) {
3639+
temporary = lowerValueToTemporary(builder, {
3640+
kind: 'PropertyStore',
3641+
object,
3642+
property: makePropertyLiteral(property.node.name),
3643+
value,
3644+
loc,
3645+
});
3646+
} else if (property.isNumericLiteral()) {
3647+
temporary = lowerValueToTemporary(builder, {
3648+
kind: 'PropertyStore',
3649+
object,
3650+
property: makePropertyLiteral(property.node.value),
3651+
value,
3652+
loc,
3653+
});
3654+
} else {
36313655
builder.errors.push({
36323656
reason: `(BuildHIR::lowerAssignment) Handle ${property.type} properties in MemberExpression`,
36333657
severity: ErrorSeverity.Todo,
@@ -3636,13 +3660,6 @@ function lowerAssignment(
36363660
});
36373661
return {kind: 'UnsupportedNode', node: lvalueNode, loc};
36383662
}
3639-
const temporary = lowerValueToTemporary(builder, {
3640-
kind: 'PropertyStore',
3641-
object,
3642-
property: property.node.name,
3643-
value,
3644-
loc,
3645-
});
36463663
return {kind: 'LoadLocal', place: temporary, loc: temporary.loc};
36473664
} else {
36483665
if (!property.isExpression()) {

‎compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
IdentifierId,
1919
InstructionId,
2020
InstructionValue,
21+
PropertyLiteral,
2122
ReactiveScopeDependency,
2223
ScopeId,
2324
} from './HIR';
@@ -172,8 +173,8 @@ export type BlockInfo = {
172173
* and make computing sets intersections simpler.
173174
*/
174175
type RootNode = {
175-
properties: Map<string, PropertyPathNode>;
176-
optionalProperties: Map<string, PropertyPathNode>;
176+
properties: Map<PropertyLiteral, PropertyPathNode>;
177+
optionalProperties: Map<PropertyLiteral, PropertyPathNode>;
177178
parent: null;
178179
// Recorded to make later computations simpler
179180
fullPath: ReactiveScopeDependency;
@@ -183,8 +184,8 @@ type RootNode = {
183184

184185
type PropertyPathNode =
185186
| {
186-
properties: Map<string, PropertyPathNode>;
187-
optionalProperties: Map<string, PropertyPathNode>;
187+
properties: Map<PropertyLiteral, PropertyPathNode>;
188+
optionalProperties: Map<PropertyLiteral, PropertyPathNode>;
188189
parent: PropertyPathNode;
189190
fullPath: ReactiveScopeDependency;
190191
hasOptional: boolean;

‎compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
DependencyPathEntry,
1717
Instruction,
1818
Terminal,
19+
PropertyLiteral,
1920
} from './HIR';
2021
import {printIdentifier} from './PrintHIR';
2122

@@ -157,7 +158,7 @@ function matchOptionalTestBlock(
157158
blocks: ReadonlyMap<BlockId, BasicBlock>,
158159
): {
159160
consequentId: IdentifierId;
160-
property: string;
161+
property: PropertyLiteral;
161162
propertyId: IdentifierId;
162163
storeLocalInstr: Instruction;
163164
consequentGoto: BlockId;

‎compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
DependencyPathEntry,
1111
GeneratedSource,
1212
Identifier,
13+
PropertyLiteral,
1314
ReactiveScopeDependency,
1415
} from '../HIR';
1516
import {printIdentifier} from '../HIR/PrintHIR';
@@ -286,7 +287,7 @@ function merge(
286287
}
287288

288289
type TreeNode<T extends string> = {
289-
properties: Map<string, TreeNode<T>>;
290+
properties: Map<PropertyLiteral, TreeNode<T>>;
290291
accessType: T;
291292
};
292293
type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
@@ -343,7 +344,7 @@ function printSubtree(
343344

344345
function makeOrMergeProperty(
345346
node: DependencyNode,
346-
property: string,
347+
property: PropertyLiteral,
347348
accessType: PropertyAccessType,
348349
): DependencyNode {
349350
let child = node.properties.get(property);

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

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -937,7 +937,7 @@ export type InstructionValue =
937937
| {
938938
kind: 'PropertyStore';
939939
object: Place;
940-
property: string;
940+
property: PropertyLiteral;
941941
value: Place;
942942
loc: SourceLocation;
943943
}
@@ -947,7 +947,7 @@ export type InstructionValue =
947947
| {
948948
kind: 'PropertyDelete';
949949
object: Place;
950-
property: string;
950+
property: PropertyLiteral;
951951
loc: SourceLocation;
952952
}
953953

@@ -1121,7 +1121,7 @@ export type StoreLocal = {
11211121
export type PropertyLoad = {
11221122
kind: 'PropertyLoad';
11231123
object: Place;
1124-
property: string;
1124+
property: PropertyLiteral;
11251125
loc: SourceLocation;
11261126
};
11271127

@@ -1502,7 +1502,17 @@ export type ReactiveScopeDeclaration = {
15021502
scope: ReactiveScope; // the scope in which the variable was originally declared
15031503
};
15041504

1505-
export type DependencyPathEntry = {property: string; optional: boolean};
1505+
const opaquePropertyLiteral = Symbol();
1506+
export type PropertyLiteral = (string | number) & {
1507+
[opaquePropertyLiteral]: 'PropertyLiteral';
1508+
};
1509+
export function makePropertyLiteral(value: string | number): PropertyLiteral {
1510+
return value as PropertyLiteral;
1511+
}
1512+
export type DependencyPathEntry = {
1513+
property: PropertyLiteral;
1514+
optional: boolean;
1515+
};
15061516
export type DependencyPath = Array<DependencyPathEntry>;
15071517
export type ReactiveScopeDependency = {
15081518
identifier: Identifier;

‎compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
TInstruction,
2323
FunctionExpression,
2424
ObjectMethod,
25+
PropertyLiteral,
2526
} from './HIR';
2627
import {
2728
collectHoistablePropertyLoads,
@@ -321,7 +322,7 @@ function collectTemporariesSidemapImpl(
321322

322323
function getProperty(
323324
object: Place,
324-
propertyName: string,
325+
propertyName: PropertyLiteral,
325326
optional: boolean,
326327
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
327328
): ReactiveScopeDependency {
@@ -519,7 +520,11 @@ class Context {
519520
);
520521
}
521522

522-
visitProperty(object: Place, property: string, optional: boolean): void {
523+
visitProperty(
524+
object: Place,
525+
property: PropertyLiteral,
526+
optional: boolean,
527+
): void {
523528
const nextDependency = getProperty(
524529
object,
525530
property,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import {CompilerError} from '../CompilerError';
9+
import {PropertyLiteral} from './HIR';
910

1011
export type BuiltInType = PrimitiveType | FunctionType | ObjectType;
1112

@@ -59,7 +60,7 @@ export type PropType = {
5960
kind: 'Property';
6061
objectType: Type;
6162
objectName: string;
62-
propertyName: string;
63+
propertyName: PropertyLiteral;
6364
};
6465

6566
export type ObjectMethod = {

‎compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,9 +145,10 @@ function collectTemporaries(
145145
}
146146
case 'PropertyLoad': {
147147
if (sidemap.react.has(value.object.identifier.id)) {
148-
if (value.property === 'useMemo' || value.property === 'useCallback') {
148+
const property = value.property;
149+
if (property === 'useMemo' || property === 'useCallback') {
149150
sidemap.manualMemos.set(instr.lvalue.identifier.id, {
150-
kind: value.property,
151+
kind: property as 'useMemo' | 'useCallback',
151152
loadInstr: instr as TInstruction<PropertyLoad>,
152153
});
153154
}

‎compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
Primitive,
2020
assertConsistentIdentifiers,
2121
assertTerminalSuccessorsExist,
22+
makePropertyLiteral,
2223
markInstructionIds,
2324
markPredecessors,
2425
mergeConsecutiveBlocks,
@@ -238,13 +239,14 @@ function evaluateInstruction(
238239
if (
239240
property !== null &&
240241
property.kind === 'Primitive' &&
241-
typeof property.value === 'string' &&
242-
isValidIdentifier(property.value)
242+
((typeof property.value === 'string' &&
243+
isValidIdentifier(property.value)) ||
244+
typeof property.value === 'number')
243245
) {
244246
const nextValue: InstructionValue = {
245247
kind: 'PropertyLoad',
246248
loc: value.loc,
247-
property: property.value,
249+
property: makePropertyLiteral(property.value),
248250
object: value.object,
249251
};
250252
instr.value = nextValue;
@@ -256,13 +258,14 @@ function evaluateInstruction(
256258
if (
257259
property !== null &&
258260
property.kind === 'Primitive' &&
259-
typeof property.value === 'string' &&
260-
isValidIdentifier(property.value)
261+
((typeof property.value === 'string' &&
262+
isValidIdentifier(property.value)) ||
263+
typeof property.value === 'number')
261264
) {
262265
const nextValue: InstructionValue = {
263266
kind: 'PropertyStore',
264267
loc: value.loc,
265-
property: property.value,
268+
property: makePropertyLiteral(property.value),
266269
object: value.object,
267270
value: value.value,
268271
};

‎compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
InstructionKind,
2222
JsxAttribute,
2323
makeInstructionId,
24+
makePropertyLiteral,
2425
ObjectProperty,
2526
Phi,
2627
Place,
@@ -446,7 +447,7 @@ function createSymbolProperty(
446447
value: {
447448
kind: 'PropertyLoad',
448449
object: {...symbolInstruction.lvalue},
449-
property: 'for',
450+
property: makePropertyLiteral('for'),
450451
loc: instr.value.loc,
451452
},
452453
loc: instr.loc,

0 commit comments

Comments
 (0)