Skip to content

Commit 38451c9

Browse files
mofeiZmofeiZ
committed
[compiler] Represent array accesses with PropertyLoad
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 faf092e commit 38451c9

26 files changed

+328
-238
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';
@@ -2019,11 +2021,11 @@ function lowerExpression(
20192021
});
20202022

20212023
// Save the result back to the property
2022-
if (typeof property === 'string') {
2024+
if (typeof property === 'string' || typeof property === 'number') {
20232025
return {
20242026
kind: 'PropertyStore',
20252027
object: {...object},
2026-
property,
2028+
property: makePropertyLiteral(property),
20272029
value: {...newValuePlace},
20282030
loc: leftExpr.node.loc ?? GeneratedSource,
20292031
};
@@ -2318,11 +2320,11 @@ function lowerExpression(
23182320
const argument = expr.get('argument');
23192321
if (argument.isMemberExpression()) {
23202322
const {object, property} = lowerMemberExpression(builder, argument);
2321-
if (typeof property === 'string') {
2323+
if (typeof property === 'string' || typeof property === 'number') {
23222324
return {
23232325
kind: 'PropertyDelete',
23242326
object,
2325-
property,
2327+
property: makePropertyLiteral(property),
23262328
loc: exprLoc,
23272329
};
23282330
} else {
@@ -2429,11 +2431,11 @@ function lowerExpression(
24292431

24302432
// Save the result back to the property
24312433
let newValuePlace;
2432-
if (typeof property === 'string') {
2434+
if (typeof property === 'string' || typeof property === 'number') {
24332435
newValuePlace = lowerValueToTemporary(builder, {
24342436
kind: 'PropertyStore',
24352437
object: {...object},
2436-
property,
2438+
property: makePropertyLiteral(property),
24372439
value: {...updatedValue},
24382440
loc: leftExpr.node.loc ?? GeneratedSource,
24392441
});
@@ -3059,7 +3061,7 @@ function lowerArguments(
30593061

30603062
type LoweredMemberExpression = {
30613063
object: Place;
3062-
property: Place | string;
3064+
property: Place | string | number;
30633065
value: InstructionValue;
30643066
};
30653067
function lowerMemberExpression(
@@ -3074,8 +3076,13 @@ function lowerMemberExpression(
30743076
const object =
30753077
loweredObject ?? lowerExpressionToTemporary(builder, objectNode);
30763078

3077-
if (!expr.node.computed) {
3078-
if (!propertyNode.isIdentifier()) {
3079+
if (!expr.node.computed || expr.node.property.type === 'NumericLiteral') {
3080+
let property: PropertyLiteral;
3081+
if (propertyNode.isIdentifier()) {
3082+
property = makePropertyLiteral(propertyNode.node.name);
3083+
} else if (propertyNode.isNumericLiteral()) {
3084+
property = makePropertyLiteral(propertyNode.node.value);
3085+
} else {
30793086
builder.errors.push({
30803087
reason: `(BuildHIR::lowerMemberExpression) Handle ${propertyNode.type} property`,
30813088
severity: ErrorSeverity.Todo,
@@ -3091,10 +3098,10 @@ function lowerMemberExpression(
30913098
const value: InstructionValue = {
30923099
kind: 'PropertyLoad',
30933100
object: {...object},
3094-
property: propertyNode.node.name,
3101+
property,
30953102
loc: exprLoc,
30963103
};
3097-
return {object, property: propertyNode.node.name, value};
3104+
return {object, property, value};
30983105
} else {
30993106
if (!propertyNode.isExpression()) {
31003107
builder.errors.push({
@@ -3212,7 +3219,7 @@ function lowerJsxMemberExpression(
32123219
return lowerValueToTemporary(builder, {
32133220
kind: 'PropertyLoad',
32143221
object: objectPlace,
3215-
property,
3222+
property: makePropertyLiteral(property),
32163223
loc,
32173224
});
32183225
}
@@ -3625,8 +3632,25 @@ function lowerAssignment(
36253632
const lvalue = lvaluePath as NodePath<t.MemberExpression>;
36263633
const property = lvalue.get('property');
36273634
const object = lowerExpressionToTemporary(builder, lvalue.get('object'));
3628-
if (!lvalue.node.computed) {
3629-
if (!property.isIdentifier()) {
3635+
if (!lvalue.node.computed || lvalue.get('property').isNumericLiteral()) {
3636+
let temporary;
3637+
if (property.isIdentifier()) {
3638+
temporary = lowerValueToTemporary(builder, {
3639+
kind: 'PropertyStore',
3640+
object,
3641+
property: makePropertyLiteral(property.node.name),
3642+
value,
3643+
loc,
3644+
});
3645+
} else if (property.isNumericLiteral()) {
3646+
temporary = lowerValueToTemporary(builder, {
3647+
kind: 'PropertyStore',
3648+
object,
3649+
property: makePropertyLiteral(property.node.value),
3650+
value,
3651+
loc,
3652+
});
3653+
} else {
36303654
builder.errors.push({
36313655
reason: `(BuildHIR::lowerAssignment) Handle ${property.type} properties in MemberExpression`,
36323656
severity: ErrorSeverity.Todo,
@@ -3635,13 +3659,6 @@ function lowerAssignment(
36353659
});
36363660
return {kind: 'UnsupportedNode', node: lvalueNode, loc};
36373661
}
3638-
const temporary = lowerValueToTemporary(builder, {
3639-
kind: 'PropertyStore',
3640-
object,
3641-
property: property.node.name,
3642-
value,
3643-
loc,
3644-
});
36453662
return {kind: 'LoadLocal', place: temporary, loc: temporary.loc};
36463663
} else {
36473664
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';
@@ -303,7 +304,7 @@ function merge(
303304
}
304305

305306
type TreeNode<T extends string> = {
306-
properties: Map<string, TreeNode<T>>;
307+
properties: Map<PropertyLiteral, TreeNode<T>>;
307308
accessType: T;
308309
};
309310
type HoistableNode = TreeNode<'Optional' | 'NonNull'>;
@@ -359,7 +360,7 @@ function printSubtree(
359360

360361
function makeOrMergeProperty(
361362
node: DependencyNode,
362-
property: string,
363+
property: PropertyLiteral,
363364
accessType: PropertyAccessType,
364365
): DependencyNode {
365366
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
@@ -941,7 +941,7 @@ export type InstructionValue =
941941
| {
942942
kind: 'PropertyStore';
943943
object: Place;
944-
property: string;
944+
property: PropertyLiteral;
945945
value: Place;
946946
loc: SourceLocation;
947947
}
@@ -951,7 +951,7 @@ export type InstructionValue =
951951
| {
952952
kind: 'PropertyDelete';
953953
object: Place;
954-
property: string;
954+
property: PropertyLiteral;
955955
loc: SourceLocation;
956956
}
957957

@@ -1125,7 +1125,7 @@ export type StoreLocal = {
11251125
export type PropertyLoad = {
11261126
kind: 'PropertyLoad';
11271127
object: Place;
1128-
property: string;
1128+
property: PropertyLiteral;
11291129
loc: SourceLocation;
11301130
};
11311131

@@ -1507,7 +1507,17 @@ export type ReactiveScopeDeclaration = {
15071507
scope: ReactiveScope; // the scope in which the variable was originally declared
15081508
};
15091509

1510-
export type DependencyPathEntry = {property: string; optional: boolean};
1510+
const opaquePropertyLiteral = Symbol();
1511+
export type PropertyLiteral = (string | number) & {
1512+
[opaquePropertyLiteral]: 'PropertyLiteral';
1513+
};
1514+
export function makePropertyLiteral(value: string | number): PropertyLiteral {
1515+
return value as PropertyLiteral;
1516+
}
1517+
export type DependencyPathEntry = {
1518+
property: PropertyLiteral;
1519+
optional: boolean;
1520+
};
15111521
export type DependencyPath = Array<DependencyPathEntry>;
15121522
export type ReactiveScopeDependency = {
15131523
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,
@@ -397,7 +398,7 @@ function collectTemporariesSidemapImpl(
397398

398399
function getProperty(
399400
object: Place,
400-
propertyName: string,
401+
propertyName: PropertyLiteral,
401402
optional: boolean,
402403
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
403404
): ReactiveScopeDependency {
@@ -598,7 +599,11 @@ class Context {
598599
);
599600
}
600601

601-
visitProperty(object: Place, property: string, optional: boolean): void {
602+
visitProperty(
603+
object: Place,
604+
property: PropertyLiteral,
605+
optional: boolean,
606+
): void {
602607
const nextDependency = getProperty(
603608
object,
604609
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,
@@ -444,7 +445,7 @@ function createSymbolProperty(
444445
value: {
445446
kind: 'PropertyLoad',
446447
object: {...symbolInstruction.lvalue},
447-
property: 'for',
448+
property: makePropertyLiteral('for'),
448449
loc: instr.value.loc,
449450
},
450451
loc: instr.loc,

0 commit comments

Comments
 (0)