Eliminate child-parent read-write conflict errors (#2542)

Summary:
Release Notes: None

This PR makes it so we no longer report conflicts for child optimized functions reading from values that their parents have written.

Couple of things for discussion:
- This creates somewhat confusing behavior because the child functions will take the concrete value after the parent functions' effects have been applied:
```javascript
(function () {
    let obj = {p: 42};
    function f() {
        obj.p += 3;
        function g() { return obj.p; } // Will optimize to `return 47;`
        obj.p += 1;
        __optimize(g);
        obj.p += 1;
        return g;
    }
    __optimize(f);
    global.f = f;
})();
```
- This PR creates the Parent/Child relationship based off `AdditionalFunctionEffects.parentAdditionalFunction` which goes off of syntactic nesting of functions instead of nesting of `__optimize` calls as in the issue. I believe basing the nesting off of `__optimize` calls could lead to somewhat unintuitive results (especially considering we store `parentAdditionalFunction` in `AdditionalFunctionEffects` to be the syntactic parent).

I am not sure if it is needed by Instant Render NTillmann? If it is, we may want to consider changing `AdditionalFunctionEffects.parentAdditionalFunction` to be based off of `__optimize` call nesting as well for consistency.

As an example a slight modification on the example above:
```javascript
(function () {
    let obj = {p: 42};
    function g() { return obj.p; }
    function f() {
        obj.p += 3;
        __optimize(g);
        obj.p += 1;
        return [g, obj];
    }
    __optimize(f);
    global.f = f;
})();
```
With this PR, we report an error. Based off of `__optimize` nesting, we would optimize `g` to `return 46;`. I would expect to see `return 42;` `return 45;` or `return obj.p;` in this case.

- To resolve the above issues, in the future, we could make any values modified by parent optimized functions into abstract values during evaluation for child optimized functions to force their accesses to be recorded in generators .

Resolves #2351
Pull Request resolved: https://github.com/facebook/prepack/pull/2542

Differential Revision: D9803741

Pulled By: cblappert

fbshipit-source-id: baca233c8de81633332b25f0776ed1a9d6c95a60

src/methods/properties.js

@@ -1530,7 +1530,7 @@ export class PropertiesImplementation {
       }
       return undefined;
     }
