From 58a3ca3b47f6a51cea48ea95ded26c9887baca38 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Mon, 30 Sep 2024 12:24:21 -0400 Subject: [PATCH] [compiler][hir-rewrite] Cleanup Identifier -> IdentifierId Since removing ExitSSA, Identifier and IdentifierId should mean the same thing ghstack-source-id: 076cacbe8360e716b0555088043502823f9ee72e Pull Request resolved: https://github.com/facebook/react/pull/31034 --- .../src/HIR/CollectHoistablePropertyLoads.ts | 66 ++----------------- .../src/HIR/PropagateScopeDependenciesHIR.ts | 49 +++++++++++++- 2 files changed, 53 insertions(+), 62 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts index ff50113b6660c..4f642e57a65f3 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/CollectHoistablePropertyLoads.ts @@ -9,7 +9,6 @@ import { Identifier, IdentifierId, InstructionId, - Place, ReactiveScopeDependency, ScopeId, } from './HIR'; @@ -88,61 +87,6 @@ export type BlockInfo = { assumedNonNullObjects: ReadonlySet; }; -export function getProperty( - object: Place, - propertyName: string, - temporaries: ReadonlyMap, -): ReactiveScopeDependency { - /* - * (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal) - * or a deep copy of an existing property dependency. - * Example 1: - * $0 = LoadLocal x - * $1 = PropertyLoad $0.y - * getProperty($0, ...) -> resolvedObject = x, resolvedDependency = null - * - * Example 2: - * $0 = LoadLocal x - * $1 = PropertyLoad $0.y - * $2 = PropertyLoad $1.z - * getProperty($1, ...) -> resolvedObject = null, resolvedDependency = x.y - * - * Example 3: - * $0 = Call(...) - * $1 = PropertyLoad $0.y - * getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null - */ - const resolvedDependency = temporaries.get(object.identifier.id); - - /** - * (2) Push the last PropertyLoad - * TODO(mofeiZ): understand optional chaining - */ - let property: ReactiveScopeDependency; - if (resolvedDependency == null) { - property = { - identifier: object.identifier, - path: [{property: propertyName, optional: false}], - }; - } else { - property = { - identifier: resolvedDependency.identifier, - path: [ - ...resolvedDependency.path, - {property: propertyName, optional: false}, - ], - }; - } - return property; -} - -export function resolveTemporary( - place: Place, - temporaries: ReadonlyMap, -): Identifier { - return temporaries.get(place.identifier.id) ?? place.identifier; -} - /** * Tree data structure to dedupe property loads (e.g. a.b.c) * and make computing sets intersections simpler. @@ -152,7 +96,7 @@ type RootNode = { parent: null; // Recorded to make later computations simpler fullPath: ReactiveScopeDependency; - root: Identifier; + root: IdentifierId; }; type PropertyLoadNode = @@ -164,18 +108,18 @@ type PropertyLoadNode = | RootNode; class Tree { - roots: Map = new Map(); + roots: Map = new Map(); getOrCreateRoot(identifier: Identifier): PropertyLoadNode { /** * Reads from a statically scoped variable are always safe in JS, * with the exception of TDZ (not addressed by this pass). */ - let rootNode = this.roots.get(identifier); + let rootNode = this.roots.get(identifier.id); if (rootNode === undefined) { rootNode = { - root: identifier, + root: identifier.id, properties: new Map(), fullPath: { identifier, @@ -183,7 +127,7 @@ class Tree { }, parent: null, }; - this.roots.set(identifier, rootNode); + this.roots.set(identifier.id, rootNode); } return rootNode; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts index 4c7dac004d80a..1fe218c352818 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PropagateScopeDependenciesHIR.ts @@ -20,7 +20,6 @@ import { import { BlockInfo, collectHoistablePropertyLoads, - getProperty, } from './CollectHoistablePropertyLoads'; import { ScopeBlockTraversal, @@ -220,6 +219,54 @@ function collectTemporariesSidemap( return temporaries; } +function getProperty( + object: Place, + propertyName: string, + temporaries: ReadonlyMap, +): ReactiveScopeDependency { + /* + * (1) Get the base object either from the temporary sidemap (e.g. a LoadLocal) + * or a deep copy of an existing property dependency. + * Example 1: + * $0 = LoadLocal x + * $1 = PropertyLoad $0.y + * getProperty($0, ...) -> resolvedObject = x, resolvedDependency = null + * + * Example 2: + * $0 = LoadLocal x + * $1 = PropertyLoad $0.y + * $2 = PropertyLoad $1.z + * getProperty($1, ...) -> resolvedObject = null, resolvedDependency = x.y + * + * Example 3: + * $0 = Call(...) + * $1 = PropertyLoad $0.y + * getProperty($0, ...) -> resolvedObject = null, resolvedDependency = null + */ + const resolvedDependency = temporaries.get(object.identifier.id); + + /** + * (2) Push the last PropertyLoad + * TODO(mofeiZ): understand optional chaining + */ + let property: ReactiveScopeDependency; + if (resolvedDependency == null) { + property = { + identifier: object.identifier, + path: [{property: propertyName, optional: false}], + }; + } else { + property = { + identifier: resolvedDependency.identifier, + path: [ + ...resolvedDependency.path, + {property: propertyName, optional: false}, + ], + }; + } + return property; +} + type Decl = { id: InstructionId; scope: Stack;