diff --git a/compiler/forget/src/HIR/visitors.ts b/compiler/forget/src/HIR/visitors.ts index 02bb3fd52a..58397223fe 100644 --- a/compiler/forget/src/HIR/visitors.ts +++ b/compiler/forget/src/HIR/visitors.ts @@ -185,6 +185,34 @@ export function* eachInstructionValueOperand( } } +export function doesPatternContainSpreadElement(pattern: Pattern): boolean { + switch (pattern.kind) { + case "ArrayPattern": { + for (const item of pattern.items) { + if (item.kind === "Spread") { + return true; + } + } + break; + } + case "ObjectPattern": { + for (const property of pattern.properties) { + if (property.kind === "Spread") { + return true; + } + } + break; + } + default: { + assertExhaustive( + pattern, + `Unexpected pattern kind '${(pattern as any).kind}'` + ); + } + } + return false; +} + export function* eachPatternOperand(pattern: Pattern): Iterable { switch (pattern.kind) { case "ArrayPattern": { diff --git a/compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts b/compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts index 581c88aedd..b00169e4c1 100644 --- a/compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts +++ b/compiler/forget/src/ReactiveScopes/InferReactiveScopeVariables.ts @@ -15,7 +15,11 @@ import { Place, ReactiveScope, } from "../HIR/HIR"; -import { eachInstructionOperand, eachPatternOperand } from "../HIR/visitors"; +import { + doesPatternContainSpreadElement, + eachInstructionOperand, + eachPatternOperand, +} from "../HIR/visitors"; import DisjointSet from "../Utils/DisjointSet"; import { assertExhaustive } from "../Utils/utils"; @@ -198,7 +202,9 @@ function isMutable({ id }: Instruction, place: Place): boolean { function mayAllocate(value: InstructionValue): boolean { switch (value.kind) { - case "Destructure": + case "Destructure": { + return doesPatternContainSpreadElement(value.lvalue.pattern); + } case "StoreLocal": case "LoadGlobal": case "TypeCastExpression": diff --git a/compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts b/compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts index 5c222aa25f..7f8a2de5ac 100644 --- a/compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts +++ b/compiler/forget/src/ReactiveScopes/PruneNonEscapingScopes.ts @@ -12,6 +12,7 @@ import { Effect, IdentifierId, InstructionId, + Pattern, Place, ReactiveFunction, ReactiveInstruction, @@ -22,7 +23,6 @@ import { ReactiveValue, ScopeId, } from "../HIR"; -import { eachPatternOperand } from "../HIR/visitors"; import { log } from "../Utils/logger"; import { assertExhaustive } from "../Utils/utils"; import { getPlaceScope } from "./BuildReactiveBlocks"; @@ -319,43 +319,48 @@ function computeMemoizationInputs( lvalue: Place | null ): { // can optionally return a custom set of lvalues per instruction - lvalues: Array; + lvalues: Array; rvalues: Array; - level: MemoizationLevel; } { switch (value.kind) { case "ConditionalExpression": { return { - lvalues: lvalue !== null ? [lvalue] : [], + // Only need to memoize if the rvalues are memoized + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Conditional }] + : [], rvalues: [ // Conditionals do not alias their test value. ...computeMemoizationInputs(value.consequent, null).rvalues, ...computeMemoizationInputs(value.alternate, null).rvalues, ], - // Only need to memoize if the rvalues are memoized - level: MemoizationLevel.Conditional, }; } case "LogicalExpression": { return { - lvalues: lvalue !== null ? [lvalue] : [], + // Only need to memoize if the rvalues are memoized + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Conditional }] + : [], rvalues: [ ...computeMemoizationInputs(value.left, null).rvalues, ...computeMemoizationInputs(value.right, null).rvalues, ], - // Only need to memoize if the rvalues are memoized - level: MemoizationLevel.Conditional, }; } case "SequenceExpression": { return { - lvalues: lvalue !== null ? [lvalue] : [], + // Only need to memoize if the rvalues are memoized + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Conditional }] + : [], // Only the final value of the sequence is a true rvalue: // values from the sequence's instructions are evaluated // as separate nodes rvalues: computeMemoizationInputs(value.value, null).rvalues, - // Only memoize if the final value was memoized - level: MemoizationLevel.Conditional, }; } case "JsxExpression": { @@ -374,20 +379,24 @@ function computeMemoizationInputs( } } return { - lvalues: lvalue !== null ? [lvalue] : [], - rvalues: operands, // JSX elements themselves are not memoized unless forced to // avoid breaking downstream memoization - level: MemoizationLevel.Unmemoized, + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Unmemoized }] + : [], + rvalues: operands, }; } case "JsxFragment": { return { - lvalues: lvalue !== null ? [lvalue] : [], - rvalues: value.children, // JSX elements themselves are not memoized unless forced to // avoid breaking downstream memoization - level: MemoizationLevel.Unmemoized, + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Unmemoized }] + : [], + rvalues: value.children, }; } case "ComputedDelete": @@ -399,68 +408,84 @@ function computeMemoizationInputs( case "BinaryExpression": case "UnaryExpression": { return { - lvalues: lvalue !== null ? [lvalue] : [], - rvalues: [], // All of these instructions return a primitive value and never need to be memoized - level: MemoizationLevel.Never, + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Never }] + : [], + rvalues: [], }; } case "TypeCastExpression": { return { - lvalues: lvalue !== null ? [lvalue] : [], // Indirection for the inner value, memoized if the value is + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Conditional }] + : [], rvalues: [value.value], - level: MemoizationLevel.Conditional, }; } case "LoadLocal": { return { - lvalues: lvalue !== null ? [lvalue] : [], // Indirection for the inner value, memoized if the value is + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Conditional }] + : [], rvalues: [value.place], - level: MemoizationLevel.Conditional, }; } case "StoreLocal": { + const lvalues = [ + { place: value.lvalue.place, level: MemoizationLevel.Conditional }, + ]; + if (lvalue !== null) { + lvalues.push({ place: lvalue, level: MemoizationLevel.Conditional }); + } return { - lvalues: - lvalue !== null ? [lvalue, value.lvalue.place] : [value.lvalue.place], // Indirection for the inner value, memoized if the value is + lvalues, rvalues: [value.value], - level: MemoizationLevel.Conditional, }; } case "Destructure": { + // Indirection for the inner value, memoized if the value is const lvalues = []; if (lvalue !== null) { - lvalues.push(lvalue); + lvalues.push({ place: lvalue, level: MemoizationLevel.Conditional }); } - lvalues.push(...eachPatternOperand(value.lvalue.pattern)); + lvalues.push(...computePatternLValues(value.lvalue.pattern)); return { lvalues: lvalues, - // Indirection for the inner value, memoized if the value is rvalues: [value.value], - level: MemoizationLevel.Conditional, }; } case "ComputedLoad": case "PropertyLoad": { return { - lvalues: lvalue !== null ? [lvalue] : [], // Indirection for the inner value, memoized if the value is + lvalues: + lvalue !== null + ? [{ place: lvalue, level: MemoizationLevel.Conditional }] + : [], // Only the object is aliased to the result, and the result only needs to be // memoized if the object is rvalues: [value.object], - level: MemoizationLevel.Conditional, }; } case "ComputedStore": { // The object being stored to acts as an lvalue (it aliases the value), but // the computed key is not aliased + const lvalues = [ + { place: value.object, level: MemoizationLevel.Conditional }, + ]; + if (lvalue !== null) { + lvalues.push({ place: lvalue, level: MemoizationLevel.Conditional }); + } return { - lvalues: lvalue !== null ? [lvalue, value.object] : [value.object], + lvalues, rvalues: [value.value], - level: MemoizationLevel.Conditional, }; } case "FunctionExpression": @@ -475,16 +500,15 @@ function computeMemoizationInputs( // All of these instructions may produce new values which must be memoized if // reachable from a return value. Any mutable rvalue may alias any other rvalue const operands = [...eachReactiveValueOperand(value)]; - const lvalues = operands.filter((operand) => - isMutableEffect(operand.effect) - ); + const lvalues = operands + .filter((operand) => isMutableEffect(operand.effect)) + .map((place) => ({ place, level: MemoizationLevel.Memoized })); if (lvalue !== null) { - lvalues.push(lvalue); + lvalues.push({ place: lvalue, level: MemoizationLevel.Memoized }); } return { lvalues, rvalues: operands, - level: MemoizationLevel.Memoized, }; } case "UnsupportedNode": { @@ -496,6 +520,45 @@ function computeMemoizationInputs( } } +function computePatternLValues(pattern: Pattern): Array { + const lvalues: Array = []; + switch (pattern.kind) { + case "ArrayPattern": { + for (const item of pattern.items) { + if (item.kind === "Identifier") { + lvalues.push({ place: item, level: MemoizationLevel.Conditional }); + } else { + lvalues.push({ place: item.place, level: MemoizationLevel.Memoized }); + } + } + break; + } + case "ObjectPattern": { + for (const property of pattern.properties) { + if (property.kind === "ObjectProperty") { + lvalues.push({ + place: property.place, + level: MemoizationLevel.Conditional, + }); + } else { + lvalues.push({ + place: property.place, + level: MemoizationLevel.Memoized, + }); + } + } + break; + } + default: { + assertExhaustive( + pattern, + `Unexpected pattern kind '${(pattern as any).kind}'` + ); + } + } + return lvalues; +} + /** * Populates the input state with the set of returned identifiers and information about each * identifier's and scope's dependencies. @@ -521,7 +584,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { } // Add the operands as dependencies of all lvalues. - for (const lvalue of aliasing.lvalues) { + for (const { place: lvalue, level } of aliasing.lvalues) { const lvalueId = state.definitions.get(lvalue.identifier.id) ?? lvalue.identifier.id; let node = state.identifiers.get(lvalueId); @@ -535,7 +598,7 @@ class CollectDependenciesVisitor extends ReactiveFunctionVisitor { }; state.identifiers.set(lvalueId, node); } - node.level = joinAliases(node.level, aliasing.level); + node.level = joinAliases(node.level, level); // This looks like NxM iterations but in practice all instructions with multiple // lvalues have only a single rvalue for (const operand of aliasing.rvalues) { diff --git a/compiler/forget/src/__tests__/fixtures/hir/destructuring.expect.md b/compiler/forget/src/__tests__/fixtures/hir/destructuring.expect.md index f40bb9601b..4a12429c04 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/destructuring.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/destructuring.expect.md @@ -28,39 +28,75 @@ function foo(a, b, c) { ```javascript function foo(a, b, c) { - const $ = React.unstable_useMemoCache(8); - const [d, t40, ...h] = a; + const $ = React.unstable_useMemoCache(18); + const c_0 = $[0] !== a; + let t0; + let d; + let h; + if (c_0) { + [d, t0, ...h] = a; + $[0] = a; + $[1] = t0; + $[2] = d; + $[3] = h; + } else { + t0 = $[1]; + d = $[2]; + h = $[3]; + } - const [t43] = t40; - const { e: t45, ...g } = t43; - const { f } = t45; + const [t1] = t0; + const c_4 = $[4] !== t1; + let t2; + let g; + if (c_4) { + ({ e: t2, ...g } = t1); + $[4] = t1; + $[5] = t2; + $[6] = g; + } else { + t2 = $[5]; + g = $[6]; + } + const { f } = t2; const { l: t51, p } = b; - const { m: t54 } = t51; - const [t56, ...o] = t54; - const [n] = t56; - const c_0 = $[0] !== d; - const c_1 = $[1] !== f; - const c_2 = $[2] !== g; - const c_3 = $[3] !== h; - const c_4 = $[4] !== n; - const c_5 = $[5] !== o; - const c_6 = $[6] !== p; - let t0; - if (c_0 || c_1 || c_2 || c_3 || c_4 || c_5 || c_6) { - t0 = [d, f, g, h, n, o, p]; - $[0] = d; - $[1] = f; - $[2] = g; - $[3] = h; - $[4] = n; - $[5] = o; - $[6] = p; - $[7] = t0; + const { m: t3 } = t51; + const c_7 = $[7] !== t3; + let t4; + let o; + if (c_7) { + [t4, ...o] = t3; + $[7] = t3; + $[8] = t4; + $[9] = o; } else { - t0 = $[7]; + t4 = $[8]; + o = $[9]; } - return t0; + const [n] = t4; + const c_10 = $[10] !== d; + const c_11 = $[11] !== f; + const c_12 = $[12] !== g; + const c_13 = $[13] !== h; + const c_14 = $[14] !== n; + const c_15 = $[15] !== o; + const c_16 = $[16] !== p; + let t5; + if (c_10 || c_11 || c_12 || c_13 || c_14 || c_15 || c_16) { + t5 = [d, f, g, h, n, o, p]; + $[10] = d; + $[11] = f; + $[12] = g; + $[13] = h; + $[14] = n; + $[15] = o; + $[16] = p; + $[17] = t5; + } else { + t5 = $[17]; + } + return t5; } ``` diff --git a/compiler/forget/src/__tests__/fixtures/hir/escape-analysis-destructured-rest-element.expect.md b/compiler/forget/src/__tests__/fixtures/hir/escape-analysis-destructured-rest-element.expect.md index 310b7ada9b..d9fde0f28d 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/escape-analysis-destructured-rest-element.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/escape-analysis-destructured-rest-element.expect.md @@ -16,9 +16,25 @@ function Component(props) { ```javascript function Component(props) { - const { a, ...b } = props.a; - - const [c, ...d] = props.c; + const $ = React.unstable_useMemoCache(4); + const c_0 = $[0] !== props.a; + let b; + if (c_0) { + ({ a, ...b } = props.a); + $[0] = props.a; + $[1] = b; + } else { + b = $[1]; + } + const c_2 = $[2] !== props.c; + let d; + if (c_2) { + [c, ...d] = props.c; + $[2] = props.c; + $[3] = d; + } else { + d = $[3]; + } return
; } diff --git a/compiler/forget/src/__tests__/fixtures/hir/unused-object-element-with-rest.expect.md b/compiler/forget/src/__tests__/fixtures/hir/unused-object-element-with-rest.expect.md index d5e0a68550..8a7576905a 100644 --- a/compiler/forget/src/__tests__/fixtures/hir/unused-object-element-with-rest.expect.md +++ b/compiler/forget/src/__tests__/fixtures/hir/unused-object-element-with-rest.expect.md @@ -14,7 +14,16 @@ function Foo(props) { ```javascript function Foo(props) { - const { unused, ...rest } = props.a; + const $ = React.unstable_useMemoCache(2); + const c_0 = $[0] !== props.a; + let rest; + if (c_0) { + ({ unused, ...rest } = props.a); + $[0] = props.a; + $[1] = rest; + } else { + rest = $[1]; + } return rest; }