diff --git a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts index acee9595550..cb475c6be85 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -77,7 +77,11 @@ import {flattenScopesWithHooksOrUseHIR} from '../ReactiveScopes/FlattenScopesWit import {pruneAlwaysInvalidatingScopes} from '../ReactiveScopes/PruneAlwaysInvalidatingScopes'; import pruneInitializationDependencies from '../ReactiveScopes/PruneInitializationDependencies'; import {stabilizeBlockIds} from '../ReactiveScopes/StabilizeBlockIds'; -import {eliminateRedundantPhi, enterSSA, leaveSSA} from '../SSA'; +import { + eliminateRedundantPhi, + enterSSA, + rewriteInstructionKindsBasedOnReassignment, +} from '../SSA'; import {inferTypes} from '../TypeInference'; import { logCodegenFunction, @@ -98,6 +102,7 @@ import { } from '../Validation'; import {validateLocalsNotReassignedAfterRender} from '../Validation/ValidateLocalsNotReassignedAfterRender'; import {outlineFunctions} from '../Optimization/OutlineFunctions'; +import {propagatePhiTypes} from '../TypeInference/PropagatePhiTypes'; export type CompilerPipelineValue = | {kind: 'ast'; name: string; value: CodegenFunction} @@ -237,8 +242,19 @@ function* runWithEnvironment( inferReactivePlaces(hir); yield log({kind: 'hir', name: 'InferReactivePlaces', value: hir}); - leaveSSA(hir); - yield log({kind: 'hir', name: 'LeaveSSA', value: hir}); + rewriteInstructionKindsBasedOnReassignment(hir); + yield log({ + kind: 'hir', + name: 'RewriteInstructionKindsBasedOnReassignment', + value: hir, + }); + + propagatePhiTypes(hir); + yield log({ + kind: 'hir', + name: 'PropagatePhiTypes', + value: hir, + }); inferReactiveScopeVariables(hir); yield log({kind: 'hir', name: 'InferReactiveScopeVariables', value: hir}); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts index 1dc228f4f39..f249329e0c9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1248,6 +1248,9 @@ export function makeIdentifierName(name: string): ValidatedIdentifier { /** * Given an unnamed identifier, promote it to a named identifier. + * + * Note: this uses the identifier's DeclarationId to ensure that all + * instances of the same declaration will have the same name. */ export function promoteTemporary(identifier: Identifier): void { CompilerError.invariant(identifier.name === null, { @@ -1258,7 +1261,7 @@ export function promoteTemporary(identifier: Identifier): void { }); identifier.name = { kind: 'promoted', - value: `#t${identifier.id}`, + value: `#t${identifier.declarationId}`, }; } @@ -1269,6 +1272,9 @@ export function isPromotedTemporary(name: string): boolean { /** * Given an unnamed identifier, promote it to a named identifier, distinguishing * it as a value that needs to be capitalized since it appears in JSX element tag position + * + * Note: this uses the identifier's DeclarationId to ensure that all + * instances of the same declaration will have the same name. */ export function promoteTemporaryJsxTag(identifier: Identifier): void { CompilerError.invariant(identifier.name === null, { @@ -1279,7 +1285,7 @@ export function promoteTemporaryJsxTag(identifier: Identifier): void { }); identifier.name = { kind: 'promoted', - value: `#T${identifier.id}`, + value: `#T${identifier.declarationId}`, }; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts index a0247e093bd..c694cf310fb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIRBuilder.ts @@ -899,3 +899,16 @@ export function createTemporaryPlace( loc: GeneratedSource, }; } + +/** + * Clones an existing Place, returning a new temporary Place that shares the + * same metadata properties as the original place (effect, reactive flag, type) + * but has a new, temporary Identifier. + */ +export function clonePlaceToTemporary(env: Environment, place: Place): Place { + const temp = createTemporaryPlace(env, place.loc); + temp.effect = place.effect; + temp.identifier.type = place.identifier.type; + temp.reactive = place.reactive; + return temp; +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts index 47263ebda24..b18e19606ce 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/AnalyseFunctions.ts @@ -20,7 +20,7 @@ import { } from '../HIR'; import {deadCodeElimination} from '../Optimization'; import {inferReactiveScopeVariables} from '../ReactiveScopes'; -import {leaveSSA} from '../SSA'; +import {rewriteInstructionKindsBasedOnReassignment} from '../SSA'; import {logHIRFunction} from '../Utils/logger'; import {inferMutableContextVariables} from './InferMutableContextVariables'; import {inferMutableRanges} from './InferMutableRanges'; @@ -108,7 +108,7 @@ function lower(func: HIRFunction): void { inferReferenceEffects(func, {isFunctionExpression: true}); deadCodeElimination(func); inferMutableRanges(func); - leaveSSA(func); + rewriteInstructionKindsBasedOnReassignment(func); inferReactiveScopeVariables(func); inferMutableContextVariables(func); logHIRFunction('AnalyseFunction (inner)', func); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts index 980e055ec66..6fd94dac09b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -18,6 +18,7 @@ import {Environment, EnvironmentConfig, ExternalFunction} from '../HIR'; import { ArrayPattern, BlockId, + DeclarationId, GeneratedSource, Identifier, IdentifierId, @@ -309,9 +310,9 @@ function codegenReactiveFunction( ): Result { for (const param of fn.params) { if (param.kind === 'Identifier') { - cx.temp.set(param.identifier.id, null); + cx.temp.set(param.identifier.declarationId, null); } else { - cx.temp.set(param.place.identifier.id, null); + cx.temp.set(param.place.identifier.declarationId, null); } } @@ -392,7 +393,11 @@ class Context { env: Environment; fnName: string; #nextCacheIndex: number = 0; - #declarations: Set = new Set(); + /** + * Tracks which named variables have been declared to dedupe declarations, + * so this uses DeclarationId instead of IdentifierId + */ + #declarations: Set = new Set(); temp: Temporaries; errors: CompilerError = new CompilerError(); objectMethods: Map = new Map(); @@ -418,11 +423,11 @@ class Context { } declare(identifier: Identifier): void { - this.#declarations.add(identifier.id); + this.#declarations.add(identifier.declarationId); } hasDeclared(identifier: Identifier): boolean { - return this.#declarations.has(identifier.id); + return this.#declarations.has(identifier.declarationId); } synthesizeName(name: string): ValidIdentifierName { @@ -1147,7 +1152,7 @@ function codegenTerminal( let catchParam = null; if (terminal.handlerBinding !== null) { catchParam = convertIdentifier(terminal.handlerBinding.identifier); - cx.temp.set(terminal.handlerBinding.identifier.id, null); + cx.temp.set(terminal.handlerBinding.identifier.declarationId, null); } return t.tryStatement( codegenBlock(cx, terminal.block), @@ -1205,7 +1210,7 @@ function codegenInstructionNullable( kind !== InstructionKind.Reassign && place.identifier.name === null ) { - cx.temp.set(place.identifier.id, null); + cx.temp.set(place.identifier.declarationId, null); } const isDeclared = cx.hasDeclared(place.identifier); hasReasign ||= isDeclared; @@ -1261,7 +1266,7 @@ function codegenInstructionNullable( ); if (instr.lvalue !== null) { if (instr.value.kind !== 'StoreContext') { - cx.temp.set(instr.lvalue.identifier.id, expr); + cx.temp.set(instr.lvalue.identifier.declarationId, expr); return null; } else { // Handle chained reassignments for context variables @@ -1530,7 +1535,7 @@ function createCallExpression( } } -type Temporaries = Map; +type Temporaries = Map; function codegenLabel(id: BlockId): string { return `bb${id}`; @@ -1549,7 +1554,7 @@ function codegenInstruction( } if (instr.lvalue.identifier.name === null) { // temporary - cx.temp.set(instr.lvalue.identifier.id, value); + cx.temp.set(instr.lvalue.identifier.declarationId, value); return t.emptyStatement(); } else { const expressionValue = convertValueToExpression(value); @@ -2498,7 +2503,7 @@ function codegenPlaceToExpression(cx: Context, place: Place): t.Expression { } function codegenPlace(cx: Context, place: Place): t.Expression | t.JSXText { - let tmp = cx.temp.get(place.identifier.id); + let tmp = cx.temp.get(place.identifier.declarationId); if (tmp != null) { return tmp; } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts index 0fe6a322fe1..eb2caa424e4 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/ExtractScopeDeclarationsFromDestructuring.ts @@ -6,6 +6,7 @@ */ import { + DeclarationId, Destructure, Environment, IdentifierId, @@ -17,6 +18,7 @@ import { ReactiveStatement, promoteTemporary, } from '../HIR'; +import {clonePlaceToTemporary} from '../HIR/HIRBuilder'; import {eachPatternOperand, mapPatternOperands} from '../HIR/visitors'; import { ReactiveFunctionTransform, @@ -82,7 +84,11 @@ export function extractScopeDeclarationsFromDestructuring( class State { env: Environment; - declared: Set = new Set(); + /** + * We need to track which program variables are already declared to convert + * declarations into reassignments, so we use DeclarationId + */ + declared: Set = new Set(); constructor(env: Environment) { this.env = env; @@ -92,7 +98,7 @@ class State { class Visitor extends ReactiveFunctionTransform { override visitScope(scope: ReactiveScopeBlock, state: State): void { for (const [, declaration] of scope.scope.declarations) { - state.declared.add(declaration.identifier.id); + state.declared.add(declaration.identifier.declarationId); } this.traverseScope(scope, state); } @@ -131,7 +137,7 @@ function transformDestructuring( let reassigned: Set = new Set(); let hasDeclaration = false; for (const place of eachPatternOperand(destructure.lvalue.pattern)) { - const isDeclared = state.declared.has(place.identifier.id); + const isDeclared = state.declared.has(place.identifier.declarationId); if (isDeclared) { reassigned.add(place.identifier.id); } @@ -150,15 +156,7 @@ function transformDestructuring( if (!reassigned.has(place.identifier.id)) { return place; } - const tempId = state.env.nextIdentifierId; - const temporary = { - ...place, - identifier: { - ...place.identifier, - id: tempId, - name: null, // overwritten below - }, - }; + const temporary = clonePlaceToTemporary(state.env, place); promoteTemporary(temporary.identifier); renamed.set(place, temporary); return temporary; diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts index 4e4d00a04b7..27aba91af2b 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -8,6 +8,7 @@ import {CompilerError, SourceLocation} from '..'; import {Environment} from '../HIR'; import { + DeclarationId, GeneratedSource, HIRFunction, Identifier, @@ -257,6 +258,14 @@ export function findDisjointMutableValues( fn: HIRFunction, ): DisjointSet { const scopeIdentifiers = new DisjointSet(); + + const declarations = new Map(); + function declareIdentifier(lvalue: Place): void { + if (!declarations.has(lvalue.identifier.declarationId)) { + declarations.set(lvalue.identifier.declarationId, lvalue.identifier); + } + } + for (const [_, block] of fn.body.blocks) { /* * If a phi is mutated after creation, then we need to alias all of its operands such that they @@ -264,14 +273,19 @@ export function findDisjointMutableValues( */ for (const phi of block.phis) { if ( - // The phi was reset because it was not mutated after creation phi.id.mutableRange.start + 1 !== phi.id.mutableRange.end && phi.id.mutableRange.end > (block.instructions.at(0)?.id ?? block.terminal.id) ) { - for (const [, phiId] of phi.operands) { - scopeIdentifiers.union([phi.id, phiId]); + const operands = [phi.id]; + const declaration = declarations.get(phi.id.declarationId); + if (declaration !== undefined) { + operands.push(declaration); + } + for (const [_, phiId] of phi.operands) { + operands.push(phiId); } + scopeIdentifiers.union(operands); } else if (fn.env.config.enableForest) { for (const [, phiId] of phi.operands) { scopeIdentifiers.union([phi.id, phiId]); @@ -286,9 +300,15 @@ export function findDisjointMutableValues( operands.push(instr.lvalue!.identifier); } if ( + instr.value.kind === 'DeclareLocal' || + instr.value.kind === 'DeclareContext' + ) { + declareIdentifier(instr.value.lvalue.place); + } else if ( instr.value.kind === 'StoreLocal' || instr.value.kind === 'StoreContext' ) { + declareIdentifier(instr.value.lvalue.place); if ( instr.value.lvalue.place.identifier.mutableRange.end > instr.value.lvalue.place.identifier.mutableRange.start + 1 @@ -303,6 +323,7 @@ export function findDisjointMutableValues( } } else if (instr.value.kind === 'Destructure') { for (const place of eachPatternOperand(instr.value.lvalue.pattern)) { + declareIdentifier(place); if ( place.identifier.mutableRange.end > place.identifier.mutableRange.start + 1 diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts index cddfc128fc8..1f3f8b12718 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -9,6 +9,7 @@ import { HIRFunction, IdentifierId, makeInstructionId, + MutableRange, Place, ReactiveValue, } from '../HIR'; @@ -110,12 +111,7 @@ function visit( operand.identifier.scope = fbtScope; // Expand the jsx element's range to account for its operands - fbtScope.range.start = makeInstructionId( - Math.min( - fbtScope.range.start, - operand.identifier.mutableRange.start, - ), - ); + expandFbtScopeRange(fbtScope.range, operand.identifier.mutableRange); fbtValues.add(operand.identifier.id); } } else if ( @@ -136,12 +132,7 @@ function visit( operand.identifier.scope = fbtScope; // Expand the jsx element's range to account for its operands - fbtScope.range.start = makeInstructionId( - Math.min( - fbtScope.range.start, - operand.identifier.mutableRange.start, - ), - ); + expandFbtScopeRange(fbtScope.range, operand.identifier.mutableRange); /* * NOTE: we add the operands as fbt values so that they are also @@ -169,12 +160,7 @@ function visit( operand.identifier.scope = fbtScope; // Expand the jsx element's range to account for its operands - fbtScope.range.start = makeInstructionId( - Math.min( - fbtScope.range.start, - operand.identifier.mutableRange.start, - ), - ); + expandFbtScopeRange(fbtScope.range, operand.identifier.mutableRange); } } } @@ -214,3 +200,14 @@ function isFbtJsxChild( fbtValues.has(lvalue.identifier.id) ); } + +function expandFbtScopeRange( + fbtRange: MutableRange, + extendWith: MutableRange, +): void { + if (extendWith.start !== 0) { + fbtRange.start = makeInstructionId( + Math.min(fbtRange.start, extendWith.start), + ); + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts index e2ad0df893e..2c9004e6ad9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MergeReactiveScopesThatInvalidateTogether.ts @@ -7,7 +7,7 @@ import {CompilerError} from '..'; import { - IdentifierId, + DeclarationId, InstructionId, InstructionKind, Place, @@ -28,7 +28,7 @@ import { BuiltInObjectId, } from '../HIR/ObjectShape'; import {eachInstructionLValue} from '../HIR/visitors'; -import {assertExhaustive} from '../Utils/utils'; +import {assertExhaustive, Iterable_some} from '../Utils/utils'; import {printReactiveScopeSummary} from './PrintReactiveFunction'; import { ReactiveFunctionTransform, @@ -97,22 +97,29 @@ function log(msg: string): void { } class FindLastUsageVisitor extends ReactiveFunctionVisitor { - lastUsage: Map = new Map(); + /* + * TODO LeaveSSA: use IdentifierId for more precise tracking + * Using DeclarationId is necessary for compatible output but produces suboptimal results + * in cases where a scope defines a variable, but that version is never read and always + * overwritten later. + * see reassignment-separate-scopes.js for example + */ + lastUsage: Map = new Map(); override visitPlace(id: InstructionId, place: Place, _state: void): void { - const previousUsage = this.lastUsage.get(place.identifier.id); + const previousUsage = this.lastUsage.get(place.identifier.declarationId); const lastUsage = previousUsage !== undefined ? makeInstructionId(Math.max(previousUsage, id)) : id; - this.lastUsage.set(place.identifier.id, lastUsage); + this.lastUsage.set(place.identifier.declarationId, lastUsage); } } class Transform extends ReactiveFunctionTransform { - lastUsage: Map; + lastUsage: Map; - constructor(lastUsage: Map) { + constructor(lastUsage: Map) { super(); this.lastUsage = lastUsage; } @@ -144,7 +151,7 @@ class Transform extends ReactiveFunctionTransform; + lvalues: Set; }; let current: MergedScope | null = null; const merged: Array = []; @@ -204,7 +211,9 @@ class Transform extends ReactiveFunctionTransform, + lastUsage: Map, ): void { - for (const [key] of scope.declarations) { - const lastUsedAt = lastUsage.get(key)!; + for (const [id, decl] of scope.declarations) { + const lastUsedAt = lastUsage.get(decl.identifier.declarationId)!; if (lastUsedAt < scope.range.end) { - scope.declarations.delete(key); + scope.declarations.delete(id); } } } @@ -400,8 +409,8 @@ function updateScopeDeclarations( */ function areLValuesLastUsedByScope( scope: ReactiveScope, - lvalues: Set, - lastUsage: Map, + lvalues: Set, + lastUsage: Map, ): boolean { for (const lvalue of lvalues) { const lastUsedAt = lastUsage.get(lvalue)!; @@ -454,8 +463,12 @@ function canMergeScopes( (next.scope.dependencies.size !== 0 && [...next.scope.dependencies].every( dep => - current.scope.declarations.has(dep.identifier.id) && - isAlwaysInvalidatingType(dep.identifier.type), + isAlwaysInvalidatingType(dep.identifier.type) && + Iterable_some( + current.scope.declarations.values(), + decl => + decl.identifier.declarationId === dep.identifier.declarationId, + ), )) ) { log(` outputs of prev are input to current`); @@ -492,7 +505,7 @@ function areEqualDependencies( let found = false; for (const bValue of b) { if ( - aValue.identifier === bValue.identifier && + aValue.identifier.declarationId === bValue.identifier.declarationId && areEqualPaths(aValue.path, bValue.path) ) { found = true; @@ -506,7 +519,7 @@ function areEqualDependencies( return true; } -function areEqualPaths(a: Array, b: Array): boolean { +export function areEqualPaths(a: Array, b: Array): boolean { return a.length === b.length && a.every((item, ix) => item === b[ix]); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts index 65b27fe639f..ac68050ddbf 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -8,12 +8,13 @@ import {CompilerError} from '../CompilerError'; import {GeneratedSource} from '../HIR'; import { + DeclarationId, Identifier, - IdentifierId, InstructionId, Place, PrunedReactiveScopeBlock, ReactiveFunction, + ReactiveScope, ReactiveScopeBlock, ReactiveValue, ScopeId, @@ -24,7 +25,6 @@ import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; class Visitor extends ReactiveFunctionVisitor { override visitScope(scopeBlock: ReactiveScopeBlock, state: State): void { - this.traverseScope(scopeBlock, state); for (const dep of scopeBlock.scope.dependencies) { const {identifier} = dep; if (identifier.name == null) { @@ -43,21 +43,23 @@ class Visitor extends ReactiveFunctionVisitor { promoteIdentifier(declaration.identifier, state); } } + this.traverseScope(scopeBlock, state); } override visitPrunedScope( scopeBlock: PrunedReactiveScopeBlock, state: State, ): void { - this.traversePrunedScope(scopeBlock, state); for (const [, declaration] of scopeBlock.scope.declarations) { if ( declaration.identifier.name == null && - state.pruned.get(declaration.identifier.id)?.usedOutsideScope === true + state.pruned.get(declaration.identifier.declarationId) + ?.usedOutsideScope === true ) { promoteIdentifier(declaration.identifier, state); } } + this.traversePrunedScope(scopeBlock, state); } override visitParam(place: Place, state: State): void { @@ -93,11 +95,75 @@ class Visitor extends ReactiveFunctionVisitor { } } -type JsxExpressionTags = Set; +class Visitor2 extends ReactiveFunctionVisitor { + override visitPlace(_id: InstructionId, place: Place, state: State): void { + if ( + place.identifier.name === null && + state.promoted.has(place.identifier.declarationId) + ) { + promoteIdentifier(place.identifier, state); + } + } + override visitLValue( + _id: InstructionId, + _lvalue: Place, + _state: State, + ): void { + this.visitPlace(_id, _lvalue, _state); + } + traverseScopeIdentifiers(scope: ReactiveScope, state: State): void { + for (const [, decl] of scope.declarations) { + if ( + decl.identifier.name === null && + state.promoted.has(decl.identifier.declarationId) + ) { + promoteIdentifier(decl.identifier, state); + } + } + for (const dep of scope.dependencies) { + if ( + dep.identifier.name === null && + state.promoted.has(dep.identifier.declarationId) + ) { + promoteIdentifier(dep.identifier, state); + } + } + for (const reassignment of scope.reassignments) { + if ( + reassignment.name === null && + state.promoted.has(reassignment.declarationId) + ) { + promoteIdentifier(reassignment, state); + } + } + } + override visitScope(scope: ReactiveScopeBlock, state: State): void { + this.traverseScope(scope, state); + this.traverseScopeIdentifiers(scope.scope, state); + } + override visitPrunedScope( + scopeBlock: PrunedReactiveScopeBlock, + state: State, + ): void { + this.traversePrunedScope(scopeBlock, state); + this.traverseScopeIdentifiers(scopeBlock.scope, state); + } + override visitReactiveFunctionValue( + _id: InstructionId, + _dependencies: Array, + fn: ReactiveFunction, + state: State, + ): void { + visitReactiveFunction(fn, this, state); + } +} + +type JsxExpressionTags = Set; type State = { tags: JsxExpressionTags; + promoted: Set; pruned: Map< - IdentifierId, + DeclarationId, {activeScopes: Array; usedOutsideScope: boolean} >; // true if referenced within another scope, false if only accessed outside of scopes }; @@ -108,9 +174,9 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor { override visitPlace(_id: InstructionId, place: Place, state: State): void { if ( this.activeScopes.length !== 0 && - state.pruned.has(place.identifier.id) + state.pruned.has(place.identifier.declarationId) ) { - const prunedPlace = state.pruned.get(place.identifier.id)!; + const prunedPlace = state.pruned.get(place.identifier.declarationId)!; if (prunedPlace.activeScopes.indexOf(this.activeScopes.at(-1)!) === -1) { prunedPlace.usedOutsideScope = true; } @@ -124,7 +190,7 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor { ): void { this.traverseValue(id, value, state); if (value.kind === 'JsxExpression' && value.tag.kind === 'Identifier') { - state.tags.add(value.tag.identifier.id); + state.tags.add(value.tag.identifier.declarationId); } } @@ -132,8 +198,8 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor { scopeBlock: PrunedReactiveScopeBlock, state: State, ): void { - for (const [id] of scopeBlock.scope.declarations) { - state.pruned.set(id, { + for (const [_id, decl] of scopeBlock.scope.declarations) { + state.pruned.set(decl.identifier.declarationId, { activeScopes: [...this.activeScopes], usedOutsideScope: false, }); @@ -151,6 +217,7 @@ class CollectPromotableTemporaries extends ReactiveFunctionVisitor { export function promoteUsedTemporaries(fn: ReactiveFunction): void { const state: State = { tags: new Set(), + promoted: new Set(), pruned: new Map(), }; visitReactiveFunction(fn, new CollectPromotableTemporaries(), state); @@ -161,6 +228,7 @@ export function promoteUsedTemporaries(fn: ReactiveFunction): void { } } visitReactiveFunction(fn, new Visitor(), state); + visitReactiveFunction(fn, new Visitor2(), state); } function promoteIdentifier(identifier: Identifier, state: State): void { @@ -171,9 +239,10 @@ function promoteIdentifier(identifier: Identifier, state: State): void { loc: GeneratedSource, suggestions: null, }); - if (state.tags.has(identifier.id)) { + if (state.tags.has(identifier.declarationId)) { promoteTemporaryJsxTag(identifier); } else { promoteTemporary(identifier); } + state.promoted.add(identifier.declarationId); } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts index 086ce017de5..690bdb75883 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -8,9 +8,9 @@ import {CompilerError} from '../CompilerError'; import { BlockId, + DeclarationId, GeneratedSource, Identifier, - IdentifierId, InstructionId, InstructionKind, isObjectMethodType, @@ -30,11 +30,12 @@ import { } from '../HIR/HIR'; import {eachInstructionValueOperand, eachPatternOperand} from '../HIR/visitors'; import {empty, Stack} from '../Utils/Stack'; -import {assertExhaustive} from '../Utils/utils'; +import {assertExhaustive, Iterable_some} from '../Utils/utils'; import { ReactiveScopeDependencyTree, ReactiveScopePropertyDependency, } from './DeriveMinimalDependencies'; +import {areEqualPaths} from './MergeReactiveScopesThatInvalidateTogether'; import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; /* @@ -76,9 +77,9 @@ type TemporariesUsedOutsideDefiningScope = { * tracks all relevant temporary declarations (currently LoadLocal and PropertyLoad) * and the scope where they are defined */ - declarations: Map; + declarations: Map; // temporaries used outside of their defining scope - usedOutsideDeclaringScope: Set; + usedOutsideDeclaringScope: Set; }; class FindPromotedTemporaries extends ReactiveFunctionVisitor { scopes: Array = []; @@ -107,7 +108,10 @@ class FindPromotedTemporaries extends ReactiveFunctionVisitor; +type DeclMap = Map; type Decl = { id: InstructionId; scope: Stack; @@ -280,7 +286,7 @@ class PoisonState { } class Context { - #temporariesUsedOutsideScope: Set; + #temporariesUsedOutsideScope: Set; #declarations: DeclMap = new Map(); #reassignments: Map = new Map(); // Reactive dependencies used in the current reactive scope. @@ -307,7 +313,7 @@ class Context { #scopes: Stack = empty(); poisonState: PoisonState = new PoisonState(new Set(), new Set(), false); - constructor(temporariesUsedOutsideScope: Set) { + constructor(temporariesUsedOutsideScope: Set) { this.#temporariesUsedOutsideScope = temporariesUsedOutsideScope; } @@ -377,7 +383,9 @@ class Context { } isUsedOutsideDeclaringScope(place: Place): boolean { - return this.#temporariesUsedOutsideScope.has(place.identifier.id); + return this.#temporariesUsedOutsideScope.has( + place.identifier.declarationId, + ); } /* @@ -440,8 +448,8 @@ class Context { * on itself. */ declare(identifier: Identifier, decl: Decl): void { - if (!this.#declarations.has(identifier.id)) { - this.#declarations.set(identifier.id, decl); + if (!this.#declarations.has(identifier.declarationId)) { + this.#declarations.set(identifier.declarationId, decl); } this.#reassignments.set(identifier, decl); } @@ -533,7 +541,7 @@ class Context { */ const currentDeclaration = this.#reassignments.get(identifier) ?? - this.#declarations.get(identifier.id); + this.#declarations.get(identifier.declarationId); const currentScope = this.currentScope.value?.value; return ( currentScope != null && @@ -599,14 +607,23 @@ class Context { * (all other decls e.g. `let x;` should be initialized in BuildHIR) */ const originalDeclaration = this.#declarations.get( - maybeDependency.identifier.id, + maybeDependency.identifier.declarationId, ); if ( originalDeclaration !== undefined && originalDeclaration.scope.value !== null ) { originalDeclaration.scope.each(scope => { - if (!this.#isScopeActive(scope.value)) { + if ( + !this.#isScopeActive(scope.value) && + // TODO LeaveSSA: key scope.declarations by DeclarationId + !Iterable_some( + scope.value.declarations.values(), + decl => + decl.identifier.declarationId === + maybeDependency.identifier.declarationId, + ) + ) { scope.value.declarations.set(maybeDependency.identifier.id, { identifier: maybeDependency.identifier, scope: originalDeclaration.scope.value!.value, @@ -637,11 +654,14 @@ class Context { const currentScope = this.currentScope.value?.value; if ( currentScope != null && - !Array.from(currentScope.reassignments).some( - identifier => identifier.id === place.identifier.id, + !Iterable_some( + currentScope.reassignments, + identifier => + identifier.declarationId === place.identifier.declarationId, ) && this.#checkValidDependency({identifier: place.identifier, path: []}) ) { + // TODO LeaveSSA: scope.reassignments should be keyed by declarationid currentScope.reassignments.add(place.identifier); } } @@ -680,7 +700,37 @@ class PropagationVisitor extends ReactiveFunctionVisitor { const scopeDependencies = context.enter(scope.scope, () => { this.visitBlock(scope.instructions, context); }); - scope.scope.dependencies = scopeDependencies; + for (const candidateDep of scopeDependencies) { + if ( + !Iterable_some( + scope.scope.dependencies, + existingDep => + existingDep.identifier.declarationId === + candidateDep.identifier.declarationId && + areEqualPaths(existingDep.path, candidateDep.path), + ) + ) { + scope.scope.dependencies.add(candidateDep); + } + } + /* + * TODO LeaveSSA: fix existing bug with duplicate deps and reassignments + * see fixture ssa-cascading-eliminated-phis, note that we cache `x` + * twice because its both a dep and a reassignment. + * + * for (const reassignment of scope.scope.reassignments) { + * if ( + * Iterable_some( + * scope.scope.dependencies.values(), + * dep => + * dep.identifier.declarationId === reassignment.declarationId && + * dep.path.length === 0, + * ) + * ) { + * scope.scope.reassignments.delete(reassignment); + * } + * } + */ } override visitPrunedScope( diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts index 94efd27a694..8608b322985 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneHoistedContexts.ts @@ -6,7 +6,7 @@ */ import { - Identifier, + DeclarationId, InstructionKind, ReactiveFunction, ReactiveInstruction, @@ -27,7 +27,7 @@ export function pruneHoistedContexts(fn: ReactiveFunction): void { visitReactiveFunction(fn, new Visitor(), hoistedIdentifiers); } -type HoistedIdentifiers = Set; +type HoistedIdentifiers = Set; class Visitor extends ReactiveFunctionTransform { override transformInstruction( @@ -39,13 +39,13 @@ class Visitor extends ReactiveFunctionTransform { instruction.value.kind === 'DeclareContext' && instruction.value.lvalue.kind === 'HoistedConst' ) { - state.add(instruction.value.lvalue.place.identifier); + state.add(instruction.value.lvalue.place.identifier.declarationId); return {kind: 'remove'}; } if ( instruction.value.kind === 'StoreContext' && - state.has(instruction.value.lvalue.place.identifier) + state.has(instruction.value.lvalue.place.identifier.declarationId) ) { return { kind: 'replace', diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts index aa9baec851f..2951d504853 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -7,8 +7,8 @@ import {CompilerError} from '../CompilerError'; import { + DeclarationId, Environment, - IdentifierId, InstructionId, Pattern, Place, @@ -115,9 +115,9 @@ export function pruneNonEscapingScopes(fn: ReactiveFunction): void { const state = new State(fn.env); for (const param of fn.params) { if (param.kind === 'Identifier') { - state.declare(param.identifier.id); + state.declare(param.identifier.declarationId); } else { - state.declare(param.place.identifier.id); + state.declare(param.place.identifier.declarationId); } } visitReactiveFunction(fn, new CollectDependenciesVisitor(fn.env), state); @@ -193,14 +193,14 @@ function joinAliases( type IdentifierNode = { level: MemoizationLevel; memoized: boolean; - dependencies: Set; + dependencies: Set; scopes: Set; seen: boolean; }; // A scope node describing its dependencies type ScopeNode = { - dependencies: Array; + dependencies: Array; seen: boolean; }; @@ -209,20 +209,30 @@ class State { env: Environment; /* * Maps lvalues for LoadLocal to the identifier being loaded, to resolve indirections - * in subsequent lvalues/rvalues + * in subsequent lvalues/rvalues. + * + * NOTE: this pass uses DeclarationId rather than IdentifierId because the pass is not + * aware of control-flow, only data flow via mutation. Instead of precisely modeling + * control flow, we analyze all values that may flow into a particular program variable, + * and then whether that program variable may escape (if so, the values flowing in may + * escape too). Thus we use DeclarationId to captures all values that may flow into + * a particular program variable, regardless of control flow paths. + * + * In the future when we convert to HIR everywhere this pass can account for control + * flow and use SSA ids. */ - definitions: Map = new Map(); + definitions: Map = new Map(); - identifiers: Map = new Map(); + identifiers: Map = new Map(); scopes: Map = new Map(); - escapingValues: Set = new Set(); + escapingValues: Set = new Set(); constructor(env: Environment) { this.env = env; } // Declare a new identifier, used for function id and params - declare(id: IdentifierId): void { + declare(id: DeclarationId): void { this.identifiers.set(id, { level: MemoizationLevel.Never, memoized: false, @@ -240,14 +250,16 @@ class State { visitOperand( id: InstructionId, place: Place, - identifier: IdentifierId, + identifier: DeclarationId, ): void { const scope = getPlaceScope(id, place); if (scope !== null) { let node = this.scopes.get(scope.id); if (node === undefined) { node = { - dependencies: [...scope.dependencies].map(dep => dep.identifier.id), + dependencies: [...scope.dependencies].map( + dep => dep.identifier.declarationId, + ), seen: false, }; this.scopes.set(scope.id, node); @@ -269,11 +281,11 @@ class State { * to determine which other values should be memoized. Returns a set of all identifiers * that should be memoized. */ -function computeMemoizedIdentifiers(state: State): Set { - const memoized = new Set(); +function computeMemoizedIdentifiers(state: State): Set { + const memoized = new Set(); // Visit an identifier, optionally forcing it to be memoized - function visit(id: IdentifierId, forceMemoize: boolean = false): boolean { + function visit(id: DeclarationId, forceMemoize: boolean = false): boolean { const node = state.identifiers.get(id); CompilerError.invariant(node !== undefined, { reason: `Expected a node for all identifiers, none found for \`${id}\``, @@ -832,14 +844,16 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { // Associate all the rvalues with the instruction's scope if it has one for (const operand of aliasing.rvalues) { const operandId = - state.definitions.get(operand.identifier.id) ?? operand.identifier.id; + state.definitions.get(operand.identifier.declarationId) ?? + operand.identifier.declarationId; state.visitOperand(instruction.id, operand, operandId); } // Add the operands as dependencies of all lvalues. for (const {place: lvalue, level} of aliasing.lvalues) { const lvalueId = - state.definitions.get(lvalue.identifier.id) ?? lvalue.identifier.id; + state.definitions.get(lvalue.identifier.declarationId) ?? + lvalue.identifier.declarationId; let node = state.identifiers.get(lvalueId); if (node === undefined) { node = { @@ -858,7 +872,8 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { */ for (const operand of aliasing.rvalues) { const operandId = - state.definitions.get(operand.identifier.id) ?? operand.identifier.id; + state.definitions.get(operand.identifier.declarationId) ?? + operand.identifier.declarationId; if (operandId === lvalueId) { continue; } @@ -870,8 +885,8 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { if (instruction.value.kind === 'LoadLocal' && instruction.lvalue !== null) { state.definitions.set( - instruction.lvalue.identifier.id, - instruction.value.place.identifier.id, + instruction.lvalue.identifier.declarationId, + instruction.value.place.identifier.declarationId, ); } else if ( instruction.value.kind === 'CallExpression' || @@ -897,7 +912,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { } for (const operand of instruction.value.args) { const place = operand.kind === 'Spread' ? operand.place : operand; - state.escapingValues.add(place.identifier.id); + state.escapingValues.add(place.identifier.declarationId); } } } @@ -910,20 +925,20 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { this.traverseTerminal(stmt, state); if (stmt.terminal.kind === 'return') { - state.escapingValues.add(stmt.terminal.value.identifier.id); + state.escapingValues.add(stmt.terminal.value.identifier.declarationId); } } } // Prune reactive scopes that do not have any memoized outputs class PruneScopesTransform extends ReactiveFunctionTransform< - Set + Set > { prunedScopes: Set = new Set(); override transformScope( scopeBlock: ReactiveScopeBlock, - state: Set, + state: Set, ): Transformed { this.visitScope(scopeBlock, state); @@ -945,11 +960,11 @@ class PruneScopesTransform extends ReactiveFunctionTransform< } const hasMemoizedOutput = - Array.from(scopeBlock.scope.declarations.keys()).some(id => - state.has(id), + Array.from(scopeBlock.scope.declarations.values()).some(decl => + state.has(decl.identifier.declarationId), ) || Array.from(scopeBlock.scope.reassignments).some(identifier => - state.has(identifier.id), + state.has(identifier.declarationId), ); if (hasMemoizedOutput) { return {kind: 'keep'}; @@ -964,7 +979,7 @@ class PruneScopesTransform extends ReactiveFunctionTransform< override transformInstruction( instruction: ReactiveInstruction, - state: Set, + state: Set, ): Transformed { this.traverseInstruction(instruction, state); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts index 535a4e5e911..76f48adacab 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/PruneTemporaryLValues.ts @@ -6,7 +6,7 @@ */ import { - Identifier, + DeclarationId, InstructionId, Place, ReactiveFunction, @@ -18,19 +18,27 @@ import {ReactiveFunctionVisitor, visitReactiveFunction} from './visitors'; * Nulls out lvalues for temporary variables that are never accessed later. This only * nulls out the lvalue itself, it does not remove the corresponding instructions. */ -export function pruneTemporaryLValues(fn: ReactiveFunction): void { - const lvalues = new Map(); +export function pruneUnusedLValues(fn: ReactiveFunction): void { + const lvalues = new Map(); visitReactiveFunction(fn, new Visitor(), lvalues); for (const [, instr] of lvalues) { instr.lvalue = null; } } -type LValues = Map; +/** + * This pass uses DeclarationIds because the lvalue IdentifierId of a compound expression + * (ternary, logical, optional) in ReactiveFunction may not be the same as the IdentifierId + * of the phi, and which is referenced later. Keying by DeclarationId ensures we don't + * delete lvalues for identifiers that are used. + * + * TODO LeaveSSA: once we use HIR everywhere, this can likely move back to using IdentifierId + */ +type LValues = Map; class Visitor extends ReactiveFunctionVisitor { override visitPlace(id: InstructionId, place: Place, state: LValues): void { - state.delete(place.identifier); + state.delete(place.identifier.declarationId); } override visitInstruction( instruction: ReactiveInstruction, @@ -41,7 +49,7 @@ class Visitor extends ReactiveFunctionVisitor { instruction.lvalue !== null && instruction.lvalue.identifier.name === null ) { - state.set(instruction.lvalue.identifier, instruction); + state.set(instruction.lvalue.identifier.declarationId, instruction); } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/RenameVariables.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/RenameVariables.ts index a68bdfa5bfd..f84965b92e6 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/RenameVariables.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/RenameVariables.ts @@ -7,8 +7,8 @@ import {CompilerError} from '../CompilerError'; import { + DeclarationId, Identifier, - IdentifierId, IdentifierName, InstructionId, Place, @@ -121,8 +121,8 @@ class Visitor extends ReactiveFunctionVisitor { } class Scopes { - #seen: Map = new Map(); - #stack: Array> = [new Map()]; + #seen: Map = new Map(); + #stack: Array> = [new Map()]; #globals: Set; names: Set = new Set(); @@ -135,7 +135,7 @@ class Scopes { if (originalName === null) { return; } - const mappedName = this.#seen.get(identifier.id); + const mappedName = this.#seen.get(identifier.declarationId); if (mappedName !== undefined) { identifier.name = mappedName; return; @@ -158,12 +158,12 @@ class Scopes { } const identifierName = makeIdentifierName(name); identifier.name = identifierName; - this.#seen.set(identifier.id, identifierName); - this.#stack.at(-1)!.set(identifierName.value, identifier.id); + this.#seen.set(identifier.declarationId, identifierName); + this.#stack.at(-1)!.set(identifierName.value, identifier.declarationId); this.names.add(identifierName.value); } - #lookup(name: string): IdentifierId | null { + #lookup(name: string): DeclarationId | null { for (let i = this.#stack.length - 1; i >= 0; i--) { const scope = this.#stack[i]!; const entry = scope.get(name); diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts index 8f6cad8d11f..55f67fc2f7d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/index.ts @@ -27,7 +27,7 @@ export {pruneAllReactiveScopes} from './PruneAllReactiveScopes'; export {pruneHoistedContexts} from './PruneHoistedContexts'; export {pruneNonEscapingScopes} from './PruneNonEscapingScopes'; export {pruneNonReactiveDependencies} from './PruneNonReactiveDependencies'; -export {pruneTemporaryLValues as pruneUnusedLValues} from './PruneTemporaryLValues'; +export {pruneUnusedLValues} from './PruneTemporaryLValues'; export {pruneUnusedLabels} from './PruneUnusedLabels'; export {pruneUnusedScopes} from './PruneUnusedScopes'; export {renameVariables} from './RenameVariables'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/SSA/LeaveSSA.ts b/compiler/packages/babel-plugin-react-compiler/src/SSA/LeaveSSA.ts deleted file mode 100644 index 8cf569a490e..00000000000 --- a/compiler/packages/babel-plugin-react-compiler/src/SSA/LeaveSSA.ts +++ /dev/null @@ -1,515 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -import {CompilerError} from '../CompilerError'; -import { - BasicBlock, - BlockId, - HIRFunction, - Identifier, - InstructionKind, - LValue, - LValuePattern, - Phi, - Place, -} from '../HIR/HIR'; -import {printIdentifier, printPlace} from '../HIR/PrintHIR'; -import { - eachInstructionLValue, - eachInstructionValueOperand, - eachPatternOperand, - eachTerminalOperand, - eachTerminalSuccessor, - terminalFallthrough, -} from '../HIR/visitors'; - -/* - * Removes SSA form by converting all phis into explicit bindings and assignments. There are two main categories - * of phis: - * - * ## Reassignments (operands are independently memoizable) - * - * These are phis that occur after some high-level control flow such as an if, switch, or loop. These phis are rewritten - * to add a new `let` binding for the phi id prior to the control flow node (ie prior to the if/switch), - * and to add a reassignment to that let binding in each of the phi's predecessors. - * - * Example: - * - * ```javascript - * // Input - * let x1 = null; - * if (a) { - * x2 = b; - * } else { - * x3 = c; - * } - * x4 = phi(x2, x3); - * return x4; - * - * // Output - * const x1 = null; - * let x4; // synthesized binding for the phi identifier - * if (a) { - * x2 = b; - * x4 = x2;; // sythesized assignment to the phi identifier - * } else { - * x3 = c; - * x4 = x3; // synthesized assignment - * } - * // phi removed - * return x4; - * ``` - * - * ## Rewrites (operands are not independently memoizable) - * - * Phis that occur inside loop constructs cannot use the reassignment strategy, because there isn't an appropriate place - * to add the new let binding. Instead, we select a single "canonical" id for these phis which is the operand that is - * defined first. Then, all assignments and references for any of the phi ir and operands are rewritten to reference - * the canonical id instead. - * - * Example: - * - * ```javascript - * // Input - * for ( - * let i1 = 0; - * { i2 = phi(i1, i2); i2 < 10 }; // note the phi in the test block - * i2 += 1 - * ) { ... } - * - * // Output - * for ( - * let i1 = 0; // i1 is defined first, so it becomes the canonical id - * i1 < 10; // rewritten to canonical id - * i1 += 1 // rewritten to canonical id - * ) - * ``` - */ -export function leaveSSA(fn: HIRFunction): void { - // Maps identifier names to their original declaration. - const declarations: Map< - string, - {lvalue: LValue | LValuePattern; place: Place} - > = new Map(); - - for (const param of fn.params) { - let place: Place = param.kind === 'Identifier' ? param : param.place; - if (place.identifier.name !== null) { - declarations.set(place.identifier.name.value, { - lvalue: { - kind: InstructionKind.Let, - place, - }, - place, - }); - } - } - - /* - * For non-memoizable phis, this maps original identifiers to the identifier they should be - * *rewritten* to. The keys are the original identifiers, and the value will be _either_ the - * phi id or, more typically, the operand that was defined prior to the phi. - */ - const rewrites: Map = new Map(); - - type PhiState = { - phi: Phi; - block: BasicBlock; - }; - - const seen = new Set(); - const backEdgePhis = new Set(); - for (const [, block] of fn.body.blocks) { - for (const phi of block.phis) { - for (const [pred] of phi.operands) { - if (!seen.has(pred)) { - backEdgePhis.add(phi); - break; - } - } - } - seen.add(block.id); - } - - for (const [, block] of fn.body.blocks) { - for (const instr of block.instructions) { - /* - * Iterate the instructions and perform any rewrites as well as promoting SSA variables to - * `let` or `reassign` where possible. - */ - const {lvalue, value} = instr; - if (value.kind === 'DeclareLocal') { - const name = value.lvalue.place.identifier.name; - if (name !== null) { - CompilerError.invariant(!declarations.has(name.value), { - reason: `Unexpected duplicate declaration`, - description: `Found duplicate declaration for \`${name.value}\``, - loc: value.lvalue.place.loc, - suggestions: null, - }); - declarations.set(name.value, { - lvalue: value.lvalue, - place: value.lvalue.place, - }); - } - } else if ( - value.kind === 'PrefixUpdate' || - value.kind === 'PostfixUpdate' - ) { - CompilerError.invariant(value.lvalue.identifier.name !== null, { - reason: `Expected update expression to be applied to a named variable`, - description: null, - loc: value.lvalue.loc, - suggestions: null, - }); - const originalLVal = declarations.get( - value.lvalue.identifier.name.value, - ); - CompilerError.invariant(originalLVal !== undefined, { - reason: `Expected update expression to be applied to a previously defined variable`, - description: null, - loc: value.lvalue.loc, - suggestions: null, - }); - originalLVal.lvalue.kind = InstructionKind.Let; - } else if (value.kind === 'StoreLocal') { - if (value.lvalue.place.identifier.name != null) { - const originalLVal = declarations.get( - value.lvalue.place.identifier.name.value, - ); - if ( - originalLVal === undefined || - originalLVal.lvalue === value.lvalue // in case this was pre-declared for the `for` initializer - ) { - CompilerError.invariant( - originalLVal !== undefined || - block.kind === 'block' || - block.kind === 'catch', - { - reason: `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`, - description: null, - loc: value.lvalue.place.loc, - suggestions: null, - }, - ); - declarations.set(value.lvalue.place.identifier.name.value, { - lvalue: value.lvalue, - place: value.lvalue.place, - }); - value.lvalue.kind = InstructionKind.Const; - } else { - /* - * This is an instance of the original id, so we need to promote the original declaration - * to a `let` and the current lval to a `reassign` - */ - originalLVal.lvalue.kind = InstructionKind.Let; - value.lvalue.kind = InstructionKind.Reassign; - } - } else if (rewrites.has(value.lvalue.place.identifier)) { - value.lvalue.kind = InstructionKind.Const; - } - } else if (value.kind === 'Destructure') { - let kind: InstructionKind | null = null; - for (const place of eachPatternOperand(value.lvalue.pattern)) { - if (place.identifier.name == null) { - CompilerError.invariant( - kind === null || kind === InstructionKind.Const, - { - reason: `Expected consistent kind for destructuring`, - description: `other places were \`${kind}\` but '${printPlace( - place, - )}' is const`, - loc: place.loc, - suggestions: null, - }, - ); - kind = InstructionKind.Const; - } else { - const originalLVal = declarations.get(place.identifier.name.value); - if ( - originalLVal === undefined || - originalLVal.lvalue === value.lvalue - ) { - CompilerError.invariant( - originalLVal !== undefined || block.kind !== 'value', - { - reason: `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`, - description: null, - loc: place.loc, - suggestions: null, - }, - ); - declarations.set(place.identifier.name.value, { - lvalue: value.lvalue, - place, - }); - CompilerError.invariant( - kind === null || kind === InstructionKind.Const, - { - reason: `Expected consistent kind for destructuring`, - description: `Other places were \`${kind}\` but '${printPlace( - place, - )}' is const`, - loc: place.loc, - suggestions: null, - }, - ); - kind = InstructionKind.Const; - } else { - CompilerError.invariant( - kind === null || kind === InstructionKind.Reassign, - { - reason: `Expected consistent kind for destructuring`, - description: `Other places were \`${kind}\` but '${printPlace( - place, - )}' is reassigned`, - loc: place.loc, - suggestions: null, - }, - ); - kind = InstructionKind.Reassign; - originalLVal.lvalue.kind = InstructionKind.Let; - } - } - } - CompilerError.invariant(kind !== null, { - reason: 'Expected at least one operand', - description: null, - loc: null, - suggestions: null, - }); - value.lvalue.kind = kind; - } - rewritePlace(lvalue, rewrites, declarations); - for (const operand of eachInstructionLValue(instr)) { - rewritePlace(operand, rewrites, declarations); - } - for (const operand of eachInstructionValueOperand(instr.value)) { - rewritePlace(operand, rewrites, declarations); - } - } - - const terminal = block.terminal; - for (const operand of eachTerminalOperand(terminal)) { - rewritePlace(operand, rewrites, declarations); - } - - /* - * Find any phi nodes which need a variable declaration in the current block - * This includes phis in fallthrough nodes, or blocks that form part of control flow - * such as for or while (and later if/switch). - */ - const reassignmentPhis: Array = []; - const rewritePhis: Array = []; - function pushPhis(phiBlock: BasicBlock): void { - for (const phi of phiBlock.phis) { - if (phi.id.name === null) { - rewritePhis.push({phi, block: phiBlock}); - } else { - reassignmentPhis.push({phi, block: phiBlock}); - } - } - } - const fallthroughId = terminalFallthrough(terminal); - if (fallthroughId !== null) { - const fallthrough = fn.body.blocks.get(fallthroughId)!; - pushPhis(fallthrough); - } - if (terminal.kind === 'while' || terminal.kind === 'for') { - const test = fn.body.blocks.get(terminal.test)!; - pushPhis(test); - - const loop = fn.body.blocks.get(terminal.loop)!; - pushPhis(loop); - } - if ( - terminal.kind === 'for' || - terminal.kind === 'for-of' || - terminal.kind === 'for-in' - ) { - let init = fn.body.blocks.get(terminal.init)!; - pushPhis(init); - - // The first block after the end of the init - let initContinuation = - terminal.kind === 'for' ? terminal.test : terminal.loop; - /* - * To avoid generating a let binding for the initializer prior to the loop, - * check to see if the for declares an iterator variable. - */ - const queue: Array = [init.id]; - while (queue.length !== 0) { - const blockId = queue.shift()!; - if (blockId === initContinuation) { - break; - } - const block = fn.body.blocks.get(blockId)!; - for (const instr of block.instructions) { - if ( - instr.value.kind === 'StoreLocal' && - instr.value.lvalue.kind !== InstructionKind.Reassign - ) { - const value = instr.value; - if (value.lvalue.place.identifier.name !== null) { - const originalLVal = declarations.get( - value.lvalue.place.identifier.name.value, - ); - if (originalLVal === undefined) { - declarations.set(value.lvalue.place.identifier.name.value, { - lvalue: value.lvalue, - place: value.lvalue.place, - }); - value.lvalue.kind = InstructionKind.Const; - } - } - } - } - - switch (block.terminal.kind) { - case 'maybe-throw': { - queue.push(block.terminal.continuation); - break; - } - case 'goto': { - queue.push(block.terminal.block); - break; - } - case 'branch': - case 'logical': - case 'optional': - case 'ternary': - case 'label': { - for (const successor of eachTerminalSuccessor(block.terminal)) { - queue.push(successor); - } - break; - } - default: { - break; - } - } - } - - if (terminal.kind === 'for' && terminal.update !== null) { - const update = fn.body.blocks.get(terminal.update)!; - pushPhis(update); - } - } - - for (const {phi, block: phiBlock} of reassignmentPhis) { - /* - * In some cases one of the phi operands can be defined *before* the let binding - * we will generate. For example, a variable that is only rebound in one branch of - * an if but not another. In this case we populate the let binding with this initial - * value rather than generate an extra assignment. - */ - let initOperand: Identifier | null = null; - for (const [, operand] of phi.operands) { - if (operand.mutableRange.start < terminal.id) { - if (initOperand == null) { - initOperand = operand; - } - } - } - - /* - * If the phi is mutated after its creation, then any values which flow into the phi - * must also have their ranges extended accordingly. - */ - const isPhiMutatedAfterCreation: boolean = - phi.id.mutableRange.end > - (phiBlock.instructions.at(0)?.id ?? phiBlock.terminal.id); - - /* - * If we never saw a declaration for this phi, it may have been pruned by DCE, so synthesize - * a new Let binding - */ - CompilerError.invariant(phi.id.name != null, { - reason: 'Expected reassignment phis to have a name', - description: null, - loc: null, - suggestions: null, - }); - const declaration = declarations.get(phi.id.name.value); - CompilerError.invariant(declaration != null, { - loc: null, - reason: 'Expected a declaration for all variables', - description: `${printIdentifier(phi.id)} in block bb${phiBlock.id}`, - suggestions: null, - }); - if (isPhiMutatedAfterCreation) { - /* - * The declaration is not guaranteed to flow into the phi, for example in the case of a variable - * that is reassigned in all control flow paths to a given phi. The original declaration's range - * has to be extended in this case (if the phi is later mutated) since we are reusing the original - * declaration instead of creating a new declaration. - * - * NOTE: this can *only* happen if the original declaration involves an instruction that DCE does - * not prune. Otherwise, the declaration would have been pruned and we'd synthesize a new one. - */ - declaration.place.identifier.mutableRange.end = phi.id.mutableRange.end; - } - rewrites.set(phi.id, declaration.place.identifier); - } - - /* - * Similar logic for rewrite phis that occur in loops, except that instead of a new let binding - * we pick one of the operands as the canonical id, and rewrite all references to the other - * operands and the phi to reference this canonical id. - */ - for (const {phi} of rewritePhis) { - let canonicalId = rewrites.get(phi.id); - if (canonicalId === undefined) { - canonicalId = phi.id; - for (const [, operand] of phi.operands) { - let canonicalOperand = rewrites.get(operand) ?? operand; - if (canonicalOperand.id < canonicalId.id) { - canonicalId = canonicalOperand; - } - } - rewrites.set(phi.id, canonicalId); - - if (canonicalId.name !== null) { - const declaration = declarations.get(canonicalId.name.value); - if (declaration !== undefined) { - declaration.lvalue.kind = InstructionKind.Let; - } - } - } - - // all versions of the variable need to be remapped to the canonical id - for (const [, operand] of phi.operands) { - rewrites.set(operand, canonicalId); - } - } - } -} - -/* - * Rewrite @param place's identifier based on the given rewrite mapping, if the identifier - * is present. Also expands the mutable range of the target identifier to include the - * place's range. - */ -function rewritePlace( - place: Place, - rewrites: Map, - declarations: Map, -): void { - const prevIdentifier = place.identifier; - const nextIdentifier = rewrites.get(prevIdentifier); - - if (nextIdentifier !== undefined) { - if (nextIdentifier === prevIdentifier) return; - place.identifier = nextIdentifier; - } else if (prevIdentifier.name != null) { - const declaration = declarations.get(prevIdentifier.name.value); - // Only rewrite identifiers that were declared within the function - if (declaration === undefined) return; - const originalIdentifier = declaration.place.identifier; - prevIdentifier.id = originalIdentifier.id; - } -} diff --git a/compiler/packages/babel-plugin-react-compiler/src/SSA/RewriteInstructionKindsBasedOnReassignment.ts b/compiler/packages/babel-plugin-react-compiler/src/SSA/RewriteInstructionKindsBasedOnReassignment.ts new file mode 100644 index 00000000000..0af28e0d125 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/SSA/RewriteInstructionKindsBasedOnReassignment.ts @@ -0,0 +1,174 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {CompilerError} from '../CompilerError'; +import { + DeclarationId, + HIRFunction, + InstructionKind, + LValue, + LValuePattern, + Place, +} from '../HIR/HIR'; +import {printPlace} from '../HIR/PrintHIR'; +import {eachPatternOperand} from '../HIR/visitors'; + +/** + * This pass rewrites the InstructionKind of instructions which declare/assign variables, + * converting the first declaration to a Const/Let depending on whether it is subsequently + * reassigned, and ensuring that subsequent reassignments are marked as a Reassign. Note + * that declarations which were const in the original program cannot become `let`, but the + * inverse is not true: a `let` which was reassigned in the source may be converted to a + * `const` if the reassignment is not used and was removed by dead code elimination. + * + * NOTE: this is a subset of the operations previously performed by the LeaveSSA pass. + */ +export function rewriteInstructionKindsBasedOnReassignment( + fn: HIRFunction, +): void { + const declarations = new Map(); + for (const param of fn.params) { + let place: Place = param.kind === 'Identifier' ? param : param.place; + if (place.identifier.name !== null) { + declarations.set(place.identifier.declarationId, { + kind: InstructionKind.Let, + place, + }); + } + } + for (const place of fn.context) { + if (place.identifier.name !== null) { + declarations.set(place.identifier.declarationId, { + kind: InstructionKind.Let, + place, + }); + } + } + for (const [, block] of fn.body.blocks) { + for (const instr of block.instructions) { + const {value} = instr; + switch (value.kind) { + case 'DeclareLocal': { + const lvalue = value.lvalue; + CompilerError.invariant( + !declarations.has(lvalue.place.identifier.declarationId), + { + reason: `Expected variable not to be defined prior to declaration`, + description: `${printPlace(lvalue.place)} was already defined`, + loc: lvalue.place.loc, + }, + ); + declarations.set(lvalue.place.identifier.declarationId, lvalue); + break; + } + case 'StoreLocal': { + const lvalue = value.lvalue; + if (lvalue.place.identifier.name !== null) { + const declaration = declarations.get( + lvalue.place.identifier.declarationId, + ); + if (declaration === undefined) { + CompilerError.invariant( + !declarations.has(lvalue.place.identifier.declarationId), + { + reason: `Expected variable not to be defined prior to declaration`, + description: `${printPlace(lvalue.place)} was already defined`, + loc: lvalue.place.loc, + }, + ); + declarations.set(lvalue.place.identifier.declarationId, lvalue); + lvalue.kind = InstructionKind.Const; + } else { + declaration.kind = InstructionKind.Let; + lvalue.kind = InstructionKind.Reassign; + } + } + break; + } + case 'Destructure': { + const lvalue = value.lvalue; + let kind: InstructionKind | null = null; + for (const place of eachPatternOperand(lvalue.pattern)) { + if (place.identifier.name === null) { + CompilerError.invariant( + kind === null || kind === InstructionKind.Const, + { + reason: `Expected consistent kind for destructuring`, + description: `other places were \`${kind}\` but '${printPlace( + place, + )}' is const`, + loc: place.loc, + suggestions: null, + }, + ); + kind = InstructionKind.Const; + } else { + const declaration = declarations.get( + place.identifier.declarationId, + ); + if (declaration === undefined) { + CompilerError.invariant(block.kind !== 'value', { + reason: `TODO: Handle reassignment in a value block where the original declaration was removed by dead code elimination (DCE)`, + description: null, + loc: place.loc, + suggestions: null, + }); + declarations.set(place.identifier.declarationId, lvalue); + CompilerError.invariant( + kind === null || kind === InstructionKind.Const, + { + reason: `Expected consistent kind for destructuring`, + description: `Other places were \`${kind}\` but '${printPlace( + place, + )}' is const`, + loc: place.loc, + suggestions: null, + }, + ); + kind = InstructionKind.Const; + } else { + CompilerError.invariant( + kind === null || kind === InstructionKind.Reassign, + { + reason: `Expected consistent kind for destructuring`, + description: `Other places were \`${kind}\` but '${printPlace( + place, + )}' is reassigned`, + loc: place.loc, + suggestions: null, + }, + ); + kind = InstructionKind.Reassign; + declaration.kind = InstructionKind.Let; + } + } + } + CompilerError.invariant(kind !== null, { + reason: 'Expected at least one operand', + description: null, + loc: null, + suggestions: null, + }); + lvalue.kind = kind; + break; + } + case 'PostfixUpdate': + case 'PrefixUpdate': { + const lvalue = value.lvalue; + const declaration = declarations.get(lvalue.identifier.declarationId); + CompilerError.invariant(declaration !== undefined, { + reason: `Expected variable to have been defined`, + description: `No declaration for ${printPlace(lvalue)}`, + loc: lvalue.loc, + }); + declaration.kind = InstructionKind.Let; + break; + } + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/SSA/index.ts b/compiler/packages/babel-plugin-react-compiler/src/SSA/index.ts index 0e9212e78de..9ce9f5b21d7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/SSA/index.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/SSA/index.ts @@ -7,4 +7,4 @@ export {eliminateRedundantPhi} from './EliminateRedundantPhi'; export {default as enterSSA} from './EnterSSA'; -export {leaveSSA} from './LeaveSSA'; +export {rewriteInstructionKindsBasedOnReassignment} from './RewriteInstructionKindsBasedOnReassignment'; diff --git a/compiler/packages/babel-plugin-react-compiler/src/TypeInference/PropagatePhiTypes.ts b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/PropagatePhiTypes.ts new file mode 100644 index 00000000000..81335c175a3 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/TypeInference/PropagatePhiTypes.ts @@ -0,0 +1,108 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {HIRFunction, IdentifierId, Type, typeEquals} from '../HIR'; + +/** + * Temporary workaround for InferTypes not propagating the types of phis. + * Previously, LeaveSSA would replace all the identifiers for each phi (operands and + * the phi itself) with a single "canonical" identifier, generally chosen as the first + * operand to flow into the phi. In case of a phi whose operand was a phi, this could + * sometimes be an operand from the earlier phi. + * + * As a result, even though InferTypes did not propagate types for phis, LeaveSSA + * could end up replacing the phi Identifier with another identifer from an operand, + * which _did_ have a type inferred. + * + * This didn't affect the initial construction of mutable ranges because InferMutableRanges + * runs before LeaveSSA - thus, the types propagated by LeaveSSA only affected later optimizations, + * notably MergeScopesThatInvalidateTogether which uses type to determine if a scope's output + * will always invalidate with its input. + * + * The long-term correct approach is to update InferTypes to infer the types of phis, + * but this is complicated because InferMutableRanges inadvertently depends on phis + * never having a known type, such that a Store effect cannot occur on a phi value. + * Once we fix InferTypes to infer phi types, then we'll also have to update InferMutableRanges + * to handle this case. + * + * As a temporary workaround, this pass propagates the type of phis and can be called + * safely *after* InferMutableRanges. Unlike LeaveSSA, this pass only propagates the + * type if all operands have the same type, it's its more correct. + */ +export function propagatePhiTypes(fn: HIRFunction): void { + /** + * We track which SSA ids have had their types propagated to handle nested ternaries, + * see the StoreLocal handling below + */ + const propagated = new Set(); + for (const [, block] of fn.body.blocks) { + for (const phi of block.phis) { + /* + * We replicate the previous LeaveSSA behavior and only propagate types for + * unnamed variables. LeaveSSA would have chosen one of the operands as the + * canonical id and taken its type as the type of all identifiers. We're + * more conservative and only propagate if the types are the same and the + * phi didn't have a type inferred. + * + * Note that this can change output slightly in cases such as + * `cond ?
: null`. + * + * Previously the first operand's type (BuiltInJsx) would have been propagated, + * and this expression may have been merged with subsequent reactive scopes + * since it appears (based on that type) to always invalidate. + * + * But the correct type is `BuiltInJsx | null`, which we can't express and + * so leave as a generic `Type`, which does not always invalidate and therefore + * does not merge with subsequent scopes. + * + * We also don't propagate scopes for named variables, to preserve compatibility + * with previous LeaveSSA behavior. + */ + if (phi.id.type.kind !== 'Type' || phi.id.name !== null) { + continue; + } + let type: Type | null = null; + for (const [, operand] of phi.operands) { + if (type === null) { + type = operand.type; + } else if (!typeEquals(type, operand.type)) { + type = null; + break; + } + } + if (type !== null) { + phi.id.type = type; + phi.type = type; + propagated.add(phi.id.id); + } + } + for (const instr of block.instructions) { + const {value} = instr; + switch (value.kind) { + case 'StoreLocal': { + /** + * Nested ternaries can lower to a form with an intermediate StoreLocal where + * the value.lvalue is the temporary of the outer ternary, and the value.value + * is the result of the inner ternary. + * + * This is a common pattern in practice and easy enough to support. Again, the + * long-term approach is to update InferTypes and InferMutableRanges. + */ + const lvalue = value.lvalue.place; + if ( + propagated.has(value.value.identifier.id) && + lvalue.identifier.type.kind === 'Type' && + lvalue.identifier.name === null + ) { + lvalue.identifier.type = value.value.identifier.type; + propagated.add(lvalue.identifier.id); + } + } + } + } + } +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts index f46382a039a..6e07e14a8d0 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Utils/utils.ts @@ -93,6 +93,18 @@ export function Set_union(a: Set, b: Set): Set { return union; } +export function Iterable_some( + iter: Iterable, + pred: (item: T) => boolean, +): boolean { + for (const item of iter) { + if (pred(item)) { + return true; + } + } + return false; +} + export function nonNull, U>( value: T | null | undefined, ): value is T { diff --git a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts index 3d2b01725c6..f0009601f1d 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Validation/ValidatePreservedManualMemoization.ts @@ -7,6 +7,7 @@ import {CompilerError, Effect, ErrorSeverity} from '..'; import { + DeclarationId, GeneratedSource, Identifier, IdentifierId, @@ -82,7 +83,7 @@ type ManualMemoBlockState = { * } else { ... } * ``` */ - decls: Set; + decls: Set; /* * normalized depslist from useMemo/useCallback @@ -204,7 +205,7 @@ function compareDeps( function validateInferredDep( dep: ReactiveScopeDependency, temporaries: Map, - declsWithinMemoBlock: Set, + declsWithinMemoBlock: Set, validDepsInMemoBlock: Array, errorState: CompilerError, memoLocation: SourceLocation, @@ -240,7 +241,7 @@ function validateInferredDep( for (const decl of declsWithinMemoBlock) { if ( normalizedDep.root.kind === 'NamedLocal' && - decl === normalizedDep.root.value.identifier.id + decl === normalizedDep.root.value.identifier.declarationId ) { return; } @@ -323,7 +324,9 @@ class Visitor extends ReactiveFunctionVisitor { const dep = collectMaybeMemoDependencies(value, this.temporaries); if (value.kind === 'StoreLocal' || value.kind === 'StoreContext') { const storeTarget = value.lvalue.place; - state.manualMemoState?.decls.add(storeTarget.identifier.id); + state.manualMemoState?.decls.add( + storeTarget.identifier.declarationId, + ); if (storeTarget.identifier.name?.kind === 'named' && dep == null) { const dep: ManualMemoDependency = { root: { @@ -343,15 +346,14 @@ class Visitor extends ReactiveFunctionVisitor { recordTemporaries(instr: ReactiveInstruction, state: VisitorState): void { const temporaries = this.temporaries; - const {value} = instr; - const lvalId = instr.lvalue?.identifier.id; + const {lvalue, value} = instr; + const lvalId = lvalue?.identifier.id; if (lvalId != null && temporaries.has(lvalId)) { return; } - const isNamedLocal = - lvalId != null && instr.lvalue?.identifier.name?.kind === 'named'; - if (isNamedLocal && state.manualMemoState != null) { - state.manualMemoState.decls.add(lvalId); + const isNamedLocal = lvalue?.identifier.name?.kind === 'named'; + if (lvalue !== null && isNamedLocal && state.manualMemoState != null) { + state.manualMemoState.decls.add(lvalue.identifier.declarationId); } const maybeDep = this.recordDepsInValue(value, state); diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-repro-invalid-mutable-range-destructured-prop.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-repro-invalid-mutable-range-destructured-prop.expect.md new file mode 100644 index 00000000000..d56f7a98ad1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-repro-invalid-mutable-range-destructured-prop.expect.md @@ -0,0 +1,90 @@ + +## Input + +```javascript +import {fbt} from 'fbt'; +import {useMemo} from 'react'; +import {ValidateMemoization} from 'shared-runtime'; + +function Component({data}) { + const el = useMemo( + () => ( + + {data.name ?? ''} + + ), + [data.name] + ); + return ; +} + +const props1 = {data: {name: 'Mike'}}; +const props2 = {data: {name: 'Mofei'}}; +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [props1], + sequentialRenders: [props1, props2, props2, props1, {...props1}], +}; + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +import { fbt } from "fbt"; +import { useMemo } from "react"; +import { ValidateMemoization } from "shared-runtime"; + +function Component(t0) { + const $ = _c(7); + const { data } = t0; + let t1; + let t2; + if ($[0] !== data.name) { + t2 = fbt._("{name}", [fbt._param("name", data.name ?? "")], { + hk: "csQUH", + }); + $[0] = data.name; + $[1] = t2; + } else { + t2 = $[1]; + } + t1 = t2; + const el = t1; + let t3; + if ($[2] !== data.name) { + t3 = [data.name]; + $[2] = data.name; + $[3] = t3; + } else { + t3 = $[3]; + } + let t4; + if ($[4] !== t3 || $[5] !== el) { + t4 = ; + $[4] = t3; + $[5] = el; + $[6] = t4; + } else { + t4 = $[6]; + } + return t4; +} + +const props1 = { data: { name: "Mike" } }; +const props2 = { data: { name: "Mofei" } }; +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [props1], + sequentialRenders: [props1, props2, props2, props1, { ...props1 }], +}; + +``` + +### Eval output +(kind: ok)
{"inputs":["Mike"],"output":"Mike"}
+
{"inputs":["Mofei"],"output":"Mofei"}
+
{"inputs":["Mofei"],"output":"Mofei"}
+
{"inputs":["Mike"],"output":"Mike"}
+
{"inputs":["Mike"],"output":"Mike"}
\ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-repro-invalid-mutable-range-destructured-prop.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-repro-invalid-mutable-range-destructured-prop.js new file mode 100644 index 00000000000..a948755bb67 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/fbt/fbt-repro-invalid-mutable-range-destructured-prop.js @@ -0,0 +1,23 @@ +import {fbt} from 'fbt'; +import {useMemo} from 'react'; +import {ValidateMemoization} from 'shared-runtime'; + +function Component({data}) { + const el = useMemo( + () => ( + + {data.name ?? ''} + + ), + [data.name] + ); + return ; +} + +const props1 = {data: {name: 'Mike'}}; +const props2 = {data: {name: 'Mofei'}}; +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [props1], + sequentialRenders: [props1, props2, props2, props1, {...props1}], +}; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-jsx.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-jsx.expect.md new file mode 100644 index 00000000000..1d4a4b5d671 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-jsx.expect.md @@ -0,0 +1,56 @@ + +## Input + +```javascript +function V0({v1, v2}: V3<{v1: any, v2: V4}>): V12.V11 { + const v5 = v1.v6?.v7; + return ( + + {v5 != null ? ( + + + + ) : ( + + )} + + ); +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function V0(t0) { + const $ = _c(4); + const { v1, v2 } = t0; + const v5 = v1.v6?.v7; + let t1; + if ($[0] !== v5 || $[1] !== v1 || $[2] !== v2) { + t1 = ( + + {v5 != null ? ( + + + + ) : ( + + )} + + ); + $[0] = v5; + $[1] = v1; + $[2] = v2; + $[3] = t1; + } else { + t1 = $[3]; + } + return t1; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-jsx.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-jsx.js new file mode 100644 index 00000000000..476ec9e35ac --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-jsx.js @@ -0,0 +1,14 @@ +function V0({v1, v2}: V3<{v1: any, v2: V4}>): V12.V11 { + const v5 = v1.v6?.v7; + return ( + + {v5 != null ? ( + + + + ) : ( + + )} + + ); +} diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-nested.expect.md b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-nested.expect.md new file mode 100644 index 00000000000..609231ca8da --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-nested.expect.md @@ -0,0 +1,72 @@ + +## Input + +```javascript +function V0({v1}: V2<{v1?: V3}>): V2b.V2a { + const v4 = v5(V6.v7({v8: V9.va})); + const vb = ( + + gmhubcw + {v1 === V3.V13 ? ( + + iawyneijcgamsfgrrjyvhjrrqvzexxwenxqoknnilmfloafyvnvkqbssqnxnexqvtcpvjysaiovjxyqrorqskfph + + ) : v16.v17('pyorztRC]EJzVuP^e') ? ( + + goprinbjmmjhfserfuqyluxcewpyjihektogc + + ) : ( + + yejarlvudihqdrdgpvahovggdnmgnueedxpbwbkdvvkdhqwrtoiual + + )} + hflmn + + ); + return vb; +} + +``` + +## Code + +```javascript +import { c as _c } from "react/compiler-runtime"; +function V0(t0) { + const $ = _c(2); + const { v1 } = t0; + v5(V6.v7({ v8: V9.va })); + let t1; + if ($[0] !== v1) { + t1 = ( + + gmhubcw + {v1 === V3.V13 ? ( + + iawyneijcgamsfgrrjyvhjrrqvzexxwenxqoknnilmfloafyvnvkqbssqnxnexqvtcpvjysaiovjxyqrorqskfph + + ) : v16.v17("pyorztRC]EJzVuP^e") ? ( + + goprinbjmmjhfserfuqyluxcewpyjihektogc + + ) : ( + + yejarlvudihqdrdgpvahovggdnmgnueedxpbwbkdvvkdhqwrtoiual + + )} + hflmn + + ); + $[0] = v1; + $[1] = t1; + } else { + t1 = $[1]; + } + const vb = t1; + return vb; +} + +``` + +### Eval output +(kind: exception) Fixture not implemented \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-nested.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-nested.js new file mode 100644 index 00000000000..74822160ae1 --- /dev/null +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/repro-propagate-type-of-ternary-nested.js @@ -0,0 +1,23 @@ +function V0({v1}: V2<{v1?: V3}>): V2b.V2a { + const v4 = v5(V6.v7({v8: V9.va})); + const vb = ( + + gmhubcw + {v1 === V3.V13 ? ( + + iawyneijcgamsfgrrjyvhjrrqvzexxwenxqoknnilmfloafyvnvkqbssqnxnexqvtcpvjysaiovjxyqrorqskfph + + ) : v16.v17('pyorztRC]EJzVuP^e') ? ( + + goprinbjmmjhfserfuqyluxcewpyjihektogc + + ) : ( + + yejarlvudihqdrdgpvahovggdnmgnueedxpbwbkdvvkdhqwrtoiual + + )} + hflmn + + ); + return vb; +} diff --git a/compiler/scripts/anonymize.js b/compiler/scripts/anonymize.js new file mode 100644 index 00000000000..278e582c101 --- /dev/null +++ b/compiler/scripts/anonymize.js @@ -0,0 +1,250 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +'use strict'; + +const fs = require('fs'); +const HermesParser = require('hermes-parser'); +const BabelParser = require('@babel/parser'); +const BabelCore = require('@babel/core'); +const invariant = require('invariant'); +const {argv, stdin} = require('process'); +const prettier = require('prettier'); +const {JSXText} = require('hermes-parser/dist/generated/ESTreeVisitorKeys'); + +function runPlugin(text, file, language) { + let ast; + if (language === 'flow') { + ast = HermesParser.parse(text, { + babel: true, + flow: 'all', + sourceFilename: file, + sourceType: 'module', + enableExperimentalComponentSyntax: true, + }); + } else { + ast = BabelParser.parse(text, { + sourceFilename: file, + plugins: ['typescript', 'jsx'], + sourceType: 'module', + }); + } + const result = BabelCore.transformFromAstSync(ast, text, { + ast: false, + filename: file, + highlightCode: false, + retainLines: true, + plugins: [[AnonymizePlugin]], + sourceType: 'module', + configFile: false, + babelrc: false, + }); + invariant( + result?.code != null, + `Expected BabelPluginReactForget to codegen successfully, got: ${result}` + ); + return result.code; +} + +async function format(code, language) { + return await prettier.format(code, { + semi: true, + parser: language === 'typescript' ? 'babel-ts' : 'flow', + }); +} + +const TAG_NAMES = new Set([ + 'a', + 'body', + 'button', + 'div', + 'form', + 'head', + 'html', + 'input', + 'label', + 'select', + 'span', + 'textarea', + + // property/attribute names + 'value', + 'checked', + 'onClick', + 'onSubmit', + 'name', +]); + +const BUILTIN_HOOKS = new Set([ + 'useContext', + 'useEffect', + 'useInsertionEffect', + 'useLayoutEffect', + 'useReducer', + 'useState', +]); + +const GLOBALS = new Set([ + 'String', + 'Object', + 'Function', + 'Number', + 'RegExp', + 'Date', + 'Error', + 'Function', + 'TypeError', + 'RangeError', + 'ReferenceError', + 'SyntaxError', + 'URIError', + 'EvalError', + 'Boolean', + 'DataView', + 'Float32Array', + 'Float64Array', + 'Int8Array', + 'Int16Array', + 'Int32Array', + 'Map', + 'Set', + 'WeakMap', + 'Uint8Array', + 'Uint8ClampedArray', + 'Uint16Array', + 'Uint32Array', + 'ArrayBuffer', + 'JSON', + 'parseFloat', + 'parseInt', + 'console', + 'isNaN', + 'eval', + 'isFinite', + 'encodeURI', + 'decodeURI', + 'encodeURIComponent', + 'decodeURIComponent', + + // common method/property names of globals + 'map', + 'push', + 'at', + 'filter', + 'slice', + 'splice', + 'add', + 'get', + 'set', + 'has', + 'size', + 'length', + 'toString', +]); + +function AnonymizePlugin(_babel) { + let index = 0; + const identifiers = new Map(); + const literals = new Map(); + return { + name: 'anonymize', + visitor: { + JSXNamespacedName(path) { + throw error('TODO: handle JSXNamedspacedName'); + }, + JSXIdentifier(path) { + const name = path.node.name; + if (TAG_NAMES.has(name)) { + return; + } + let nextName = identifiers.get(name); + if (nextName == null) { + const isCapitalized = + name.slice(0, 1).toUpperCase() === name.slice(0, 1); + nextName = isCapitalized + ? `Component${(index++).toString(16).toUpperCase()}` + : `c${(index++).toString(16)}`; + identifiers.set(name, nextName); + } + path.node.name = nextName; + }, + Identifier(path) { + const name = path.node.name; + if (BUILTIN_HOOKS.has(name) || GLOBALS.has(name)) { + return; + } + let nextName = identifiers.get(name); + if (nextName == null) { + const isCapitalized = + name.slice(0, 1).toUpperCase() === name.slice(0, 1); + const prefix = isCapitalized ? 'V' : 'v'; + nextName = `${prefix}${(index++).toString(16)}`; + if (name.startsWith('use')) { + nextName = + 'use' + nextName.slice(0, 1).toUpperCase() + nextName.slice(1); + } + identifiers.set(name, nextName); + } + path.node.name = nextName; + }, + JSXText(path) { + const value = path.node.value; + let nextValue = literals.get(value); + if (nextValue == null) { + let string = ''; + while (string.length < value.length) { + string += String.fromCharCode(Math.round(Math.random() * 25) + 97); + } + nextValue = string; + literals.set(value, nextValue); + } + path.node.value = nextValue; + }, + StringLiteral(path) { + const value = path.node.value; + let nextValue = literals.get(value); + if (nextValue == null) { + let string = ''; + while (string.length < value.length) { + string += String.fromCharCode(Math.round(Math.random() * 58) + 65); + } + nextValue = string; + literals.set(value, nextValue); + } + path.node.value = nextValue; + }, + NumericLiteral(path) { + const value = path.node.value; + let nextValue = literals.get(value); + if (nextValue == null) { + nextValue = Number.isInteger(value) + ? Math.round(Math.random() * Number.MAX_SAFE_INTEGER) + : Math.random() * Number.MAX_VALUE; + literals.set(value, nextValue); + } + path.node.value = nextValue; + }, + }, + }; +} + +let file; +let text; +if (argv.length >= 3) { + file = argv[2]; + text = fs.readFileSync(file, 'utf8'); +} else { + // read from stdin + file = 'stdin.js'; + text = fs.readFileSync(stdin.fd, 'utf8'); +} +const language = + file.endsWith('.ts') || file.endsWith('.tsx') ? 'typescript' : 'flow'; +const result = runPlugin(text, file, language); +format(result, language).then(formatted => { + console.log(formatted); +});