Skip to content

Commit

Permalink
[compiler] Kill markReactiveIdentifier and friends
Browse files Browse the repository at this point in the history
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: #31178
  • Loading branch information
mvitousek committed Oct 12, 2024
1 parent 6cf5bd9 commit 147374d
Showing 1 changed file with 4 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ export function inferReactivePlaces(fn: HIRFunction): void {
}

do {
const identifierMapping = new Map<Identifier, Identifier>();
for (const [, block] of fn.body.blocks) {
let hasReactiveControl = isReactiveControlledBlock(block.id);

Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 147374d

Please sign in to comment.