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

@@ -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 {

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<AdditionalFunctionTransform>
+): 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<FunctionValue, FunctionValue>,
+  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<ECMAScriptSourceFunctionValue | BoundFunctionValue, ReactEvaluatedNode>
+  alreadyEvaluated: Map<ECMAScriptSourceFunctionValue | BoundFunctionValue, ReactEvaluatedNode>,
+  preEvaluationComponentToWriteEffectFunction: Map<FunctionValue, FunctionValue>
 ): 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<ECMAScriptSourceFunctionValue | BoundFunctionValue, ReactEvaluatedNode>
+  alreadyEvaluated: Map<ECMAScriptSourceFunctionValue | BoundFunctionValue, ReactEvaluatedNode>,
+  preEvaluationComponentToWriteEffectFunction: Map<FunctionValue, FunctionValue>
 ): 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);
 }

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<AdditionalFunctionTransform>
 ): 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<BabelNodeStatement>) => {
+  transforms.push((body: Array<BabelNodeStatement>) => {
     // 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<AdditionalFunctionTransform>
 ): 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<BabelNodeStatement>) => {
+  transforms.push((body: Array<BabelNodeStatement>) => {
     // 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

src/realm.js

@@ -477,6 +477,7 @@ export class Realm {
 
   optimizedFunctions: Map<FunctionValue | AbstractValue, ArgModel | void>;
   arrayNestedOptimizedFunctionsEnabled: boolean;
+  currentOptimizedFunction: FunctionValue | void;
 
   eagerlyRequireModuleDependencies: void | boolean;
 
@@ -822,6 +823,18 @@ export class Realm {
     return result;
   }
 
+  withNewOptimizedFunction<T>(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));
   }

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<FunctionValue, FunctionValue>;
 
   _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);

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(() => {

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<BabelNodeStatement>) => 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<BabelNodeStatement>) => void>,
+  transforms: Array<AdditionalFunctionTransform>,
   additionalRoots: Set<ObjectValue>,
 };
 

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<FunctionValue, AdditionalFunctionEffects>,
+  preEvaluationComponentToWriteEffectFunction: Map<FunctionValue, FunctionValue>,
+  optimizedFunction: FunctionValue,
+  parentOptimizedFunction: FunctionValue | void,
+  transforms: Array<AdditionalFunctionTransform> = []
 ): 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(),
   };

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<FunctionValue, AdditionalFunctionEffects>,
+    optimizedFunction: FunctionValue,
+    preEvaluationComponentToWriteEffectFunction: Map<FunctionValue, FunctionValue>,
     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<FunctionValue, AdditionalFunctionEffects>,
+    preEvaluationComponentToWriteEffectFunction: Map<FunctionValue, FunctionValue>,
+    optimizedFunction: FunctionValue
   ): Generator {
     return realm.withEffectsAppliedInGlobalEnv(
-      this._generatorOfEffects.bind(this, realm, name, environmentRecordIdAfterGlobalCode),
+      this._generatorOfEffects.bind(
+        this,
+        realm,
+        name,
+        additionalFunctionEffects,
+        optimizedFunction,
+        preEvaluationComponentToWriteEffectFunction
+      ),
       effects
     );
   }

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)();
+  };
+})();

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)();
+  };
+})();

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();
+  };
+})();

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()()();
+  };
+})();