From 1a4ab4e89d145c91a5293a8e6588d464522d8a8d Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <dg@domgan.com>
Date: Fri, 13 Jul 2018 12:15:17 -0700
Subject: [PATCH] Refactor and clean up realm.reportSideEffectCallbacks (#2254)

Summary:
Release notes: none

The previous way we deal with side-effect callbacks was confusing and hard to understand. This PR now introduces a Set to store callbacks in and the old logic has been removed.
Pull Request resolved: https://github.com/facebook/prepack/pull/2254

Differential Revision: D8840154

Pulled By: trueadm

fbshipit-source-id: ce5b03f186414285d5ca6d2d0c2c385f3c56f981
---
 src/completions.js |  6 ++++--
 src/realm.js       | 42 +++++++++++++++++++++---------------------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/completions.js b/src/completions.js
index ca8b2dd2a..99946f941 100644
--- a/src/completions.js
+++ b/src/completions.js
@@ -65,8 +65,10 @@ export class ThrowCompletion extends AbruptCompletion {
     super(value, precedingEffects, location);
     this.nativeStack = nativeStack || new Error().stack;
     let realm = value.$Realm;
-    if (realm.isInPureScope() && realm.reportSideEffectCallback !== undefined) {
-      realm.reportSideEffectCallback("EXCEPTION_THROWN", undefined, location);
+    if (realm.isInPureScope()) {
+      for (let callback of realm.reportSideEffectCallbacks) {
+        callback("EXCEPTION_THROWN", undefined, location);
+      }
     }
   }
 
diff --git a/src/realm.js b/src/realm.js
index 6209df74d..d211e191d 100644
--- a/src/realm.js
+++ b/src/realm.js
@@ -300,6 +300,8 @@ export class Realm {
       verbose: opts.reactVerbose || false,
     };
 
+    this.reportSideEffectCallbacks = new Set();
+
     this.alreadyDescribedLocations = new WeakMap();
     this.stripFlow = opts.stripFlow || false;
 
@@ -346,9 +348,9 @@ export class Realm {
   createdObjects: void | CreatedObjects;
   createdObjectsTrackedForLeaks: void | CreatedObjects;
   reportObjectGetOwnProperties: void | (ObjectValue => void);
-  reportSideEffectCallback:
-    | void
-    | ((sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, expressionLocation: any) => void);
+  reportSideEffectCallbacks: Set<
+    (sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, expressionLocation: any) => void
+  >;
   reportPropertyAccess: void | (PropertyBinding => void);
   savedCompletion: void | PossiblyNormalCompletion;
 
@@ -716,26 +718,21 @@ export class Realm {
       | ((sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, value: void | Value) => void)
   ): T {
     let saved_createdObjectsTrackedForLeaks = this.createdObjectsTrackedForLeaks;
-    let saved_reportSideEffectCallback = this.reportSideEffectCallback;
     // Track all objects (including function closures) created during
     // this call. This will be used to make the assumption that every
     // *other* object is unchanged (pure). These objects are marked
     // as leaked if they're passed to abstract functions.
     this.createdObjectsTrackedForLeaks = new Set();
-    this.reportSideEffectCallback = (...args) => {
-      if (reportSideEffectFunc != null) {
-        reportSideEffectFunc(...args);
-      }
-      // Ensure we call any previously nested side-effect callbacks
-      if (saved_reportSideEffectCallback != null) {
-        saved_reportSideEffectCallback(...args);
-      }
-    };
+    if (reportSideEffectFunc !== null) {
+      this.reportSideEffectCallbacks.add(reportSideEffectFunc);
+    }
     try {
       return f();
     } finally {
       this.createdObjectsTrackedForLeaks = saved_createdObjectsTrackedForLeaks;
-      this.reportSideEffectCallback = saved_reportSideEffectCallback;
+      if (reportSideEffectFunc !== null) {
+        this.reportSideEffectCallbacks.delete(reportSideEffectFunc);
+      }
     }
   }
 
@@ -1539,8 +1536,7 @@ export class Realm {
       this.modifiedBindings !== undefined &&
       !this.modifiedBindings.has(binding) &&
       value !== undefined &&
-      this.isInPureScope() &&
-      this.reportSideEffectCallback !== undefined
+      this.isInPureScope()
     ) {
       let env = binding.environment;
 
@@ -1548,7 +1544,9 @@ export class Realm {
         !(env instanceof DeclarativeEnvironmentRecord) ||
         (env instanceof DeclarativeEnvironmentRecord && !isDefinedInsidePureFn(env))
       ) {
-        this.reportSideEffectCallback("MODIFIED_BINDING", binding, value.expressionLocation);
+        for (let callback of this.reportSideEffectCallbacks) {
+          callback("MODIFIED_BINDING", binding, value.expressionLocation);
+        }
       }
     }
 
@@ -1591,11 +1589,13 @@ export class Realm {
 
       if (createdObjectsTrackedForLeaks !== undefined && !createdObjectsTrackedForLeaks.has(object)) {
         if (binding.object === this.$GlobalObject) {
-          this.reportSideEffectCallback &&
-            this.reportSideEffectCallback("MODIFIED_GLOBAL", binding, object.expressionLocation);
+          for (let callback of this.reportSideEffectCallbacks) {
+            callback("MODIFIED_GLOBAL", binding, object.expressionLocation);
+          }
         } else {
-          this.reportSideEffectCallback &&
-            this.reportSideEffectCallback("MODIFIED_PROPERTY", binding, object.expressionLocation);
+          for (let callback of this.reportSideEffectCallbacks) {
+            callback("MODIFIED_PROPERTY", binding, object.expressionLocation);
+          }
         }
       }
     }