diff --git a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts index f57e96e4e6..40764cf383 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Entrypoint/Pipeline.ts @@ -36,6 +36,7 @@ import { import { CodegenFunction, alignReactiveScopesToBlockScopes, + assertScopeInstructionsWithinScopes, buildReactiveBlocks, buildReactiveFunction, codegenReactiveFunction, @@ -261,6 +262,8 @@ function* runWithEnvironment( value: reactiveFunction, }); + assertScopeInstructionsWithinScopes(reactiveFunction); + flattenScopesWithHooks(reactiveFunction); yield log({ kind: "reactive", diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts index e3b44798f2..61e0b542a7 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AlignReactiveScopesToBlockScopes.ts @@ -82,7 +82,7 @@ class Visitor extends ReactiveFunctionVisitor { override visitBlock(block: ReactiveBlock, state: Context): void { state.enter(() => { this.traverseBlock(block, state); - }, "block"); + }); } } @@ -91,10 +91,7 @@ type PendingReactiveScope = { active: boolean; scope: ReactiveScope }; class Context { // For each block scope (outer array) stores a list of ReactiveScopes that start // in that block scope. - #blockScopes: Array<{ - kind: "block" | "value"; - scopes: Array; - }> = []; + #blockScopes: Array> = []; // ReactiveScopes whose declaring block scope has ended but may still need to // be "closed" (ie have their range.end be updated). A given scope can be in @@ -105,11 +102,11 @@ class Context { // the above data structures they're in, to avoid tracking the same scope twice. #seenScopes: Set = new Set(); - enter(fn: () => void, kind: "block" | "value" = "block"): void { - this.#blockScopes.push({ kind, scopes: [] }); + enter(fn: () => void): void { + this.#blockScopes.push([]); fn(); const lastScope = this.#blockScopes.pop()!; - for (const scope of lastScope.scopes) { + for (const scope of lastScope) { if (scope.active) { this.#unclosedScopes.push(scope); } @@ -118,10 +115,7 @@ class Context { visitId(id: InstructionId): void { const currentScopes = this.#blockScopes.at(-1)!; - if (currentScopes.kind === "value") { - return; - } - const scopes = [...currentScopes.scopes, ...this.#unclosedScopes]; + const scopes = [...currentScopes, ...this.#unclosedScopes]; for (const pending of scopes) { if (!pending.active) { continue; @@ -137,7 +131,7 @@ class Context { if (!this.#seenScopes.has(scope.id)) { const currentScopes = this.#blockScopes.at(-1)!; this.#seenScopes.add(scope.id); - currentScopes.scopes.push({ + currentScopes.push({ active: true, scope, }); diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AssertScopeInstructionsWithinScope.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AssertScopeInstructionsWithinScope.ts new file mode 100644 index 0000000000..245f8e1292 --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/AssertScopeInstructionsWithinScope.ts @@ -0,0 +1,76 @@ +/** + * 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 { visitReactiveFunction } from "."; +import { CompilerError } from ".."; +import { + InstructionId, + Place, + ReactiveFunction, + ReactiveScopeBlock, + ScopeId, +} from "../HIR"; +import { getPlaceScope } from "./BuildReactiveBlocks"; +import { ReactiveFunctionVisitor } from "./visitors"; + +/** + * Internal validation pass that checks all the instructions involved in creating + * values for a given scope are within the corresponding ReactiveScopeBlock. Errors + * in HIR/ReactiveFunction structure and alias analysis could theoretically create + * a structure such as: + * + * Function + * LabelTerminal + * Instruction in scope 0 + * Instruction in scope 0 + * + * Because ReactiveScopeBlocks are closed when their surrounding block ends, this + * structure would create reactive scopes as follows: + * + * Function + * LabelTerminal + * ReactiveScopeBlock scope=0 + * Instruction in scope 0 + * Instruction in scope 0 + * + * This pass asserts we didn't accidentally end up with such a structure, as a guard + * against compiler coding mistakes in earlier passes. + */ +export function assertScopeInstructionsWithinScopes( + fn: ReactiveFunction +): void { + visitReactiveFunction(fn, new Visitor(), undefined); +} + +class Visitor extends ReactiveFunctionVisitor { + seenScopes: Set = new Set(); + activeScopes: Set = new Set(); + + override visitPlace(id: InstructionId, place: Place, _state: void): void { + const scope = getPlaceScope(id, place); + if ( + scope !== null && + this.seenScopes.has(scope.id) && + !this.activeScopes.has(scope.id) + ) { + CompilerError.invariant(false, { + description: `Instruction [${id}] is part of scope @${scope.id}, but that scope has already completed.`, + loc: place.loc, + reason: + "Encountered an instruction that should be part of a scope, but where that scope has already completed", + suggestions: null, + }); + } + } + + override visitScope(block: ReactiveScopeBlock, state: void): void { + this.seenScopes.add(block.scope.id); + this.activeScopes.add(block.scope.id); + this.traverseScope(block, state); + this.activeScopes.delete(block.scope.id); + } +} diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/index.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/index.ts index fa231999aa..ba84e0256c 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/index.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/index.ts @@ -6,6 +6,7 @@ */ export { alignReactiveScopesToBlockScopes } from "./AlignReactiveScopesToBlockScopes"; +export { assertScopeInstructionsWithinScopes } from "./AssertScopeInstructionsWithinScope"; export { buildReactiveBlocks } from "./BuildReactiveBlocks"; export { buildReactiveFunction } from "./BuildReactiveFunction"; export {