From b1c07ab3c41e590f34ab7ec8e71cbae379b32076 Mon Sep 17 00:00:00 2001 From: Mofei Zhang Date: Fri, 19 Jul 2024 13:19:33 -0400 Subject: [PATCH] [compiler][donotcommit] show differences between identifier + scope ranges [ghstack-poisoned] --- .../src/Entrypoint/Pipeline.ts | 3 + .../src/HIR/BuildReactiveScopeTerminalsHIR.ts | 64 +++++++++++++++++++ .../src/HIR/HIR.ts | 4 ++ .../HIR/MergeOverlappingReactiveScopesHIR.ts | 4 ++ .../src/HIR/PrintHIR.ts | 10 +-- .../ReactiveScopes/AlignMethodCallScopes.ts | 10 +++ .../ReactiveScopes/AlignObjectMethodScopes.ts | 4 ++ .../MemoizeFbtAndMacroOperandsInSameScope.ts | 8 +++ 8 files changed, 100 insertions(+), 7 deletions(-) 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 077d30f121a30..5360224b0af04 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/Entrypoint/Pipeline.ts @@ -98,6 +98,7 @@ import { } from "../Validation"; import { validateLocalsNotReassignedAfterRender } from "../Validation/ValidateLocalsNotReassignedAfterRender"; import { outlineFunctions } from "../Optimization/OutlineFunctions"; +import { assertMutableRangesAreAlwaysScopes } from "../HIR/BuildReactiveScopeTerminalsHIR"; export type CompilerPipelineValue = | { kind: "ast"; name: string; value: CodegenFunction } @@ -299,6 +300,8 @@ function* runWithEnvironment( value: hir, }); + assertMutableRangesAreAlwaysScopes(hir); + assertValidBlockNesting(hir); flattenReactiveLoopsHIR(hir); diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts index f12e78eb744b5..1f30dae11e70e 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/BuildReactiveScopeTerminalsHIR.ts @@ -9,10 +9,74 @@ import { GotoVariant, HIRFunction, InstructionId, + MutableRange, + Place, ReactiveScope, ReactiveScopeTerminal, ScopeId, } from "./HIR"; +import { printPlace } from "./PrintHIR"; +import { + eachInstructionLValue, + eachInstructionOperand, + eachTerminalOperand, +} from "./visitors"; + +export function assertMutableRangesAreAlwaysScopes(fn: HIRFunction): void { + const scopeRanges: Set = new Set(); + + for (const [_, block] of fn.body.blocks) { + if ( + block.terminal.kind === "scope" || + block.terminal.kind === "pruned-scope" + ) { + scopeRanges.add(block.terminal.scope.range); + } + } + + const validate = (place: Place): void => { + if ( + place.identifier.mutableRange.end > + place.identifier.mutableRange.start + 1 + ) { + if (scopeRanges.size !== 0) { + CompilerError.invariant( + scopeRanges.has(place.identifier.mutableRange), + { + reason: "Mutable range not found in scope ranges!", + description: `${printPlace(place)}`, + loc: place.loc, + } + ); + } + if (place.identifier.scope != null) { + // Useful for debugging before we build reactive scope terminals + CompilerError.invariant( + place.identifier.scope.range === place.identifier.mutableRange, + { + reason: "Mutable range inconsistent with range of attached scope", + description: `${printPlace(place)}`, + loc: place.loc, + } + ); + } + } + }; + + for (const [_, block] of fn.body.blocks) { + for (const instr of block.instructions) { + for (const place of eachInstructionLValue(instr)) { + validate(place); + } + for (const place of eachInstructionOperand(instr)) { + validate(place); + } + } + for (const place of eachTerminalOperand(block.terminal)) { + validate(place); + } + } +} /** * This pass assumes that all program blocks are properly nested with respect to fallthroughs 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 b79414de0310f..6ff1b0a9cc38f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/HIR.ts @@ -1270,6 +1270,10 @@ export type AbstractValue = { context: ReadonlySet; }; +export function isRangeMutable(range: MutableRange): boolean { + return range.end > range.start + 1; +} + /** * The reason for the kind of a value. */ diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeOverlappingReactiveScopesHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeOverlappingReactiveScopesHIR.ts index 6ce0c101fbea1..0447f920c784c 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeOverlappingReactiveScopesHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/MergeOverlappingReactiveScopesHIR.ts @@ -3,6 +3,7 @@ import { InstructionId, Place, ReactiveScope, + isRangeMutable, makeInstructionId, } from "."; import { getPlaceScope } from "../ReactiveScopes/BuildReactiveBlocks"; @@ -124,6 +125,9 @@ export function mergeOverlappingReactiveScopesHIR(fn: HIRFunction): void { const nextScope = joinedScopes.find(originalScope); if (nextScope !== null && nextScope !== originalScope) { place.identifier.scope = nextScope; + if (isRangeMutable(place.identifier.mutableRange)) { + place.identifier.mutableRange = nextScope.range; + } } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts index 837b3729dce13..686b2d23566e8 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/PrintHIR.ts @@ -35,7 +35,7 @@ import type { Terminal, Type, } from "./HIR"; -import { GotoVariant, InstructionKind } from "./HIR"; +import { GotoVariant, InstructionKind, isRangeMutable } from "./HIR"; export type Options = { indent: number; @@ -716,10 +716,6 @@ export function printInstructionValue(instrValue: ReactiveValue): string { return value; } -function isMutable(range: MutableRange): boolean { - return range.end > range.start + 1; -} - const DEBUG_MUTABLE_RANGES = false; function printMutableRange(identifier: Identifier): string { if (DEBUG_MUTABLE_RANGES) { @@ -732,11 +728,11 @@ function printMutableRange(identifier: Identifier): string { ) { return `[${range.start}:${range.end}] scope=[${scopeRange.start}:${scopeRange.end}]`; } - return isMutable(range) ? `[${range.start}:${range.end}]` : ""; + return isRangeMutable(range) ? `[${range.start}:${range.end}]` : ""; } // in non-debug mode, prefer the scope range if it exists const range = identifier.scope?.range ?? identifier.mutableRange; - return isMutable(range) ? `[${range.start}:${range.end}]` : ""; + return isRangeMutable(range) ? `[${range.start}:${range.end}]` : ""; } export function printLValue(lval: LValue): string { diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignMethodCallScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignMethodCallScopes.ts index fd56f6cab9dd5..a190149d236cb 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignMethodCallScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignMethodCallScopes.ts @@ -9,6 +9,7 @@ import { HIRFunction, IdentifierId, ReactiveScope, + isRangeMutable, makeInstructionId, } from "../HIR"; import DisjointSet from "../Utils/DisjointSet"; @@ -69,10 +70,19 @@ export function alignMethodCallScopes(fn: HIRFunction): void { const mappedScope = scopeMapping.get(instr.lvalue.identifier.id); if (mappedScope !== undefined) { instr.lvalue.identifier.scope = mappedScope; + if ( + mappedScope != null && + isRangeMutable(instr.lvalue.identifier.mutableRange) + ) { + instr.lvalue.identifier.mutableRange = mappedScope.range; + } } else if (instr.lvalue.identifier.scope !== null) { const mergedScope = mergedScopes.find(instr.lvalue.identifier.scope); if (mergedScope != null) { instr.lvalue.identifier.scope = mergedScope; + if (isRangeMutable(instr.lvalue.identifier.mutableRange)) { + instr.lvalue.identifier.mutableRange = mergedScope.range; + } } } } diff --git a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts index b6f8f833c8532..a46675e5296b7 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/AlignObjectMethodScopes.ts @@ -11,6 +11,7 @@ import { HIRFunction, Identifier, ReactiveScope, + isRangeMutable, makeInstructionId, } from "../HIR"; import { eachInstructionValueOperand } from "../HIR/visitors"; @@ -93,6 +94,9 @@ export function alignObjectMethodScopes(fn: HIRFunction): void { const root = scopeGroupsMap.get(identifier.scope); if (root != null) { identifier.scope = root; + if (isRangeMutable(identifier.mutableRange)) { + identifier.mutableRange = root.range; + } } // otherwise, this identifier's scope was not affected by this pass } 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 037765f1e9cab..281069c840c34 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/ReactiveScopes/MemoizeFbtAndMacroOperandsInSameScope.ts @@ -7,10 +7,12 @@ import { HIRFunction, + Identifier, IdentifierId, makeInstructionId, Place, ReactiveValue, + isRangeMutable, } from "../HIR"; import { eachReactiveValueOperand } from "./visitors"; @@ -134,6 +136,9 @@ function visit( */ for (const operand of eachReactiveValueOperand(value)) { operand.identifier.scope = fbtScope; + if (isRangeMutable(operand.identifier.mutableRange)) { + operand.identifier.mutableRange = fbtScope.range; + } // Expand the jsx element's range to account for its operands fbtScope.range.start = makeInstructionId( @@ -167,6 +172,9 @@ function visit( continue; } operand.identifier.scope = fbtScope; + if (isRangeMutable(operand.identifier.mutableRange)) { + operand.identifier.mutableRange = fbtScope.range; + } // Expand the jsx element's range to account for its operands fbtScope.range.start = makeInstructionId(