Skip to content

[compiler] Represent array accesses with PropertyLoad #32287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mofeiZ merged 1 commit into from
Feb 18, 2025
Merged

[compiler] Represent array accesses with PropertyLoad #32287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 38 additions & 21 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/BuildHIR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ import {
ObjectProperty,
ObjectPropertyKey,
Place,
PropertyLiteral,
ReturnTerminal,
SourceLocation,
SpreadPattern,
ThrowTerminal,
Type,
makeInstructionId,
makePropertyLiteral,
makeType,
promoteTemporary,
} from './HIR';
Expand Down Expand Up @@ -2017,11 +2019,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,
};
Expand Down Expand Up @@ -2316,11 +2318,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 {
Expand Down Expand Up @@ -2427,11 +2429,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,
});
Expand Down Expand Up @@ -3057,7 +3059,7 @@ function lowerArguments(

type LoweredMemberExpression = {
object: Place;
property: Place | string;
property: Place | string | number;
value: InstructionValue;
};
function lowerMemberExpression(
Expand All @@ -3072,8 +3074,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,
Expand All @@ -3089,10 +3096,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({
Expand Down Expand Up @@ -3210,7 +3217,7 @@ function lowerJsxMemberExpression(
return lowerValueToTemporary(builder, {
kind: 'PropertyLoad',
object: objectPlace,
property,
property: makePropertyLiteral(property),
loc,
});
}
Expand Down Expand Up @@ -3626,8 +3633,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,
Expand All @@ -3636,13 +3660,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()) {
Expand Down
9 changes: 5 additions & 4 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
IdentifierId,
InstructionId,
InstructionValue,
PropertyLiteral,
ReactiveScopeDependency,
ScopeId,
} from './HIR';
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/CollectOptionalChainDependencies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
DependencyPathEntry,
Instruction,
Terminal,
PropertyLiteral,
} from './HIR';
import {printIdentifier} from './PrintHIR';

Expand Down Expand Up @@ -157,7 +158,7 @@ function matchOptionalTestBlock(
blocks: ReadonlyMap<BlockId, BasicBlock>,
): {
consequentId: IdentifierId;
property: string;
property: PropertyLiteral;
propertyId: IdentifierId;
storeLocalInstr: Instruction;
consequentGoto: BlockId;
Expand Down
5 changes: 3 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/DeriveMinimalDependenciesHIR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
DependencyPathEntry,
GeneratedSource,
Identifier,
PropertyLiteral,
ReactiveScopeDependency,
} from '../HIR';
import {printIdentifier} from '../HIR/PrintHIR';
Expand Down Expand Up @@ -286,7 +287,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'>;
Expand Down Expand Up @@ -343,7 +344,7 @@ function printSubtree(

function makeOrMergeProperty(
node: DependencyNode,
property: string,
property: PropertyLiteral,
accessType: PropertyAccessType,
): DependencyNode {
let child = node.properties.get(property);
Expand Down
18 changes: 14 additions & 4 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ export type InstructionValue =
| {
kind: 'PropertyStore';
object: Place;
property: string;
property: PropertyLiteral;
value: Place;
loc: SourceLocation;
}
Expand All @@ -947,7 +947,7 @@ export type InstructionValue =
| {
kind: 'PropertyDelete';
object: Place;
property: string;
property: PropertyLiteral;
loc: SourceLocation;
}

Expand Down Expand Up @@ -1121,7 +1121,7 @@ export type StoreLocal = {
export type PropertyLoad = {
kind: 'PropertyLoad';
object: Place;
property: string;
property: PropertyLiteral;
loc: SourceLocation;
};

Expand Down Expand Up @@ -1502,7 +1502,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;
}
Comment on lines +1505 to +1511
Copy link
Contributor Author

@mofeiZ mofeiZ Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the primary change: all 'static' instructions (PropertyLoad, PropertyStore, and PropertyDelete) can now operate on either string or numeric indices

export type DependencyPathEntry = {
property: PropertyLiteral;
optional: boolean;
};
export type DependencyPath = Array<DependencyPathEntry>;
export type ReactiveScopeDependency = {
identifier: Identifier;
Expand Down
9 changes: 7 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
TInstruction,
FunctionExpression,
ObjectMethod,
PropertyLiteral,
} from './HIR';
import {
collectHoistablePropertyLoads,
Expand Down Expand Up @@ -321,7 +322,7 @@ function collectTemporariesSidemapImpl(

function getProperty(
object: Place,
propertyName: string,
propertyName: PropertyLiteral,
optional: boolean,
temporaries: ReadonlyMap<IdentifierId, ReactiveScopeDependency>,
): ReactiveScopeDependency {
Expand Down Expand Up @@ -519,7 +520,11 @@ class Context {
);
}

visitProperty(object: Place, property: string, optional: boolean): void {
visitProperty(
object: Place,
property: PropertyLiteral,
optional: boolean,
): void {
const nextDependency = getProperty(
object,
property,
Expand Down
3 changes: 2 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/HIR/Types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import {CompilerError} from '../CompilerError';
import {PropertyLiteral} from './HIR';

export type BuiltInType = PrimitiveType | FunctionType | ObjectType;

Expand Down Expand Up @@ -59,7 +60,7 @@ export type PropType = {
kind: 'Property';
objectType: Type;
objectName: string;
propertyName: string;
propertyName: PropertyLiteral;
};

export type ObjectMethod = {
Expand Down
5 changes: 3 additions & 2 deletions compiler/packages/babel-plugin-react-compiler/src/Inference/DropManualMemoization.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
});
}
Expand Down
15 changes: 9 additions & 6 deletions compiler/packages/babel-plugin-react-compiler/src/Optimization/ConstantPropagation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
Primitive,
assertConsistentIdentifiers,
assertTerminalSuccessorsExist,
makePropertyLiteral,
markInstructionIds,
markPredecessors,
mergeConsecutiveBlocks,
Expand Down Expand Up @@ -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;
Expand All @@ -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,
};
Expand Down
3 changes: 2 additions & 1 deletion compiler/packages/babel-plugin-react-compiler/src/Optimization/InlineJsxTransform.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
InstructionKind,
JsxAttribute,
makeInstructionId,
makePropertyLiteral,
ObjectProperty,
Phi,
Place,
Expand Down Expand Up @@ -446,7 +447,7 @@ function createSymbolProperty(
value: {
kind: 'PropertyLoad',
object: {...symbolInstruction.lvalue},
property: 'for',
property: makePropertyLiteral('for'),
loc: instr.value.loc,
},
loc: instr.loc,
Expand Down
Loading
Loading