diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts index 6a08bf6d4f..5f5e5b9db4 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -827,10 +827,15 @@ export type ReactiveScope = { id: ScopeId; range: MutableRange; dependencies: Set; - declarations: Map; + declarations: Map; reassignments: Set; }; +export type ReactiveScopeDeclaration = { + identifier: Identifier; + scope: ReactiveScope; // the scope in which the variable was originally declared +}; + export type ReactiveScopeDependency = { identifier: Identifier; path: Array; diff --git a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts index d78ec70946..4d6f152c91 100644 --- a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -191,20 +191,20 @@ function codegenReactiveScope( ); } let firstOutputIndex: number | null = null; - for (const [, declaration] of scope.declarations) { + for (const [, { identifier }] of scope.declarations) { const index = cx.nextCacheIndex; if (firstOutputIndex === null) { firstOutputIndex = index; } invariant( - declaration.name != null, + identifier.name != null, "Expected identifier '@%s' to be named", - declaration.id + identifier.id ); - const name = convertIdentifier(declaration); - if (!cx.hasDeclared(declaration)) { + const name = convertIdentifier(identifier); + if (!cx.hasDeclared(identifier)) { statements.push( t.variableDeclaration("let", [t.variableDeclarator(name)]) ); @@ -227,7 +227,7 @@ function codegenReactiveScope( ) ) ); - cx.declare(declaration); + cx.declare(identifier); } for (const reassignment of scope.reassignments) { const index = cx.nextCacheIndex; diff --git a/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts index ec4785f5c8..ee3c490f69 100644 --- a/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts @@ -47,7 +47,9 @@ export function printReactiveBlock( }] dependencies=[${Array.from(block.scope.dependencies) .map((dep) => printDependency(dep)) .join(", ")}] declarations=[${Array.from(block.scope.declarations) - .map(([, decl]) => printIdentifier(decl)) + .map(([, decl]) => + printIdentifier({ ...decl.identifier, scope: decl.scope }) + ) .join(", ")}] reassignments=[${Array.from(block.scope.reassignments).map( (reassign) => printIdentifier(reassign) )}] {` diff --git a/compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts b/compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts index e5578622f8..fd32580943 100644 --- a/compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts +++ b/compiler/forget/src/ReactiveScopes/PromoteUsedTemporaries.ts @@ -29,9 +29,9 @@ class Visitor extends ReactiveFunctionVisitor { // value. // Many of our current test fixtures do not return a value, so // it is better for now to promote (and memoize) every output. - for (const [, identifier] of block.scope.declarations) { - if (identifier.name == null) { - identifier.name = `t${state.nextId++}`; + for (const [, declaration] of block.scope.declarations) { + if (declaration.identifier.name == null) { + declaration.identifier.name = `t${state.nextId++}`; } } } diff --git a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts index 21448a06b6..60d15876f3 100644 --- a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -317,13 +317,16 @@ class Context { const originalDeclaration = this.#declarations.get( maybeDependency.identifier.id ); - if (originalDeclaration !== undefined) { + if ( + originalDeclaration !== undefined && + originalDeclaration.scope.value !== null + ) { originalDeclaration.scope.each((scope) => { if (!this.#isScopeActive(scope)) { - scope.declarations.set( - maybeDependency.identifier.id, - maybeDependency.identifier - ); + scope.declarations.set(maybeDependency.identifier.id, { + identifier: maybeDependency.identifier, + scope: originalDeclaration.scope.value!, // checked above + }); } }); } diff --git a/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts b/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts index df482b3cc8..91b7db4cf6 100644 --- a/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts +++ b/compiler/forget/src/ReactiveScopes/PruneNonReactiveDependencies.ts @@ -34,12 +34,12 @@ class Visitor extends ReactiveFunctionVisitor { if (scope.scope.dependencies.size === 0) { // If a scope has no dependencies, then its declarations are all non-reactive for (const [, declaration] of scope.scope.declarations) { - state.delete(declaration.id); + state.delete(declaration.identifier.id); } } else { // otherwise, all the scope's declarations are reactive for (const [, declaration] of scope.scope.declarations) { - state.add(declaration.id); + state.add(declaration.identifier.id); } } } diff --git a/compiler/forget/src/ReactiveScopes/PruneUnusedScopes.ts b/compiler/forget/src/ReactiveScopes/PruneUnusedScopes.ts index 6e2787c11f..a4fa2e7fd1 100644 --- a/compiler/forget/src/ReactiveScopes/PruneUnusedScopes.ts +++ b/compiler/forget/src/ReactiveScopes/PruneUnusedScopes.ts @@ -30,8 +30,11 @@ class Transform extends ReactiveFunctionTransform { ): Transformed { this.visitScope(scopeBlock, state); if ( - scopeBlock.scope.declarations.size === 0 && - scopeBlock.scope.reassignments.size === 0 + scopeBlock.scope.reassignments.size === 0 && + (scopeBlock.scope.declarations.size === 0 || + // Can prune scopes where all declarations bubbled up from inner + // scopes + !hasOwnDeclaration(scopeBlock)) ) { return { kind: "replace-many", value: scopeBlock.instructions }; } else { @@ -39,3 +42,16 @@ class Transform extends ReactiveFunctionTransform { } } } + +/** + * Does the scope block declare any values of its own? This can return + * false if all the block's declarations are propagated from nested scopes. + */ +function hasOwnDeclaration(block: ReactiveScopeBlock): boolean { + for (const declaration of block.scope.declarations.values()) { + if (declaration.scope.id === block.scope.id) { + return true; + } + } + return false; +} diff --git a/compiler/forget/src/__tests__/fixtures/compiler/dependencies.expect.md b/compiler/forget/src/__tests__/fixtures/compiler/dependencies.expect.md index f9e0a011c2..6ad952df97 100644 --- a/compiler/forget/src/__tests__/fixtures/compiler/dependencies.expect.md +++ b/compiler/forget/src/__tests__/fixtures/compiler/dependencies.expect.md @@ -25,36 +25,25 @@ function foo(x, y, z) { ```javascript import * as React from "react"; function foo(x, y, z) { - const $ = React.unstable_useMemoCache(7); - const c_0 = $[0] !== z; - const c_1 = $[1] !== x; - const c_2 = $[2] !== y; + const $ = React.unstable_useMemoCache(3); + const items = [z]; + items.push(x); + const c_0 = $[0] !== x; + const c_1 = $[1] !== y; let items2; - if (c_0 || c_1 || c_2) { - const items = [z]; - items.push(x); - const c_4 = $[4] !== x; - const c_5 = $[5] !== y; - if (c_4 || c_5) { - items2 = []; - if (x) { - items2.push(y); - } - $[4] = x; - $[5] = y; - $[6] = items2; - } else { - items2 = $[6]; + if (c_0 || c_1) { + items2 = []; + if (x) { + items2.push(y); } - if (y) { - items.push(x); - } - $[0] = z; - $[1] = x; - $[2] = y; - $[3] = items2; + $[0] = x; + $[1] = y; + $[2] = items2; } else { - items2 = $[3]; + items2 = $[2]; + } + if (y) { + items.push(x); } return items2; }