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
This commit is contained in:
Dominic Gannaway 2018-07-13 12:15:17 -07:00 committed by Facebook Github Bot
parent cd1b752e92
commit 1a4ab4e89d
2 changed files with 25 additions and 23 deletions

View File

@ -65,8 +65,10 @@ export class ThrowCompletion extends AbruptCompletion {
super(value, precedingEffects, location); super(value, precedingEffects, location);
this.nativeStack = nativeStack || new Error().stack; this.nativeStack = nativeStack || new Error().stack;
let realm = value.$Realm; let realm = value.$Realm;
if (realm.isInPureScope() && realm.reportSideEffectCallback !== undefined) { if (realm.isInPureScope()) {
realm.reportSideEffectCallback("EXCEPTION_THROWN", undefined, location); for (let callback of realm.reportSideEffectCallbacks) {
callback("EXCEPTION_THROWN", undefined, location);
}
} }
} }

View File

@ -300,6 +300,8 @@ export class Realm {
verbose: opts.reactVerbose || false, verbose: opts.reactVerbose || false,
}; };
this.reportSideEffectCallbacks = new Set();
this.alreadyDescribedLocations = new WeakMap(); this.alreadyDescribedLocations = new WeakMap();
this.stripFlow = opts.stripFlow || false; this.stripFlow = opts.stripFlow || false;
@ -346,9 +348,9 @@ export class Realm {
createdObjects: void | CreatedObjects; createdObjects: void | CreatedObjects;
createdObjectsTrackedForLeaks: void | CreatedObjects; createdObjectsTrackedForLeaks: void | CreatedObjects;
reportObjectGetOwnProperties: void | (ObjectValue => void); reportObjectGetOwnProperties: void | (ObjectValue => void);
reportSideEffectCallback: reportSideEffectCallbacks: Set<
| void (sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, expressionLocation: any) => void
| ((sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, expressionLocation: any) => void); >;
reportPropertyAccess: void | (PropertyBinding => void); reportPropertyAccess: void | (PropertyBinding => void);
savedCompletion: void | PossiblyNormalCompletion; savedCompletion: void | PossiblyNormalCompletion;
@ -716,26 +718,21 @@ export class Realm {
| ((sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, value: void | Value) => void) | ((sideEffectType: SideEffectType, binding: void | Binding | PropertyBinding, value: void | Value) => void)
): T { ): T {
let saved_createdObjectsTrackedForLeaks = this.createdObjectsTrackedForLeaks; let saved_createdObjectsTrackedForLeaks = this.createdObjectsTrackedForLeaks;
let saved_reportSideEffectCallback = this.reportSideEffectCallback;
// Track all objects (including function closures) created during // Track all objects (including function closures) created during
// this call. This will be used to make the assumption that every // this call. This will be used to make the assumption that every
// *other* object is unchanged (pure). These objects are marked // *other* object is unchanged (pure). These objects are marked
// as leaked if they're passed to abstract functions. // as leaked if they're passed to abstract functions.
this.createdObjectsTrackedForLeaks = new Set(); this.createdObjectsTrackedForLeaks = new Set();
this.reportSideEffectCallback = (...args) => { if (reportSideEffectFunc !== null) {
if (reportSideEffectFunc != null) { this.reportSideEffectCallbacks.add(reportSideEffectFunc);
reportSideEffectFunc(...args); }
}
// Ensure we call any previously nested side-effect callbacks
if (saved_reportSideEffectCallback != null) {
saved_reportSideEffectCallback(...args);
}
};
try { try {
return f(); return f();
} finally { } finally {
this.createdObjectsTrackedForLeaks = saved_createdObjectsTrackedForLeaks; 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 !== undefined &&
!this.modifiedBindings.has(binding) && !this.modifiedBindings.has(binding) &&
value !== undefined && value !== undefined &&
this.isInPureScope() && this.isInPureScope()
this.reportSideEffectCallback !== undefined
) { ) {
let env = binding.environment; let env = binding.environment;
@ -1548,7 +1544,9 @@ export class Realm {
!(env instanceof DeclarativeEnvironmentRecord) || !(env instanceof DeclarativeEnvironmentRecord) ||
(env instanceof DeclarativeEnvironmentRecord && !isDefinedInsidePureFn(env)) (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 (createdObjectsTrackedForLeaks !== undefined && !createdObjectsTrackedForLeaks.has(object)) {
if (binding.object === this.$GlobalObject) { if (binding.object === this.$GlobalObject) {
this.reportSideEffectCallback && for (let callback of this.reportSideEffectCallbacks) {
this.reportSideEffectCallback("MODIFIED_GLOBAL", binding, object.expressionLocation); callback("MODIFIED_GLOBAL", binding, object.expressionLocation);
}
} else { } else {
this.reportSideEffectCallback && for (let callback of this.reportSideEffectCallbacks) {
this.reportSideEffectCallback("MODIFIED_PROPERTY", binding, object.expressionLocation); callback("MODIFIED_PROPERTY", binding, object.expressionLocation);
}
} }
} }
} }