From c69cba357f5b24ec110ecac40b30de48fa83ae3a Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Tue, 3 Jan 2023 15:34:39 -0800 Subject: [PATCH] Use distinct type for scope dependencies This is a pre-req to deleting the `Place.memberPath` field. We no longer need memberPath in the HIR now that we have PropertyStore and PropertyLoad. However, scope dependencies use memberPath to track the precise fields that a computation depends upon. This PR changes scopes dependencies to use a new `ReactiveScopeDependency` type (Place + optional path), which allows the next PR to remove `Place.memberPath`. --- compiler/forget/src/HIR/HIR.ts | 7 ++- .../ReactiveScopes/CodegenReactiveFunction.ts | 16 +++++- .../ReactiveScopes/PrintReactiveFunction.ts | 12 ++++- .../PropagateScopeDependencies.ts | 54 ++++++++++--------- 4 files changed, 62 insertions(+), 27 deletions(-) diff --git a/compiler/forget/src/HIR/HIR.ts b/compiler/forget/src/HIR/HIR.ts index 6a90e04637..3187297bc2 100644 --- a/compiler/forget/src/HIR/HIR.ts +++ b/compiler/forget/src/HIR/HIR.ts @@ -388,10 +388,15 @@ export enum Effect { export type ReactiveScope = { id: ScopeId; range: MutableRange; - dependencies: Set; + dependencies: Set; outputs: Set; }; +export type ReactiveScopeDependency = { + place: Place; + path: Array | null; +}; + /** * Simulated opaque type for BlockIds to prevent using normal numbers as block ids * accidentally. diff --git a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts index 9434866701..10f1bea45f 100644 --- a/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/CodegenReactiveFunction.ts @@ -23,6 +23,7 @@ import { ReactiveBlock, ReactiveFunction, ReactiveScope, + ReactiveScopeDependency, ReactiveTerminal, ReactiveValueBlock, } from "../HIR/HIR"; @@ -137,7 +138,7 @@ function codegenReactiveScope( for (const dep of scope.dependencies) { const index = cx.nextCacheIndex; const changeIdentifier = t.identifier(`c_${index}`); - const depValue = codegenPlace(cx.temp, dep); + const depValue = codegenDependency(cx, dep); changeIdentifiers.push(changeIdentifier); statements.push( @@ -380,3 +381,16 @@ function codegenValueBlock( return t.sequenceExpression(expressions); } } + +function codegenDependency( + cx: Context, + dependency: ReactiveScopeDependency +): t.Expression { + let object: t.Expression = convertIdentifier(dependency.place.identifier); + if (dependency.path !== null) { + for (const path of dependency.path) { + object = t.memberExpression(object, t.identifier(path)); + } + } + return object; +} diff --git a/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts b/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts index 79dc510a22..7149e9e9b8 100644 --- a/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts +++ b/compiler/forget/src/ReactiveScopes/PrintReactiveFunction.ts @@ -9,6 +9,7 @@ import invariant from "invariant"; import { ReactiveFunction, ReactiveScopeBlock, + ReactiveScopeDependency, ReactiveStatement, ReactiveTerminal, ReactiveValueBlock, @@ -43,7 +44,7 @@ export function printReactiveBlock( `scope @${block.scope.id} [${block.scope.range.start}:${ block.scope.range.end }] deps=[${Array.from(block.scope.dependencies) - .map((dep) => printPlace(dep)) + .map((dep) => printDependency(dep)) .join(", ")}] out=[${Array.from(block.scope.outputs) .map((out) => printIdentifier(out)) .join(", ")}] {` @@ -52,6 +53,15 @@ export function printReactiveBlock( writer.writeLine("}"); } +function printDependency(dependency: ReactiveScopeDependency): string { + const place = printPlace(dependency.place); + if (dependency.path === null) { + return place; + } else { + return `${place}${dependency.path.map((prop) => `.${prop}`).join("")}`; + } +} + export function printReactiveInstructions( writer: Writer, instructions: Array diff --git a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts index c4b2959e82..3817d54a5c 100644 --- a/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts +++ b/compiler/forget/src/ReactiveScopes/PropagateScopeDependencies.ts @@ -17,6 +17,7 @@ import { ReactiveBlock, ReactiveFunction, ReactiveScope, + ReactiveScopeDependency, ReactiveValueBlock, } from "../HIR/HIR"; import { eachInstructionValueOperand } from "../HIR/visitors"; @@ -55,13 +56,13 @@ type Scopes = Array; class Context { #declarations: DeclMap = new Map(); - #dependencies: Set = new Set(); - #properties: Map = new Map(); + #dependencies: Set = new Set(); + #properties: Map = new Map(); #scopes: Scopes = []; - enter(scope: ReactiveScope, fn: () => void): Set { + enter(scope: ReactiveScope, fn: () => void): Set { const previousDependencies = this.#dependencies; - const scopedDependencies = new Set(); + const scopedDependencies = new Set(); this.#dependencies = scopedDependencies; this.#scopes.push(scope); fn(); @@ -83,17 +84,17 @@ class Context { object.memberPath === null, "Expected operands to have null memberPath" ); - const objectPlace = this.#properties.get(object.identifier); - let place: Place; - if (objectPlace === undefined) { - place = { ...object, memberPath: [property] }; + const objectDependency = this.#properties.get(object.identifier); + let nextDependency: ReactiveScopeDependency; + if (objectDependency === undefined) { + nextDependency = { place: object, path: [property] }; } else { - place = { - ...objectPlace, - memberPath: [...(objectPlace.memberPath ?? []), property], + nextDependency = { + place: objectDependency.place, + path: [...(objectDependency.path ?? []), property], }; } - this.#properties.set(lvalue.identifier, place); + this.#properties.set(lvalue.identifier, nextDependency); } #isScopeActive(scope: ReactiveScope): boolean { @@ -104,27 +105,32 @@ class Context { return this.#scopes.at(-1)!; } - visitOperand(operand: Place): void { - let maybeDependency: Place; - if (operand.memberPath !== null) { + visitOperand(place: Place): void { + this.visitDependency({ place, path: null }); + } + + visitDependency(dependency: ReactiveScopeDependency): void { + let maybeDependency: ReactiveScopeDependency; + if (dependency.path !== null) { // Operands may have memberPaths when propagating depenencies of an inner scope upward // In this case we use the dependency as-is - maybeDependency = operand; + maybeDependency = dependency; } else { // Otherwise if this operand is a temporary created for a property load, resolve it to // the expanded Place. Fall back to using the operand as-is. - maybeDependency = this.#properties.get(operand.identifier) ?? operand; + maybeDependency = + this.#properties.get(dependency.place.identifier) ?? dependency; } - const decl = this.#declarations.get(maybeDependency.identifier); + const decl = this.#declarations.get(maybeDependency.place.identifier); // Any value used after its defining scope has concluded must be added as an // output of its defining scope. Regardless of whether its a const or not, // some later code needs access to the value. if (decl !== undefined) { - const operandScope = maybeDependency.identifier.scope; + const operandScope = maybeDependency.place.identifier.scope; if (operandScope !== null && !this.#isScopeActive(operandScope)) { - operandScope.outputs.add(maybeDependency.identifier); + operandScope.outputs.add(maybeDependency.place.identifier); } } @@ -140,15 +146,15 @@ class Context { // Check if there is an existing dependency that describes this operand for (const dep of this.#dependencies) { // not the same identifier - if (dep.identifier !== maybeDependency.identifier) { + if (dep.place.identifier !== maybeDependency.place.identifier) { continue; } - const depPath = dep.memberPath; + const depPath = dep.path; // existing dep covers all paths if (depPath === null) { return; } - const operandPath = maybeDependency.memberPath; + const operandPath = maybeDependency.path; // existing dep is for a path, this operand covers all paths so swap them if (operandPath === null) { this.#dependencies.delete(dep); @@ -187,7 +193,7 @@ function visit(context: Context, block: ReactiveBlock): void { // normal dependency collection. child scopes may have dependencies // on values created within the outer scope, which necessarily cannot // be dependencies of the outer scope - context.visitOperand(dep); + context.visitDependency(dep); } break; }