From 48ecaf92d51813a1b1f1bcba1cdcbf48a501f2c6 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 25 Mar 2024 15:31:23 -0700 Subject: [PATCH] Reset scopes from AnalyzeFunctions in InferReactiveScopeVariables Updates InferReactiveScopeVariables to first prune scopes attached during AnalyzeFunctions. This ensures that after this pass the only scopes that exist on identifiers in the outer program are those that the pass explicitly inferred, and not accidentally leftover. --- .../InferReactiveScopeVariables.ts | 58 ++++++++----------- ...ange-shared-inner-outer-function.expect.md | 44 +++++++------- 2 files changed, 47 insertions(+), 55 deletions(-) diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts index b7f87cdd36..aeaccab381 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -9,7 +9,6 @@ import { Environment } from "../HIR"; import { HIRFunction, Identifier, - IdentifierId, Instruction, makeInstructionId, Place, @@ -17,8 +16,10 @@ import { } from "../HIR/HIR"; import { doesPatternContainSpreadElement, + eachInstructionLValue, eachInstructionOperand, eachPatternOperand, + eachTerminalOperand, } from "../HIR/visitors"; import DisjointSet from "../Utils/DisjointSet"; import { assertExhaustive } from "../Utils/utils"; @@ -80,6 +81,27 @@ import { assertExhaustive } from "../Utils/utils"; * ``` */ export function inferReactiveScopeVariables(fn: HIRFunction): void { + // First reset any scopes that may have been created from inner functions + for (const [, block] of fn.body.blocks) { + for (const phi of block.phis) { + phi.id.scope = null; + for (const [, operand] of phi.operands) { + operand.scope = null; + } + } + for (const instr of block.instructions) { + for (const lvalue of eachInstructionLValue(instr)) { + lvalue.identifier.scope = null; + } + for (const operand of eachInstructionOperand(instr)) { + operand.identifier.scope = null; + } + } + for (const operand of eachTerminalOperand(block.terminal)) { + operand.identifier.scope = null; + } + } + /* * Represents the set of reactive scopes as disjoint sets of identifiers * that mutate together. @@ -189,10 +211,6 @@ export function findDisjointMutableValues( fn: HIRFunction ): DisjointSet { const scopeIdentifiers = new DisjointSet(); - const declarations: Map | null = fn.env.config - .enableForest - ? new Map() - : null; for (const [_, block] of fn.body.blocks) { /* * If a phi is mutated after creation, then we need to alias all of its operands such that they @@ -221,14 +239,7 @@ export function findDisjointMutableValues( if (range.end > range.start + 1 || mayAllocate(fn.env, instr)) { operands.push(instr.lvalue!.identifier); } - if (instr.value.kind === "DeclareLocal") { - if (declarations !== null) { - declarations.set( - instr.value.lvalue.place.identifier.id, - instr.value.lvalue.place - ); - } - } else if ( + if ( instr.value.kind === "StoreLocal" || instr.value.kind === "StoreContext" ) { @@ -244,27 +255,6 @@ export function findDisjointMutableValues( ) { operands.push(instr.value.value.identifier); } - if (declarations !== null) { - const declaration = declarations.get( - instr.value.lvalue.place.identifier.id - ); - if (declaration !== undefined) { - declaration.identifier.mutableRange.end = makeInstructionId( - Math.max( - declaration.identifier.mutableRange.end, - instr.value.lvalue.place.identifier.mutableRange.end - ) - ); - instr.value.lvalue.place.identifier.mutableRange.start = - makeInstructionId( - Math.min( - declaration.identifier.mutableRange.start, - instr.value.lvalue.place.identifier.mutableRange.start - ) - ); - operands.push(declaration.identifier); - } - } } else if (instr.value.kind === "Destructure") { for (const place of eachPatternOperand(instr.value.lvalue.pattern)) { if ( diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md index 3c9eb66f5f..cb5f23541a 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/repro-mutable-range-shared-inner-outer-function.expect.md @@ -34,34 +34,36 @@ export const FIXTURE_ENTRYPOINT = { import { unstable_useMemoCache as useMemoCache } from "react"; // @enableAssumeHooksFollowRulesOfReact @enableTransitivelyFreezeFunctionExpressions let cond = true; function Component(props) { - const $ = useMemoCache(1); + const $ = useMemoCache(2); + let a; + let b; let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = Symbol.for("react.early_return_sentinel"); - bb8: { - let a; - let b; - const f = () => { - if (cond) { - a = {}; - b = []; - } else { - a = {}; - b = []; - } - a.property = true; - b.push(false); - }; - t0 =
; - break bb8; - } + t0 = () => { + if (cond) { + a = {}; + b = []; + } else { + a = {}; + b = []; + } + + a.property = true; + b.push(false); + }; $[0] = t0; } else { t0 = $[0]; } - if (t0 !== Symbol.for("react.early_return_sentinel")) { - return t0; + const f = t0; + let t1; + if ($[1] === Symbol.for("react.memo_cache_sentinel")) { + t1 =
; + $[1] = t1; + } else { + t1 = $[1]; } + return t1; } export const FIXTURE_ENTRYPOINT = {