-    realm.callReportPropertyAccess(existingBinding);
+    realm.callReportPropertyAccess(existingBinding, false);
     if (!existingBinding.descriptor) {
       if (realm.invariantLevel >= 2 && O.isIntrinsic()) {
         let realmGenerator = realm.generator;

src/realm.js

@@ -368,7 +368,7 @@ export class Realm {
   reportSideEffectCallbacks: Set<
     (sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, expressionLocation: any) => void
   >;
-  reportPropertyAccess: void | (PropertyBinding => void);
+  reportPropertyAccess: void | ((PropertyBinding, boolean) => void);
   savedCompletion: void | JoinedNormalAndAbruptCompletions;
 
   activeLexicalEnvironments: Set<LexicalEnvironment>;
@@ -1486,9 +1486,9 @@ export class Realm {
     }
   }
 
-  callReportPropertyAccess(binding: PropertyBinding): void {
+  callReportPropertyAccess(binding: PropertyBinding, isWrite: boolean): void {
     if (this.reportPropertyAccess !== undefined) {
-      this.reportPropertyAccess(binding);
+      this.reportPropertyAccess(binding, isWrite);
     }
   }
 
@@ -1523,7 +1523,7 @@ export class Realm {
       // This only happens during speculative execution and is reported elsewhere
       throw new FatalError("Trying to modify a property in read-only realm");
     }
-    this.callReportPropertyAccess(binding);
+    this.callReportPropertyAccess(binding, true);
     if (this.modifiedProperties !== undefined && !this.modifiedProperties.has(binding)) {
       let clone;
       let desc = binding.descriptor;

src/serializer/functions.js

@@ -286,6 +286,16 @@ export class Functions {
 
     // check that functions are independent
     let conflicts: Map<BabelNodeSourceLocation, CompilerDiagnostic> = new Map();
+    let isParentOf = (possibleParent, fun) => {
+      if (fun === undefined) return false;
+      let effects = this.writeEffects.get(fun);
+      invariant(effects !== undefined);
+      if (effects.parentAdditionalFunction !== undefined) {
+        if (effects.parentAdditionalFunction === possibleParent) return true;
+        return isParentOf(possibleParent, effects.parentAdditionalFunction);
+      }
+      return false;
+    };
     for (let fun1 of additionalFunctions) {
       invariant(fun1 instanceof FunctionValue);
       let fun1Location = fun1.expressionLocation;
@@ -316,19 +326,26 @@ export class Functions {
             fun2Name,
             conflicts,
             e1.modifiedProperties,
+            isParentOf(fun1, fun2),
             Utils.createModelledFunctionCall(this.realm, fun2)
           );
           return null;
         };
-        let fun2Effects = this.writeEffects.get(fun2);
-        invariant(fun2Effects);
-        if (fun2Effects.parentAdditionalFunction) {
-          let parentEffects = this.writeEffects.get(fun2Effects.parentAdditionalFunction);
-          invariant(parentEffects);
-          this.realm.withEffectsAppliedInGlobalEnv(reportFn, parentEffects.effects);
-        } else {
-          reportFn();
-        }
+        // Recursively apply all parent effects
+        let withPossibleParentEffectsApplied = (toExecute, optimizedFunction) => {
+          let funEffects = this.writeEffects.get(optimizedFunction);
+          invariant(funEffects !== undefined);
+          let parentAdditionalFunction = funEffects.parentAdditionalFunction;
+          if (parentAdditionalFunction !== undefined) {
+            let parentEffects = this.writeEffects.get(parentAdditionalFunction);
+            invariant(parentEffects !== undefined);
+            let newToExecute = () => this.realm.withEffectsAppliedInGlobalEnv(toExecute, parentEffects.effects);
+            withPossibleParentEffectsApplied(newToExecute, parentAdditionalFunction);
+          } else {
+            toExecute();
+          }
+        };
+        withPossibleParentEffectsApplied(reportFn, fun2);
       }
     }
     if (conflicts.size > 0) {
@@ -346,6 +363,7 @@ export class Functions {
     f2name: string,
     conflicts: Map<BabelNodeSourceLocation, CompilerDiagnostic>,
     pbs: PropertyBindings,
+    f1IsParentOfF2: boolean,
     call2: void => Value
   ): void {
     let reportConflict = (
@@ -379,11 +397,11 @@ export class Functions {
         reportConflict(location, ob.getDebugName(), undefined, ob.expressionLocation);
     };
     let oldReportPropertyAccess = this.realm.reportPropertyAccess;
-    this.realm.reportPropertyAccess = (pb: PropertyBinding) => {
+    this.realm.reportPropertyAccess = (pb: PropertyBinding, isWrite: boolean) => {
       if (ObjectValue.refuseSerializationOnPropertyBinding(pb)) return;
       let location = this.realm.currentLocation;
       if (!location) return; // happens only when accessing an additional function property
-      if (pbs.has(pb) && !conflicts.has(location)) {
+      if (pbs.has(pb) && (!f1IsParentOfF2 || isWrite) && !conflicts.has(location)) {
         let originalLocation =
           pb.descriptor instanceof PropertyDescriptor && pb.descriptor.value && !Array.isArray(pb.descriptor.value)
             ? pb.descriptor.value.expressionLocation

test/serializer/optimized-functions/ParentInitializesLocal.js

@@ -0,0 +1,23 @@
+(function() {
+  function f() {
+    let obj = { p: 42 };
+    function g() {
+      return obj.p;
+    }
+    function h() {
+      return obj.p;
+    }
+    if (global.__optimize) {
+      __optimize(g);
+      __optimize(h);
+    }
+    return [g, h];
+  }
+  global.__optimize && __optimize(f);
+  global.f = f;
+
+  global.inspect = function() {
+    let [g, h] = f();
+    return g() + h();
+  };
+})();

test/serializer/optimized-functions/ParentInitializesLocal2.js

@@ -0,0 +1,21 @@
+(function() {
+  function f() {
+    let obj = { p: 42 };
+    function g() {
+      function h() {
+        return obj.p;
+      }
+      global.__optimize && __optimize(h);
+      return h;
+    }
+    if (global.__optimize) __optimize(g);
+    return g;
+  }
+  global.__optimize && __optimize(f);
+  global.f = f;
+
+  global.inspect = function() {
+    let g = f();
+    return g()();
+  };
+})();