[compiler][donotcommit] show differences between identifier + scope ranges

ghstack-source-id: f9abcf8c2f
Pull Request resolved: https://github.com/facebook/react/pull/30409
This commit is contained in:
Mofei Zhang
2024-07-19 13:19:35 -04:00
parent 7789b60b1e
commit fa3de495f4
8 changed files with 100 additions and 7 deletions
@@ -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);
@@ -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<MutableRange> = 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
@@ -1270,6 +1270,10 @@ export type AbstractValue = {
context: ReadonlySet<Place>;
};
export function isRangeMutable(range: MutableRange): boolean {
return range.end > range.start + 1;
}
/**
* The reason for the kind of a value.
*/
@@ -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;
}
}
}
}
@@ -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 {
@@ -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;
}
}
}
}
@@ -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
}
@@ -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(