From 473470a9a5476a04b12269dd9371001335b4abc7 Mon Sep 17 00:00:00 2001 From: Chris Blappert Date: Wed, 3 Oct 2018 11:17:49 -0700 Subject: [PATCH] Filter ModifiedBindings by environment's creating optimized function (#2551) Summary: Release Notes: None Changes the logic for determining which ModifiedBindings need serialization to work properly in the face of nested optimized functions. We should only serialize `ModifiedBinding`s if their environment was not created by the optimized function or its children (i.e. the binding should not be local to the optimized function). This also solves the issue that React components don't have a parent chain which is important for properly handling nested optimized functions. Solves #2550. Solves #2430, Solves #2426, Solves #2423, Solves #2422 (some were solved by previous PRs, just adding the tests here as well). Pull Request resolved: https://github.com/facebook/prepack/pull/2551 Differential Revision: D10010048 Pulled By: cblappert fbshipit-source-id: 2a855017514832a70d024f1ae9a91e9f07736ce0 --- src/environment.js | 2 + src/react/optimizing.js | 163 +++++++++++------- src/react/utils.js | 10 +- src/realm.js | 13 ++ src/serializer/functions.js | 25 ++- src/serializer/serializer.js | 10 +- src/serializer/types.js | 4 +- src/serializer/utils.js | 24 ++- src/utils/generator.js | 54 +++++- .../optimized-functions/Issue2422.js | 22 +++ .../optimized-functions/Issue2423.js | 22 +++ .../MissingModifiedBinding.js | 19 ++ .../WrongScopeTripleNested.js | 18 ++ 13 files changed, 288 insertions(+), 98 deletions(-) create mode 100644 test/serializer/optimized-functions/Issue2422.js create mode 100644 test/serializer/optimized-functions/Issue2423.js create mode 100644 test/serializer/optimized-functions/MissingModifiedBinding.js create mode 100644 test/serializer/optimized-functions/WrongScopeTripleNested.js diff --git a/src/environment.js b/src/environment.js index 920f80f02..369c876b9 100644 --- a/src/environment.js +++ b/src/environment.js @@ -88,6 +88,7 @@ export class EnvironmentRecord { isReadOnly: boolean; $NewTarget: void | ObjectValue; id: number; + creatingOptimizedFunction: FunctionValue | void; static nextId: number = 0; @@ -96,6 +97,7 @@ export class EnvironmentRecord { this.realm = realm; this.isReadOnly = false; this.id = EnvironmentRecord.nextId++; + this.creatingOptimizedFunction = realm.currentOptimizedFunction; } HasBinding(N: string): boolean { diff --git a/src/react/optimizing.js b/src/react/optimizing.js index 8ae17ec9e..82557a8e3 100644 --- a/src/react/optimizing.js +++ b/src/react/optimizing.js @@ -10,7 +10,13 @@ /* @flow strict-local */ import { type Effects, Realm } from "../realm.js"; -import { AbstractValue, ECMAScriptSourceFunctionValue, ObjectValue, BoundFunctionValue } from "../values/index.js"; +import { + AbstractValue, + ECMAScriptSourceFunctionValue, + ObjectValue, + BoundFunctionValue, + FunctionValue, +} from "../values/index.js"; import { createAdditionalEffects } from "../serializer/utils.js"; import { convertFunctionalComponentToComplexClassComponent, @@ -22,7 +28,12 @@ import { normalizeFunctionalComponentParamaters, valueIsClassComponent, } from "./utils.js"; -import { type WriteEffects, type ReactEvaluatedNode, ReactStatistics } from "../serializer/types.js"; +import { + type WriteEffects, + type ReactEvaluatedNode, + ReactStatistics, + type AdditionalFunctionTransform, +} from "../serializer/types.js"; import { Reconciler, type ComponentTreeState } from "./reconcilation.js"; import { ReconcilerFatalError } from "./errors.js"; import { Properties } from "../singletons.js"; @@ -31,6 +42,61 @@ import invariant from "../invariant.js"; import type { ReactComponentTreeConfig } from "../types.js"; import { Logger } from "../utils/logger.js"; +function writeEffectsKeyOfComponentValue( + realm: Realm, + componentType: ECMAScriptSourceFunctionValue | BoundFunctionValue, + componentTreeState: ComponentTreeState, + transforms: Array +): FunctionValue { + if (valueIsClassComponent(realm, componentType)) { + if (componentTreeState.status === "SIMPLE") { + // if the root component was a class and is now simple, we can convert it from a class + // component to a functional component + if (componentType instanceof BoundFunctionValue) { + let targetFunction = componentType.$BoundTargetFunction; + invariant(targetFunction instanceof ECMAScriptSourceFunctionValue); + convertSimpleClassComponentToFunctionalComponent(realm, targetFunction, transforms); + normalizeFunctionalComponentParamaters(targetFunction); + return targetFunction; + } else { + convertSimpleClassComponentToFunctionalComponent(realm, componentType, transforms); + normalizeFunctionalComponentParamaters(componentType); + return componentType; + } + } else { + let prototype = Get(realm, componentType, "prototype"); + invariant(prototype instanceof ObjectValue); + let renderMethod = Get(realm, prototype, "render"); + invariant(renderMethod instanceof ECMAScriptSourceFunctionValue); + return renderMethod; + } + } else { + if (componentTreeState.status === "COMPLEX") { + convertFunctionalComponentToComplexClassComponent( + realm, + componentType, + componentTreeState.componentType, + transforms + ); + let prototype = Get(realm, componentType, "prototype"); + invariant(prototype instanceof ObjectValue); + let renderMethod = Get(realm, prototype, "render"); + invariant(renderMethod instanceof ECMAScriptSourceFunctionValue); + return renderMethod; + } else { + if (componentType instanceof BoundFunctionValue) { + let targetFunction = componentType.$BoundTargetFunction; + invariant(targetFunction instanceof ECMAScriptSourceFunctionValue); + normalizeFunctionalComponentParamaters(targetFunction); + return targetFunction; + } else { + normalizeFunctionalComponentParamaters(componentType); + return componentType; + } + } + } +} + function applyWriteEffectsForOptimizedComponent( realm: Realm, componentType: ECMAScriptSourceFunctionValue | BoundFunctionValue, @@ -38,15 +104,24 @@ function applyWriteEffectsForOptimizedComponent( componentTreeState: ComponentTreeState, evaluatedNode: ReactEvaluatedNode, writeEffects: WriteEffects, - environmentRecordIdAfterGlobalCode: number + preEvaluationComponentToWriteEffectFunction: Map, + parentOptimizedFunction: FunctionValue | void ): void { let effects = _effects; + let transforms = []; + let writeEffectsKey = writeEffectsKeyOfComponentValue(realm, componentType, componentTreeState, transforms); + // NB: Must be done here because its required by cAE + preEvaluationComponentToWriteEffectFunction.set(componentType, writeEffectsKey); let additionalFunctionEffects = createAdditionalEffects( realm, effects, false, "ReactAdditionalFunctionEffects", - environmentRecordIdAfterGlobalCode + writeEffects, + preEvaluationComponentToWriteEffectFunction, + writeEffectsKey, + parentOptimizedFunction, + transforms ); if (additionalFunctionEffects === null) { throw new ReconcilerFatalError( @@ -62,53 +137,7 @@ function applyWriteEffectsForOptimizedComponent( // in the reconciler return; } - if (valueIsClassComponent(realm, componentType)) { - if (componentTreeState.status === "SIMPLE") { - // if the root component was a class and is now simple, we can convert it from a class - // component to a functional component - if (componentType instanceof BoundFunctionValue) { - let targetFunction = componentType.$BoundTargetFunction; - invariant(targetFunction instanceof ECMAScriptSourceFunctionValue); - convertSimpleClassComponentToFunctionalComponent(realm, targetFunction, additionalFunctionEffects); - normalizeFunctionalComponentParamaters(targetFunction); - writeEffects.set(targetFunction, additionalFunctionEffects); - } else { - convertSimpleClassComponentToFunctionalComponent(realm, componentType, additionalFunctionEffects); - normalizeFunctionalComponentParamaters(componentType); - writeEffects.set(componentType, additionalFunctionEffects); - } - } else { - let prototype = Get(realm, componentType, "prototype"); - invariant(prototype instanceof ObjectValue); - let renderMethod = Get(realm, prototype, "render"); - invariant(renderMethod instanceof ECMAScriptSourceFunctionValue); - writeEffects.set(renderMethod, additionalFunctionEffects); - } - } else { - if (componentTreeState.status === "COMPLEX") { - convertFunctionalComponentToComplexClassComponent( - realm, - componentType, - componentTreeState.componentType, - additionalFunctionEffects - ); - let prototype = Get(realm, componentType, "prototype"); - invariant(prototype instanceof ObjectValue); - let renderMethod = Get(realm, prototype, "render"); - invariant(renderMethod instanceof ECMAScriptSourceFunctionValue); - writeEffects.set(renderMethod, additionalFunctionEffects); - } else { - if (componentType instanceof BoundFunctionValue) { - let targetFunction = componentType.$BoundTargetFunction; - invariant(targetFunction instanceof ECMAScriptSourceFunctionValue); - normalizeFunctionalComponentParamaters(targetFunction); - writeEffects.set(targetFunction, additionalFunctionEffects); - } else { - normalizeFunctionalComponentParamaters(componentType); - writeEffects.set(componentType, additionalFunctionEffects); - } - } - } + writeEffects.set(writeEffectsKey, additionalFunctionEffects); // apply contextTypes for legacy context if (componentTreeState.contextTypes.size > 0) { let contextTypes = new ObjectValue(realm, realm.intrinsics.ObjectPrototype); @@ -124,9 +153,9 @@ function optimizeReactComponentTreeBranches( realm: Realm, reconciler: Reconciler, writeEffects: WriteEffects, - environmentRecordIdAfterGlobalCode: number, logger: Logger, - alreadyEvaluated: Map + alreadyEvaluated: Map, + preEvaluationComponentToWriteEffectFunction: Map ): void { if (realm.react.verbose && reconciler.branchedComponentTrees.length > 0) { logger.logInformation(` Evaluating React component tree branches...`); @@ -147,11 +176,17 @@ function optimizeReactComponentTreeBranches( if (realm.react.verbose) { logger.logInformation(` Evaluating ${evaluatedNode.name} (branch)`); } - let branchEffects = reconciler.resolveReactComponentTree(branchComponentType, null, null, evaluatedNode); + let parentOptimizedFunction = realm.currentOptimizedFunction; + let branchEffects = realm.withNewOptimizedFunction( + () => reconciler.resolveReactComponentTree(branchComponentType, null, null, evaluatedNode), + branchComponentType + ); + if (realm.react.verbose) { logger.logInformation(` ✔ ${evaluatedNode.name} (branch)`); } let branchComponentTreeState = reconciler.componentTreeState; + applyWriteEffectsForOptimizedComponent( realm, branchComponentType, @@ -159,7 +194,8 @@ function optimizeReactComponentTreeBranches( branchComponentTreeState, evaluatedNode, writeEffects, - environmentRecordIdAfterGlobalCode + preEvaluationComponentToWriteEffectFunction, + parentOptimizedFunction ); } } @@ -169,10 +205,10 @@ export function optimizeReactComponentTreeRoot( componentRoot: ECMAScriptSourceFunctionValue | BoundFunctionValue | AbstractValue, config: ReactComponentTreeConfig, writeEffects: WriteEffects, - environmentRecordIdAfterGlobalCode: number, logger: Logger, statistics: ReactStatistics, - alreadyEvaluated: Map + alreadyEvaluated: Map, + preEvaluationComponentToWriteEffectFunction: Map ): void { let reconciler = new Reconciler(realm, config, alreadyEvaluated, statistics, logger); let componentType = getComponentTypeFromRootValue(realm, componentRoot); @@ -188,7 +224,11 @@ export function optimizeReactComponentTreeRoot( if (realm.react.verbose) { logger.logInformation(` Evaluating ${evaluatedRootNode.name} (root)`); } - let componentTreeEffects = reconciler.resolveReactComponentTree(componentType, null, null, evaluatedRootNode); + let parentOptimizedFunction = realm.currentOptimizedFunction; + let componentTreeEffects = realm.withNewOptimizedFunction( + () => reconciler.resolveReactComponentTree(componentType, null, null, evaluatedRootNode), + componentType + ); if (realm.react.verbose) { logger.logInformation(` ✔ ${evaluatedRootNode.name} (root)`); } @@ -200,7 +240,8 @@ export function optimizeReactComponentTreeRoot( reconciler.componentTreeState, evaluatedRootNode, writeEffects, - environmentRecordIdAfterGlobalCode + preEvaluationComponentToWriteEffectFunction, + parentOptimizedFunction ); let startingComponentTreeBranches = 0; do { @@ -209,9 +250,9 @@ export function optimizeReactComponentTreeRoot( realm, reconciler, writeEffects, - environmentRecordIdAfterGlobalCode, logger, - alreadyEvaluated + alreadyEvaluated, + preEvaluationComponentToWriteEffectFunction ); } while (startingComponentTreeBranches !== reconciler.branchedComponentTrees.length); } diff --git a/src/react/utils.js b/src/react/utils.js index 9baa33b79..9d07d9462 100644 --- a/src/react/utils.js +++ b/src/react/utils.js @@ -34,7 +34,7 @@ import { TemporalObjectAssignEntry } from "../utils/generator.js"; import type { Descriptor, ReactComponentTreeConfig, ReactHint, PropertyBinding } from "../types.js"; import { Get, IsDataDescriptor } from "../methods/index.js"; import { computeBinary } from "../evaluators/BinaryExpression.js"; -import type { AdditionalFunctionEffects, ReactEvaluatedNode } from "../serializer/types.js"; +import type { AdditionalFunctionTransform, ReactEvaluatedNode } from "../serializer/types.js"; import invariant from "../invariant.js"; import { Create, Properties, To } from "../singletons.js"; import traverse from "@babel/traverse"; @@ -347,7 +347,7 @@ function GetDescriptorForProperty(value: ObjectValue, propertyName: string): ?De export function convertSimpleClassComponentToFunctionalComponent( realm: Realm, complexComponentType: ECMAScriptSourceFunctionValue, - additionalFunctionEffects: AdditionalFunctionEffects + transforms: Array ): void { let prototype = complexComponentType.properties.get("prototype"); invariant(prototype); @@ -362,7 +362,7 @@ export function convertSimpleClassComponentToFunctionalComponent( // give the function the functional components params complexComponentType.$FormalParameters = [t.identifier("props"), t.identifier("context")]; // add a transform to occur after the additional function has serialized the body of the class - additionalFunctionEffects.transforms.push((body: Array) => { + transforms.push((body: Array) => { // as this was a class before and is now a functional component, we need to replace // this.props and this.context to props and context, via the function arugments let funcNode = t.functionExpression(null, [], t.blockStatement(body)); @@ -493,7 +493,7 @@ export function convertFunctionalComponentToComplexClassComponent( realm: Realm, functionalComponentType: ECMAScriptSourceFunctionValue | BoundFunctionValue, complexComponentType: void | ECMAScriptSourceFunctionValue | BoundFunctionValue, - additionalFunctionEffects: AdditionalFunctionEffects + transforms: Array ): void { invariant( complexComponentType instanceof ECMAScriptSourceFunctionValue || complexComponentType instanceof BoundFunctionValue @@ -527,7 +527,7 @@ export function convertFunctionalComponentToComplexClassComponent( functionalComponentType.symbols.set(symbol, binding); } // add a transform to occur after the additional function has serialized the body of the class - additionalFunctionEffects.transforms.push((body: Array) => { + transforms.push((body: Array) => { // as we've converted a functional component to a complex one, we are going to have issues with // "props" and "context" references, as they're now going to be "this.props" and "this.context". // we simply need a to add to vars to beginning of the body to get around this diff --git a/src/realm.js b/src/realm.js index f708f5362..936f16136 100644 --- a/src/realm.js +++ b/src/realm.js @@ -477,6 +477,7 @@ export class Realm { optimizedFunctions: Map; arrayNestedOptimizedFunctionsEnabled: boolean; + currentOptimizedFunction: FunctionValue | void; eagerlyRequireModuleDependencies: void | boolean; @@ -822,6 +823,18 @@ export class Realm { return result; } + withNewOptimizedFunction(func: () => T, optimizedFunction: FunctionValue): T { + let result: T; + let previousOptimizedFunction = this.currentOptimizedFunction; + this.currentOptimizedFunction = optimizedFunction; + try { + result = func(); + } finally { + this.currentOptimizedFunction = previousOptimizedFunction; + } + return result; + } + evaluateNodeForEffectsInGlobalEnv(node: BabelNode, state?: any, generatorName?: string): Effects { return this.wrapInGlobalEnv(() => this.evaluateNodeForEffects(node, false, this.$GlobalEnv, state, generatorName)); } diff --git a/src/serializer/functions.js b/src/serializer/functions.js index 6ad8e069c..5d90f1487 100644 --- a/src/serializer/functions.js +++ b/src/serializer/functions.js @@ -53,6 +53,7 @@ export class Functions { this._writeEffects = new Map(); this._noopFunction = undefined; this._optimizedFunctionId = 0; + this.reactFunctionMap = new Map(); } realm: Realm; @@ -60,6 +61,7 @@ export class Functions { _writeEffects: WriteEffects; _noopFunction: void | ECMAScriptSourceFunctionValue; _optimizedFunctionId: number; + reactFunctionMap: Map; _unwrapAbstract(value: AbstractValue): Value { let elements = value.values.getElements(); @@ -154,7 +156,7 @@ export class Functions { return recordedAdditionalFunctions; } - optimizeReactComponentTreeRoots(statistics: ReactStatistics, environmentRecordIdAfterGlobalCode: number): void { + optimizeReactComponentTreeRoots(statistics: ReactStatistics): void { let logger = this.moduleTracer.modules.logger; let recordedReactRootValues = this._generateInitialAdditionalFunctions("__reactComponentTrees"); // Get write effects of the components @@ -169,14 +171,17 @@ export class Functions { componentRoot, config, this._writeEffects, - environmentRecordIdAfterGlobalCode, logger, statistics, - alreadyEvaluated + alreadyEvaluated, + this.reactFunctionMap ); } } + // Note: this may only be used by nested optimized functions that are known to be evaluated inside of their parent + // optimized function's __optimize call (e.g. array.map/filter). In this case, lexical nesting is equivalent to the + // nesting of __optimize calls. getDeclaringOptimizedFunction(functionValue: ECMAScriptSourceFunctionValue): void | FunctionValue { for (let [optimizedFunctionValue, additionalEffects] of this._writeEffects) { // CreatedObjects is all objects created by this optimized function but not @@ -186,14 +191,16 @@ export class Functions { } } - processCollectedNestedOptimizedFunctions(environmentRecordIdAfterGlobalCode: number): void { + processCollectedNestedOptimizedFunctions(): void { for (let [functionValue, effects] of this.realm.collectedNestedOptimizedFunctionEffects) { let additionalFunctionEffects = createAdditionalEffects( this.realm, effects, true, "AdditionalFunctionEffects", - environmentRecordIdAfterGlobalCode, + this._writeEffects, + this.reactFunctionMap, + functionValue, this.getDeclaringOptimizedFunction(functionValue) ); invariant(additionalFunctionEffects !== null); @@ -210,12 +217,12 @@ export class Functions { let currentOptimizedFunctionId = this._optimizedFunctionId++; invariant(value instanceof ECMAScriptSourceFunctionValue); for (let t1 of this.realm.tracers) t1.beginOptimizingFunction(currentOptimizedFunctionId, value); - func(value, argModel); + this.realm.withNewOptimizedFunction(() => func(value, argModel), value); for (let t2 of this.realm.tracers) t2.endOptimizingFunction(currentOptimizedFunctionId); for (let [oldValue, model] of oldRealmOptimizedFunctions) this.realm.optimizedFunctions.set(oldValue, model); } - checkThatFunctionsAreIndependent(environmentRecordIdAfterGlobalCode: number): void { + checkThatFunctionsAreIndependent(): void { let additionalFunctionsToProcess = this._generateOptimizedFunctionsFromRealm(); // When we find declarations of nested optimized functions, we need to apply the parent // effects. @@ -246,7 +253,9 @@ export class Functions { effects, true, "AdditionalFunctionEffects", - environmentRecordIdAfterGlobalCode, + this._writeEffects, + this.reactFunctionMap, + functionValue, this.getDeclaringOptimizedFunction(functionValue) ); invariant(additionalFunctionEffects); diff --git a/src/serializer/serializer.js b/src/serializer/serializer.js index 83cfe0b99..4e9b7a6fd 100644 --- a/src/serializer/serializer.js +++ b/src/serializer/serializer.js @@ -9,7 +9,6 @@ /* @flow */ -import { EnvironmentRecord } from "../environment.js"; import { Realm, ExecutionContext } from "../realm.js"; import { CompilerDiagnostic, FatalError } from "../errors.js"; import { SourceFileCollection } from "../types.js"; @@ -141,7 +140,6 @@ export class Serializer { } let code = this._execute(sourceFileCollection, sourceMaps, onParse); - let environmentRecordIdAfterGlobalCode = EnvironmentRecord.nextId; if (this.logger.hasErrors()) return undefined; @@ -149,20 +147,18 @@ export class Serializer { statistics.resolveInitializedModules.measure(() => this.modules.resolveInitializedModules()); } - statistics.checkThatFunctionsAreIndependent.measure(() => - this.functions.checkThatFunctionsAreIndependent(environmentRecordIdAfterGlobalCode) - ); + statistics.checkThatFunctionsAreIndependent.measure(() => this.functions.checkThatFunctionsAreIndependent()); let reactStatistics; if (this.realm.react.enabled) { statistics.optimizeReactComponentTreeRoots.measure(() => { reactStatistics = new ReactStatistics(); - this.functions.optimizeReactComponentTreeRoots(reactStatistics, environmentRecordIdAfterGlobalCode); + this.functions.optimizeReactComponentTreeRoots(reactStatistics); }); } statistics.processCollectedNestedOptimizedFunctions.measure(() => - this.functions.processCollectedNestedOptimizedFunctions(environmentRecordIdAfterGlobalCode) + this.functions.processCollectedNestedOptimizedFunctions() ); statistics.dumpIR.measure(() => { diff --git a/src/serializer/types.js b/src/serializer/types.js index f0de9e91a..7954ea403 100644 --- a/src/serializer/types.js +++ b/src/serializer/types.js @@ -45,13 +45,15 @@ export type SerializedBody = { optimizedFunction?: FunctionValue, // defined if any only if type is OptimizedFunction }; +export type AdditionalFunctionTransform = (body: Array) => void; + export type AdditionalFunctionEffects = { // All of these effects must be applied for this additional function // to be properly setup parentAdditionalFunction: FunctionValue | void, effects: Effects, generator: Generator, - transforms: Array<(body: Array) => void>, + transforms: Array, additionalRoots: Set, }; diff --git a/src/serializer/utils.js b/src/serializer/utils.js index c0e275c20..9bb97ab78 100644 --- a/src/serializer/utils.js +++ b/src/serializer/utils.js @@ -26,7 +26,7 @@ import invariant from "../invariant.js"; import { IsArray, IsArrayIndex } from "../methods/index.js"; import { Logger } from "../utils/logger.js"; import { Generator } from "../utils/generator.js"; -import type { AdditionalFunctionEffects } from "./types"; +import type { AdditionalFunctionEffects, AdditionalFunctionTransform } from "./types"; import type { Binding } from "../environment.js"; import type { BabelNodeSourceLocation } from "@babel/types"; import { optionalStringOfLocation } from "../utils/babelhelpers.js"; @@ -205,14 +205,24 @@ export function createAdditionalEffects( effects: Effects, fatalOnAbrupt: boolean, name: string, - environmentRecordIdAfterGlobalCode: number, - parentAdditionalFunction: FunctionValue | void = undefined + additionalFunctionEffects: Map, + preEvaluationComponentToWriteEffectFunction: Map, + optimizedFunction: FunctionValue, + parentOptimizedFunction: FunctionValue | void, + transforms: Array = [] ): AdditionalFunctionEffects | null { - let generator = Generator.fromEffects(effects, realm, name, environmentRecordIdAfterGlobalCode); - let retValue: AdditionalFunctionEffects = { - parentAdditionalFunction, + let generator = Generator.fromEffects( effects, - transforms: [], + realm, + name, + additionalFunctionEffects, + preEvaluationComponentToWriteEffectFunction, + optimizedFunction + ); + let retValue: AdditionalFunctionEffects = { + parentAdditionalFunction: parentOptimizedFunction || undefined, + effects, + transforms, generator, additionalRoots: new Set(), }; diff --git a/src/utils/generator.js b/src/utils/generator.js index 251c0f23e..416417aa9 100644 --- a/src/utils/generator.js +++ b/src/utils/generator.js @@ -57,6 +57,8 @@ import type { SerializerOptions } from "../options.js"; import type { PathConditions, ShapeInformationInterface } from "../types.js"; import { PreludeGenerator } from "./PreludeGenerator.js"; import { PropertyDescriptor } from "../descriptors.js"; +import type { AdditionalFunctionEffects } from "../serializer/types.js"; +import { FunctionEnvironmentRecord } from "../environment"; export type OperationDescriptorType = | "ABSTRACT_FROM_TEMPLATE" @@ -680,7 +682,9 @@ export class Generator { static _generatorOfEffects( realm: Realm, name: string, - environmentRecordIdAfterGlobalCode: number, + additionalFunctionEffects: Map, + optimizedFunction: FunctionValue, + preEvaluationComponentToWriteEffectFunction: Map, effects: Effects ): Generator { let { result, generator, modifiedBindings, modifiedProperties, createdObjects } = effects; @@ -698,13 +702,36 @@ export class Generator { output.emitPropertyModification(propertyBinding); } - for (let modifiedBinding of modifiedBindings.keys()) { - // TODO #2430: Instead of looking at the environment ids, keep instead track of a createdEnvironmentRecords set, - // and only consider bindings here from environment records that already existed, or even better, - // ensure upstream that only such bindings are ever added to the modified-bindings set. - if (modifiedBinding.environment.id >= environmentRecordIdAfterGlobalCode) continue; + for (let [modifiedBinding, previousValue] of modifiedBindings.entries()) { + let cannonicalize = functionValue => + preEvaluationComponentToWriteEffectFunction.get(functionValue) || functionValue; + let optimizedFunctionValue = optimizedFunction; + invariant(optimizedFunctionValue); + invariant( + cannonicalize(optimizedFunctionValue) === optimizedFunctionValue, + "These values should be canonical already" + ); + // Walks up the parent chain for the given optimized function checking if the value or any of its parents are + // equal to the optimized function we're currently building a generator for. + let valueOrParentEqualsFunction = functionValue => { + let canonicalOptimizedFunction = cannonicalize(functionValue); + if (canonicalOptimizedFunction === optimizedFunctionValue) return true; + let additionalEffects = additionalFunctionEffects.get(canonicalOptimizedFunction); + invariant(additionalEffects !== undefined); + let parent = additionalEffects.parentAdditionalFunction; + if (parent !== undefined) return valueOrParentEqualsFunction(parent); + return false; + }; - output.emitBindingModification(modifiedBinding); + let environment = modifiedBinding.environment; + if (environment instanceof FunctionEnvironmentRecord && environment.$FunctionObject === optimizedFunctionValue) + continue; + let creatingOptimizedFunction = environment.creatingOptimizedFunction; + if (creatingOptimizedFunction && valueOrParentEqualsFunction(creatingOptimizedFunction)) continue; + // TODO #2586: modifiedBinding.value should always exist + if (modifiedBinding.value || previousValue.value) { + output.emitBindingModification(modifiedBinding); + } } if (result instanceof UndefinedValue) return output; @@ -729,10 +756,19 @@ export class Generator { effects: Effects, realm: Realm, name: string, - environmentRecordIdAfterGlobalCode: number = 0 + additionalFunctionEffects: Map, + preEvaluationComponentToWriteEffectFunction: Map, + optimizedFunction: FunctionValue ): Generator { return realm.withEffectsAppliedInGlobalEnv( - this._generatorOfEffects.bind(this, realm, name, environmentRecordIdAfterGlobalCode), + this._generatorOfEffects.bind( + this, + realm, + name, + additionalFunctionEffects, + optimizedFunction, + preEvaluationComponentToWriteEffectFunction + ), effects ); } diff --git a/test/serializer/optimized-functions/Issue2422.js b/test/serializer/optimized-functions/Issue2422.js new file mode 100644 index 000000000..b45a4fdc0 --- /dev/null +++ b/test/serializer/optimized-functions/Issue2422.js @@ -0,0 +1,22 @@ +// does not contain:23 +(function() { + function f(c) { + let h; + if (c) { + h = function() { + return 23 + 42; + }; + function g() { + return h(); + } + global.__optimize && __optimize(g); + return g; + } + } + global.__optimize && __optimize(f); + global.f = f; + + global.inspect = function() { + f(true)(); + }; +})(); diff --git a/test/serializer/optimized-functions/Issue2423.js b/test/serializer/optimized-functions/Issue2423.js new file mode 100644 index 000000000..5d01506ec --- /dev/null +++ b/test/serializer/optimized-functions/Issue2423.js @@ -0,0 +1,22 @@ +// does not contain:23 +(function() { + function f(c) { + let h; + if (c) { + h = function() { + return 23 + 42; + }; + function g() { + return h(); + } + global.__optimize && __optimize(g); + return g; + } + } + global.__optimize && __optimize(f); + global.f = f; + + global.inspect = function() { + return f(true)(); + }; +})(); diff --git a/test/serializer/optimized-functions/MissingModifiedBinding.js b/test/serializer/optimized-functions/MissingModifiedBinding.js new file mode 100644 index 000000000..074c584c1 --- /dev/null +++ b/test/serializer/optimized-functions/MissingModifiedBinding.js @@ -0,0 +1,19 @@ +(function() { + function mkLocation() { + var x; + function setter(y) { + x = y; + } + function getter() { + return x; + } + if (global.__optimize) __optimize(setter); + return { setter, getter }; + } + if (global.__optimize) __optimize(mkLocation); + global.inspect = function() { + var l = mkLocation(); + l.setter(42); + return l.getter(); + }; +})(); diff --git a/test/serializer/optimized-functions/WrongScopeTripleNested.js b/test/serializer/optimized-functions/WrongScopeTripleNested.js new file mode 100644 index 000000000..6690720ef --- /dev/null +++ b/test/serializer/optimized-functions/WrongScopeTripleNested.js @@ -0,0 +1,18 @@ +(function() { + function outer() { + let x = {}; + function middle() { + function inner() { + return x; + } + if (global.__optimize) __optimize(inner); + return inner; + } + if (global.__optimize) __optimize(middle); + return middle; + } + if (global.__optimize) __optimize(outer); + global.inspect = function() { + return outer()()() === outer()()(); + }; +})();