From 147374d71a73f3f63ff6532dc081d1175f783352 Mon Sep 17 00:00:00 2001 From: Mike Vitousek Date: Fri, 11 Oct 2024 16:19:45 -0700 Subject: [PATCH] [compiler] Kill markReactiveIdentifier and friends Summary: With the previous PR we no longer need to mark identifiers as reactive in contexts where we don't have places. We already deleted most uses of markReactiveId; the last case was to track identifiers through loadlocals etc -- but we already use a disjoint alias map that accounts for loadlocals when setting reactivity. ghstack-source-id: 69ce0a78b0729da3fe9d08177bf7d827af5325fb Pull Request resolved: https://github.com/facebook/react/pull/31178 --- .../src/Inference/InferReactivePlaces.ts | 46 ++----------------- 1 file changed, 4 insertions(+), 42 deletions(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts index 98b1ec75e1337..344949b020a99 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Inference/InferReactivePlaces.ts @@ -157,7 +157,6 @@ export function inferReactivePlaces(fn: HIRFunction): void { } do { - const identifierMapping = new Map(); for (const [, block] of fn.body.blocks) { let hasReactiveControl = isReactiveControlledBlock(block.id); @@ -233,10 +232,6 @@ export function inferReactivePlaces(fn: HIRFunction): void { case Effect.ConditionallyMutate: case Effect.Mutate: { if (isMutable(instruction, operand)) { - const resolvedId = identifierMapping.get(operand.identifier); - if (resolvedId !== undefined) { - reactiveIdentifiers.markReactiveIdentifier(resolvedId); - } reactiveIdentifiers.markReactive(operand); } break; @@ -263,31 +258,6 @@ export function inferReactivePlaces(fn: HIRFunction): void { } } } - - switch (value.kind) { - case 'LoadLocal': { - identifierMapping.set( - instruction.lvalue.identifier, - value.place.identifier, - ); - break; - } - case 'PropertyLoad': - case 'ComputedLoad': { - const resolvedId = - identifierMapping.get(value.object.identifier) ?? - value.object.identifier; - identifierMapping.set(instruction.lvalue.identifier, resolvedId); - break; - } - case 'LoadContext': { - identifierMapping.set( - instruction.lvalue.identifier, - value.place.identifier, - ); - break; - } - } } for (const operand of eachTerminalOperand(block.terminal)) { reactiveIdentifiers.isReactive(operand); @@ -372,27 +342,19 @@ class ReactivityMap { } isReactive(place: Place): boolean { - const reactive = this.isReactiveIdentifier(place.identifier); + const identifier = + this.aliasedIdentifiers.find(place.identifier) ?? place.identifier; + const reactive = this.reactive.has(identifier.id); if (reactive) { place.reactive = true; } return reactive; } - isReactiveIdentifier(inputIdentifier: Identifier): boolean { - const identifier = - this.aliasedIdentifiers.find(inputIdentifier) ?? inputIdentifier; - return this.reactive.has(identifier.id); - } - markReactive(place: Place): void { place.reactive = true; - this.markReactiveIdentifier(place.identifier); - } - - markReactiveIdentifier(inputIdentifier: Identifier): void { const identifier = - this.aliasedIdentifiers.find(inputIdentifier) ?? inputIdentifier; + this.aliasedIdentifiers.find(place.identifier) ?? place.identifier; if (!this.reactive.has(identifier.id)) { this.hasChanges = true; this.reactive.add(identifier.id);