diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts index 0f56cce988..a161c9a12b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/HIR.ts @@ -528,6 +528,13 @@ export type Instruction = { loc: SourceLocation; }; +export type TInstruction = { + id: InstructionId; + lvalue: Place; + value: T; + loc: SourceLocation; +}; + export type LValue = { place: Place; kind: InstructionKind; @@ -625,6 +632,17 @@ export type Phi = { type: Type; }; +export type StartMemoize = { + kind: "StartMemoize"; + deps: Array; + loc: SourceLocation; +}; +export type FinishMemoize = { + kind: "FinishMemoize"; + decl: Place; + loc: SourceLocation; +}; + /* * Forget currently does not handle MethodCall correctly in * all cases. Specifically, we do not bind the receiver and method property @@ -657,6 +675,12 @@ export type CallExpression = { typeArguments?: Array; }; +export type LoadLocal = { + kind: "LoadLocal"; + place: Place; + loc: SourceLocation; +}; + /* * The value of a given instruction. Note that values are not recursive: complex * values such as objects or arrays are always defined by instructions to define @@ -667,11 +691,7 @@ export type CallExpression = { */ export type InstructionValue = - | { - kind: "LoadLocal"; - place: Place; - loc: SourceLocation; - } + | LoadLocal | { kind: "LoadContext"; place: Place; @@ -773,12 +793,7 @@ export type InstructionValue = loc: SourceLocation; } // load `object.property` - | { - kind: "PropertyLoad"; - object: Place; - property: string; - loc: SourceLocation; - } + | PropertyLoad // `delete object.property` | { kind: "PropertyDelete"; @@ -873,7 +888,8 @@ export type InstructionValue = * during codegen. It can't be pruned during DCE because we need to preserve the * instruction so it can be visible in InferReferenceEffects. */ - | { kind: "Memoize"; value: Place; loc: SourceLocation } + | StartMemoize + | FinishMemoize /* * Catch-all for statements such as type imports, nested class declarations, etc * which are not directly represented, but included for completeness and to allow @@ -929,6 +945,13 @@ export type Primitive = { export type JSXText = { kind: "JSXText"; value: string; loc: SourceLocation }; +export type PropertyLoad = { + kind: "PropertyLoad"; + object: Place; + property: string; + loc: SourceLocation; +}; + export type LoadGlobal = { kind: "LoadGlobal"; name: string; diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts index f248e08eb8..ad8c96ac06 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/PrintHIR.ts @@ -600,8 +600,14 @@ export function printInstructionValue(instrValue: ReactiveValue): string { } ${printPlace(instrValue.value)}`; break; } - case "Memoize": { - value = `Memoize ${printPlace(instrValue.value)}`; + case "StartMemoize": { + value = `StartMemoize deps=${instrValue.deps.map((dep) => + printPlace(dep) + )}`; + break; + } + case "FinishMemoize": { + value = `FinishMemoize decl=${printPlace(instrValue.decl)}`; break; } case "ReactiveFunctionValue": { diff --git a/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts b/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts index ec814e1ab6..5af5248416 100644 --- a/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts +++ b/compiler/packages/babel-plugin-react-forget/src/HIR/visitors.ts @@ -216,8 +216,14 @@ export function* eachInstructionValueOperand( yield instrValue.value; break; } - case "Memoize": { - yield instrValue.value; + case "StartMemoize": { + for (const dep of instrValue.deps) { + yield dep; + } + break; + } + case "FinishMemoize": { + yield instrValue.decl; break; } case "Debugger": @@ -521,8 +527,14 @@ export function mapInstructionValueOperands( instrValue.value = fn(instrValue.value); break; } - case "Memoize": { - instrValue.value = fn(instrValue.value); + case "StartMemoize": { + for (let i = 0; i < instrValue.deps.length; i++) { + instrValue.deps[i] = fn(instrValue.deps[i]); + } + break; + } + case "FinishMemoize": { + instrValue.decl = fn(instrValue.decl); break; } case "Debugger": diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts index 80994aca39..0244a06687 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/DropManualMemoization.ts @@ -5,22 +5,210 @@ * LICENSE file in the root directory of this source tree. */ -import { CompilerError } from ".."; +import { CompilerError, SourceLocation } from ".."; import { + CallExpression, Effect, + Environment, + FinishMemoize, FunctionExpression, HIRFunction, IdentifierId, Instruction, + InstructionId, + LoadGlobal, + LoadLocal, + MethodCall, Place, + PropertyLoad, SpreadPattern, + StartMemoize, + TInstruction, getHookKindForType, makeInstructionId, } from "../HIR"; import { createTemporaryPlace, markInstructionIds } from "../HIR/HIRBuilder"; -import { HookKind } from "../HIR/ObjectShape"; import { eachInstructionValueOperand } from "../HIR/visitors"; +type ManualMemoCallee = { + kind: "useMemo" | "useCallback"; + loadInstr: TInstruction | TInstruction; +}; + +type IdentifierSidemap = { + functions: Map>; + manualMemos: Map; + react: Set; +}; + +function collectTemporaries( + instr: Instruction, + env: Environment, + sidemap: IdentifierSidemap +): void { + const { value } = instr; + switch (value.kind) { + case "FunctionExpression": { + sidemap.functions.set( + instr.lvalue.identifier.id, + instr as TInstruction + ); + break; + } + case "LoadGlobal": { + const global = env.getGlobalDeclaration(value.name); + const hookKind = global !== null ? getHookKindForType(env, global) : null; + const lvalId = instr.lvalue.identifier.id; + if (hookKind === "useMemo" || hookKind === "useCallback") { + sidemap.manualMemos.set(lvalId, { + kind: hookKind, + loadInstr: instr as TInstruction, + }); + } else if (value.name === "React") { + sidemap.react.add(lvalId); + } + break; + } + case "PropertyLoad": { + if (sidemap.react.has(value.object.identifier.id)) { + if (value.property === "useMemo" || value.property === "useCallback") { + sidemap.manualMemos.set(instr.lvalue.identifier.id, { + kind: value.property, + loadInstr: instr as TInstruction, + }); + } + } + break; + } + } +} + +function makeManualMemoizationMarkers( + fnExpr: Place, + env: Environment, + depsList: Array, + memoDecl: Place +): [TInstruction, TInstruction] { + return [ + { + id: makeInstructionId(0), + lvalue: createTemporaryPlace(env), + value: { + kind: "StartMemoize", + /* + * Use deps list from source instead of inferred deps + * as dependencies + */ + deps: depsList, + loc: fnExpr.loc, + }, + loc: fnExpr.loc, + }, + { + id: makeInstructionId(0), + lvalue: createTemporaryPlace(env), + value: { + kind: "FinishMemoize", + decl: { ...memoDecl }, + loc: fnExpr.loc, + }, + loc: fnExpr.loc, + }, + ]; +} + +function getManualMemoizationReplacement( + fn: Place, + loc: SourceLocation, + kind: "useMemo" | "useCallback" +): LoadLocal | CallExpression { + if (kind === "useMemo") { + /* + * Replace the hook callee with the fn arg. + * + * before: + * $1 = LoadGlobal useMemo // load the useMemo global + * $2 = FunctionExpression ... // memo function + * $3 = ArrayExpression [ ... ] // deps array + * $4 = Call $1 ($2, $3 ) // invoke useMemo w fn and deps + * + * after: + * $1 = LoadGlobal useMemo // load the useMemo global (dead code) + * $2 = FunctionExpression ... // memo function + * $3 = ArrayExpression [ ... ] // deps array (dead code) + * $4 = Call $2 () // invoke the memo function itself + * + * Note that a later pass (InlineImmediatelyInvokedFunctionExpressions) will + * inline the useMemo callback along with any other immediately invoked IIFEs. + */ + return { + kind: "CallExpression", + callee: fn, + /* + * Drop the args, including the deps array which DCE will remove + * later. + */ + args: [], + loc, + }; + } else { + /* + * Instead of a Call, just alias the callback directly. + * + * before: + * $1 = LoadGlobal useCallback + * $2 = FunctionExpression ... // the callback being memoized + * $3 = ArrayExpression ... // deps array + * $4 = Call $1 ( $2, $3 ) // invoke useCallback + * + * after: + * $1 = LoadGlobal useCallback // dead code + * $2 = FunctionExpression ... // the callback being memoized + * $3 = ArrayExpression ... // deps array (dead code) + * $4 = LoadLocal $2 // reference the function + */ + return { + kind: "LoadLocal", + place: { + kind: "Identifier", + identifier: fn.identifier, + effect: Effect.Unknown, + reactive: false, + loc, + }, + loc, + }; + } +} + +function extractManualMemoizationArgs( + instr: TInstruction | TInstruction, + kind: "useCallback" | "useMemo" +): { + fnPlace: Place; +} { + const [fnPlace] = instr.value.args as Array< + Place | SpreadPattern | undefined + >; + if (fnPlace == null) { + CompilerError.throwInvalidReact({ + reason: `Expected ${kind} call to pass a callback function`, + loc: instr.value.loc, + suggestions: null, + }); + } + if (fnPlace?.kind !== "Identifier") { + CompilerError.throwInvalidReact({ + reason: `Unexpected arguments to ${kind} call`, + loc: instr.value.loc, + suggestions: null, + }); + } + return { + fnPlace, + }; +} + /* * Removes manual memoization using the `useMemo` and `useCallback` APIs. This pass is designed * to compose with InlineImmediatelyInvokedFunctionExpressions, and needs to run prior to entering @@ -30,274 +218,135 @@ import { eachInstructionValueOperand } from "../HIR/visitors"; * eg `React.useMemo()`. */ export function dropManualMemoization(func: HIRFunction): void { - const functions = new Map(); - const hooks = new Map(); - const react = new Set(); - let hasChanges = false; + const isValidationEnabled = + func.env.config.validatePreserveExistingMemoizationGuarantees || + func.env.config.enablePreserveExistingMemoizationGuarantees; + const sidemap: IdentifierSidemap = { + functions: new Map(), + manualMemos: new Map(), + react: new Set(), + }; + + /** + * Phase 1: + * - Overwrite manual memoization from + * CallExpression callee="useMemo/Callback", args=[fnArg, depslist]) + * to either + * CallExpression callee=fnArg + * LoadLocal fnArg + * - (if validation is enabled) collect manual memoization markers + */ + const queuedInserts: Map< + InstructionId, + { + kind: "before" | "after"; + value: TInstruction | TInstruction; + } + > = new Map(); for (const [_, block] of func.body.blocks) { - let nextInstructions: Array | null = null; for (let i = 0; i < block.instructions.length; i++) { const instr = block.instructions[i]!; - switch (instr.value.kind) { - case "FunctionExpression": { - functions.set(instr.lvalue.identifier.id, instr.value); - break; - } - case "LoadGlobal": { - const global = func.env.getGlobalDeclaration(instr.value.name); - const hookKind = - global !== null ? getHookKindForType(func.env, global) : null; - if (hookKind === "useMemo" || hookKind === "useCallback") { - hooks.set(instr.lvalue.identifier.id, hookKind); - } else if (instr.value.name === "React") { - react.add(instr.lvalue.identifier.id); - } - break; - } - case "PropertyLoad": { - if (react.has(instr.value.object.identifier.id)) { - if ( - instr.value.property === "useMemo" || - instr.value.property === "useCallback" - ) { - hooks.set(instr.lvalue.identifier.id, instr.value.property); + if ( + instr.value.kind === "CallExpression" || + instr.value.kind === "MethodCall" + ) { + const id = + instr.value.kind === "CallExpression" + ? instr.value.callee.identifier.id + : instr.value.property.identifier.id; + + const manualMemo = sidemap.manualMemos.get(id); + if (manualMemo != null) { + const { fnPlace } = extractManualMemoizationArgs( + instr as TInstruction | TInstruction, + manualMemo.kind + ); + instr.value = getManualMemoizationReplacement( + fnPlace, + instr.value.loc, + manualMemo.kind + ); + if (isValidationEnabled) { + const inlineMemoFn = sidemap.functions.get(fnPlace.identifier.id); + if (inlineMemoFn == null) { + CompilerError.throwInvalidReact({ + reason: + "DepsValidation: Expected function literal as manual memoization callback", + suggestions: [], + loc: fnPlace.loc, + }); } - } - break; - } - case "MethodCall": - case "CallExpression": { - const id = - instr.value.kind === "CallExpression" - ? instr.value.callee.identifier.id - : instr.value.property.identifier.id; - const hookKind = hooks.get(id); - if (hookKind != null) { - if (hookKind === "useMemo") { - const [fn] = instr.value.args as Array< - Place | SpreadPattern | undefined - >; - if (fn == null) { - CompilerError.throwInvalidReact({ - reason: "Expected useMemo call to pass a callback function", - loc: instr.loc, - suggestions: null, - }); - } - /* - * Replace the hook callee with the fn arg. - * - * before: - * $1 = LoadGlobal useMemo // load the useMemo global - * $2 = FunctionExpression ... // memo function - * $3 = ArrayExpression [ ... ] // deps array - * $4 = Call $1 ($2, $3 ) // invoke useMemo w fn and deps - * - * after: - * $1 = LoadGlobal useMemo // load the useMemo global (dead code) - * $2 = FunctionExpression ... // memo function - * $3 = ArrayExpression [ ... ] // deps array (dead code) - * $4 = Call $2 () // invoke the memo function itself - * - * Note that a later pass (InlineImmediatelyInvokedFunctionExpressions) will - * inline the useMemo callback along with any other immediately invoked IIFEs. - */ - if (fn.kind === "Identifier") { - instr.value = { - kind: "CallExpression", - callee: fn, - /* - * Drop the args, including the deps array which DCE will remove - * later. - */ - args: [], - loc: instr.value.loc, - }; - - if ( - func.env.config.enablePreserveExistingMemoizationGuarantees || - func.env.config.validatePreserveExistingMemoizationGuarantees - ) { - /** - * When this flag is enabled we also compile in a 'Memoize' instruction - * to preserve the intended memoization boundary: - * - * Normal output: - * $1 = LoadGlobal useMemo // load the useMemo global (dead code) - * $2 = FunctionExpression ... // memo function - * $3 = ArrayExpression [ ... ] // deps array (dead code) - * $4 = Call $2 () // invoke the memo function itself - * - * Output w flag enabled: - * $1 = LoadGlobal useMemo // load the useMemo global (dead code) - * $2 = FunctionExpression ... // memo function - * $3 = ArrayExpression [ ... ] // deps array (dead code) - * .. = Memoize ... // memoize dependencies - * $4 = Call $2 () // invoke the memo function itself - * .. = Memoize $4 // preserve memo information - * - * Note that Memoize does not produce a result and is called for its side - * effects only. - */ - nextInstructions = - nextInstructions ?? block.instructions.slice(0, i); - - const functionExpression = functions.get(fn.identifier.id); - if (functionExpression !== undefined) { - for (const operand of eachInstructionValueOperand( - functionExpression - )) { - const temp = createTemporaryPlace(func.env); - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: temp, - value: { - kind: "Memoize", - value: { ...operand }, - loc: instr.loc, - }, - loc: instr.loc, - }); - } - } - - nextInstructions.push(instr); - - const temp = createTemporaryPlace(func.env); - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: temp, - value: { - kind: "Memoize", - value: { ...instr.lvalue }, - loc: instr.loc, - }, - loc: instr.loc, - }); - continue; - } - } - } else if (hookKind === "useCallback") { - const [fn] = instr.value.args as Array< - Place | SpreadPattern | undefined - >; - if (fn == null) { - CompilerError.throwInvalidReact({ - reason: "Expected useMemo call to pass a callback function", - loc: instr.loc, - suggestions: null, - }); - } - - /* - * Instead of a Call, just alias the callback directly. - * - * before: - * $1 = LoadGlobal useCallback - * $2 = FunctionExpression ... // the callback being memoized - * $3 = ArrayExpression ... // deps array - * $4 = Call $1 ( $2, $3 ) // invoke useCallback - * - * after: - * $1 = LoadGlobal useCallback // dead code - * $2 = FunctionExpression ... // the callback being memoized - * $3 = ArrayExpression ... // deps array (dead code) - * $4 = LoadLocal $2 // reference the function - */ - if (fn.kind === "Identifier") { - instr.value = { - kind: "LoadLocal", - place: { + const memoDecl: Place = + manualMemo.kind === "useMemo" + ? instr.lvalue + : { kind: "Identifier", - identifier: fn.identifier, + identifier: fnPlace.identifier, effect: Effect.Unknown, reactive: false, - loc: instr.value.loc, - }, - loc: instr.value.loc, - }; - if ( - func.env.config.enablePreserveExistingMemoizationGuarantees || - func.env.config.validatePreserveExistingMemoizationGuarantees - ) { - nextInstructions = - nextInstructions ?? block.instructions.slice(0, i); - /** - * With the flag enabled the output changes to use a Memoize instruction instead - * a loadlocal to load the function expression into the original temporary: - * - * Normal output: - * $1 = LoadGlobal useCallback // dead code - * $2 = FunctionExpression ... // the callback being memoized - * $3 = ArrayExpression ... // deps array (dead code) - * $4 = LoadLocal $2 // reference the function - * - * With flag enabled: - * $1 = LoadGlobal useCallback // dead code - * $2 = FunctionExpression ... // the callback being memoized - * $3 = ArrayExpression ... // deps array (dead code) - * .. = Memoize ... // memoize dependencies - * $n = Memoize $2 // reference the function - * $4 = LoadLocal $2 // reference the function - * - * Note that Memoize does not produce a result and is called for its side effects - * only. - */ - const functionExpression = functions.get(fn.identifier.id); - if (functionExpression !== undefined) { - for (const operand of eachInstructionValueOperand( - functionExpression - )) { - const temp = createTemporaryPlace(func.env); - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: temp, - value: { - kind: "Memoize", - value: { ...operand }, - loc: instr.loc, - }, - loc: instr.loc, - }); - } - } - nextInstructions.push(instr); + loc: fnPlace.loc, + }; - const temp = createTemporaryPlace(func.env); - nextInstructions.push({ - id: makeInstructionId(0), - lvalue: { ...temp }, - value: { - kind: "Memoize", - value: { - kind: "Identifier", - identifier: fn.identifier, - effect: Effect.Unknown, - reactive: false, - loc: instr.value.loc, - }, - loc: instr.value.loc, - }, - loc: instr.loc, - }); - continue; - } - } - } + const [startMarker, finishMarker] = makeManualMemoizationMarkers( + fnPlace, + func.env, + // Next PR will replace this with depslist from source + [...eachInstructionValueOperand(inlineMemoFn.value)], + memoDecl + ); + + /* + * This PR reorders startMarker to right before the inlineMemoFn + * since startMarker references inlineMemoFn.deps. + * Next PR will move startMarker earlier, to after the `useMemo`/ + * `useCallback` load itself (as it also changes startMarker to + * not reference lowered deps anymore). + */ + queuedInserts.set(inlineMemoFn.id, { + kind: "before", + value: startMarker, + }); + queuedInserts.set(instr.id, { kind: "after", value: finishMarker }); + continue; } - break; + } + } else { + collectTemporaries(instr, func.env, sidemap); + } + } + } + + /** + * Phase 2: Insert manual memoization markers as needed + */ + if (queuedInserts.size > 0) { + let hasChanges = false; + for (const [_, block] of func.body.blocks) { + let nextInstructions: Array | null = null; + for (let i = 0; i < block.instructions.length; i++) { + const instr = block.instructions[i]; + const insertInstr = queuedInserts.get(instr.id); + if (insertInstr != null) { + nextInstructions = nextInstructions ?? block.instructions.slice(0, i); + if (insertInstr.kind === "before") { + nextInstructions.push(insertInstr.value); + nextInstructions.push(instr); + } else { + nextInstructions.push(instr); + nextInstructions.push(insertInstr.value); + } + } else if (nextInstructions != null) { + nextInstructions.push(instr); } } if (nextInstructions !== null) { - nextInstructions.push(instr); + block.instructions = nextInstructions; + hasChanges = true; } } - if (nextInstructions !== null) { - block.instructions = nextInstructions; - hasChanges = true; + + if (hasChanges) { + markInstructionIds(func.body); } } - if (hasChanges) { - markInstructionIds(func.body); - } } diff --git a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts index cdd0c9e915..b91c75ca31 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Inference/InferReferenceEffects.ts @@ -1391,21 +1391,24 @@ function inferBlock( state.alias(lvalue, instrValue.value); continue; } - case "Memoize": { - if (env.config.enablePreserveExistingMemoizationGuarantees) { - state.reference( - instrValue.value, - functionEffects, - Effect.Freeze, - ValueReason.Other - ); - } else { - state.reference( - instrValue.value, - functionEffects, - Effect.Read, - ValueReason.Other - ); + case "StartMemoize": + case "FinishMemoize": { + for (const val of eachInstructionValueOperand(instrValue)) { + if (env.config.enablePreserveExistingMemoizationGuarantees) { + state.reference( + val, + functionEffects, + Effect.Freeze, + ValueReason.Other + ); + } else { + state.reference( + val, + functionEffects, + Effect.Read, + ValueReason.Other + ); + } } const lvalue = instr.lvalue; lvalue.effect = Effect.ConditionallyMutate; diff --git a/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts b/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts index 2e450f4f3a..2ce93475a8 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Optimization/DeadCodeElimination.ts @@ -338,7 +338,8 @@ function pruneableValue(value: InstructionValue, state: State): boolean { case "StoreContext": { return false; } - case "Memoize": { + case "StartMemoize": + case "FinishMemoize": { /** * This instruction is used by the @enablePreserveExistingMemoizationGuarantees feature * to preserve information about memoization semantics in the original code. We can't diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 278ff2d237..e16c23a940 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -939,7 +939,10 @@ function codegenInstructionNullable( assertExhaustive(kind, `Unexpected instruction kind '${kind}'`); } } - } else if (instr.value.kind === "Memoize") { + } else if ( + instr.value.kind === "StartMemoize" || + instr.value.kind === "FinishMemoize" + ) { return null; } else if (instr.value.kind === "Debugger") { return t.debuggerStatement(); @@ -1787,7 +1790,8 @@ function codegenInstructionValue( break; } case "ReactiveFunctionValue": - case "Memoize": + case "StartMemoize": + case "FinishMemoize": case "Debugger": case "DeclareLocal": case "DeclareContext": 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 0ff2638b31..b7f87cdd36 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -154,7 +154,8 @@ function mayAllocate(env: Environment, instruction: Instruction): boolean { case "NextIterableOf": case "NextPropertyOf": case "Debugger": - case "Memoize": + case "StartMemoize": + case "FinishMemoize": case "UnaryExpression": case "BinaryExpression": case "PropertyLoad": { diff --git a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts index 982c8e889d..3e3c54422c 100644 --- a/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -453,7 +453,8 @@ function computeMemoizationInputs( }; } case "NextPropertyOf": - case "Memoize": + case "StartMemoize": + case "FinishMemoize": case "Debugger": case "ComputedDelete": case "PropertyDelete": @@ -926,8 +927,8 @@ class PruneScopesTransform extends ReactiveFunctionTransform< * need to be memoized. Remove associated `Memoize` instructions so that * we don't report false positives on "missing" memoization of these values. */ - if (instruction.value.kind === "Memoize") { - const identifier = instruction.value.value.identifier; + if (instruction.value.kind === "FinishMemoize") { + const identifier = instruction.value.decl.identifier; if ( identifier.scope !== null && this.prunedScopes.has(identifier.scope.id) diff --git a/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts b/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts index 3576fd6895..e68ce57357 100644 --- a/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts +++ b/compiler/packages/babel-plugin-react-forget/src/TypeInference/InferTypes.ts @@ -335,7 +335,8 @@ function* generateInstructionTypes( case "NextIterableOf": case "UnsupportedNode": case "Debugger": - case "Memoize": { + case "FinishMemoize": + case "StartMemoize": { break; } default: diff --git a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts index 28cd30f935..702e2ff223 100644 --- a/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts +++ b/compiler/packages/babel-plugin-react-forget/src/Validation/ValidatePreservedManualMemoization.ts @@ -14,6 +14,7 @@ import { ReactiveScopeBlock, ScopeId, } from "../HIR"; +import { eachInstructionValueOperand } from "../HIR/visitors"; import { isMutable } from "../ReactiveScopes/InferReactiveScopeVariables"; import { ReactiveFunctionVisitor, @@ -71,20 +72,24 @@ class Visitor extends ReactiveFunctionVisitor { state: CompilerError ): void { this.traverseInstruction(instruction, state); - if (instruction.value.kind === "Memoize") { - const value = instruction.value.value; - if ( - isMutable(instruction as Instruction, value) || - isUnmemoized(value.identifier, this.scopes) - ) { - state.push({ - reason: - "This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized", - description: null, - severity: ErrorSeverity.InvalidReact, - loc: typeof instruction.loc !== "symbol" ? instruction.loc : null, - suggestions: null, - }); + if ( + instruction.value.kind === "StartMemoize" || + instruction.value.kind === "FinishMemoize" + ) { + for (const value of eachInstructionValueOperand(instruction.value)) { + if ( + isMutable(instruction as Instruction, value) || + isUnmemoized(value.identifier, this.scopes) + ) { + state.push({ + reason: + "This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized", + description: null, + severity: ErrorSeverity.InvalidReact, + loc: typeof instruction.loc !== "symbol" ? instruction.loc : null, + suggestions: null, + }); + } } } } diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md index 75841e4b40..c0fd9b5c4b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-accesses-ref-mutated-later-via-function-preserve-memoization.expect.md @@ -37,7 +37,7 @@ export const FIXTURE_ENTRYPOINT = { 5 | const ref = useRef({ inner: null }); 6 | > 7 | const onChange = useCallback((event) => { - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ > 8 | // The ref should still be mutable here even though function deps are frozen in | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 9 | // @enablePreserveExistingMemoizationGuarantees mode @@ -45,7 +45,7 @@ export const FIXTURE_ENTRYPOINT = { > 10 | ref.current.inner = event.target.value; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 11 | }); - | ^^^^^ [ReactForget] InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) + | ^^^^ [ReactForget] InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) [ReactForget] InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) 12 | diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md index f8d60aafa0..6c57503e82 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.todo-useCallback-set-ref-nested-property-ref-modified-later-preserve-memoization.expect.md @@ -34,7 +34,7 @@ export const FIXTURE_ENTRYPOINT = { 5 | const ref = useRef({ inner: null }); 6 | > 7 | const onChange = useCallback((event) => { - | ^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^ > 8 | // The ref should still be mutable here even though function deps are frozen in | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 9 | // @enablePreserveExistingMemoizationGuarantees mode @@ -42,7 +42,7 @@ export const FIXTURE_ENTRYPOINT = { > 10 | ref.current.inner = event.target.value; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 11 | }); - | ^^^^^ [ReactForget] InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) + | ^^^^ [ReactForget] InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) [ReactForget] InvalidReact: This value was manually memoized, but cannot be memoized under Forget because it may be mutated after it is memoized (7:11) 12 | diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md index 223c3fe87e..13c3ea751b 100644 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.expect.md @@ -2,22 +2,32 @@ ## Input ```javascript -function Component(props) { - const x = useMemo(someHelper, []); +import { useMemo } from "react"; +import { makeArray } from "shared-runtime"; + +function Component() { + const x = useMemo(makeArray, []); return x; } +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + ``` ## Code ```javascript -import { unstable_useMemoCache as useMemoCache } from "react"; -function Component(props) { +import { useMemo, unstable_useMemoCache as useMemoCache } from "react"; +import { makeArray } from "shared-runtime"; + +function Component() { const $ = useMemoCache(1); let t0; if ($[0] === Symbol.for("react.memo_cache_sentinel")) { - t0 = someHelper(); + t0 = makeArray(); $[0] = t0; } else { t0 = $[0]; @@ -26,5 +36,10 @@ function Component(props) { return x; } +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +}; + ``` \ No newline at end of file diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.js b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.js deleted file mode 100644 index 0a9e80fec2..0000000000 --- a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.js +++ /dev/null @@ -1,4 +0,0 @@ -function Component(props) { - const x = useMemo(someHelper, []); - return x; -} diff --git a/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.ts b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.ts new file mode 100644 index 0000000000..f194a281ec --- /dev/null +++ b/compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/useMemo-named-function.ts @@ -0,0 +1,12 @@ +import { useMemo } from "react"; +import { makeArray } from "shared-runtime"; + +function Component() { + const x = useMemo(makeArray, []); + return x; +} + +export const FIXTURE_ENTRYPOINT = { + fn: Component, + params: [{}], +};