Make sure that Completion.effects and Effects.result are consistent (#2244)

Summary:
Release note: none

I'm trying to move from a world where we pass arounds effects and completions separately to a world where only completions are passed around. To get there, we need to get a one to one correspondence between completions and the effects that they complete. This is not currently the case and getting there seems to be quite a bit of work.

To make things more reviewable, I'm breaking out the first part of that. Hence, the todos in the code.

This bit only sets up Effects to always make itself be the value of the effects property of its completion (result). Because of existing cases where a single completion can complete several Effects objects, there is an exemption for such cases, along with todos.
Pull Request resolved: https://github.com/facebook/prepack/pull/2244

Differential Revision: D8825625

Pulled By: hermanventer

fbshipit-source-id: 7fb835d68e806ffd96845983f4d3db6837d1ad5a

src/completions.js

@@ -11,23 +11,24 @@
 
 import type { BabelNodeSourceLocation } from "babel-types";
 import invariant from "./invariant.js";
-import type { Effects, Realm } from "./realm.js";
+import { Effects, Realm } from "./realm.js";
 import { AbstractValue, EmptyValue, Value } from "./values/index.js";
 
 export class Completion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation, target?: ?string) {
+  constructor(value: Value, precedingEffects: void | Effects, location: ?BabelNodeSourceLocation, target?: ?string) {
     this.value = value;
+    this.effects = precedingEffects;
+    if (precedingEffects !== undefined) precedingEffects.result = this;
     this.target = target;
     this.location = location;
     invariant(this.constructor !== Completion, "Completion is an abstract base class");
   }
 
   value: Value;
+  effects: void | Effects;
   target: ?string;
   location: ?BabelNodeSourceLocation;
 
-  effects: ?Effects;
-
   toDisplayString(): string {
     return "[" + this.constructor.name + " value " + (this.value ? this.value.toDisplayString() : "undefined") + "]";
   }
@@ -35,8 +36,8 @@ export class Completion {
 
 // Normal completions are returned just like spec completions
 export class NormalCompletion extends Completion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation, target?: ?string) {
-    super(value, location, target);
+  constructor(value: Value, precedingEffects: void | Effects, location: ?BabelNodeSourceLocation, target?: ?string) {
+    super(value, precedingEffects, location, target);
     invariant(this.constructor !== NormalCompletion, "NormalCompletion is an abstract base class");
   }
 }
@@ -48,15 +49,20 @@ export class SimpleNormalCompletion extends NormalCompletion {}
 // Abrupt completions are thrown as exeptions, to make it a easier
 // to quickly get to the matching high level construct.
 export class AbruptCompletion extends Completion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation, target?: ?string) {
-    super(value, location, target);
+  constructor(value: Value, precedingEffects: void | Effects, location: ?BabelNodeSourceLocation, target?: ?string) {
+    super(value, precedingEffects, location, target);
     invariant(this.constructor !== AbruptCompletion, "AbruptCompletion is an abstract base class");
   }
 }
 
 export class ThrowCompletion extends AbruptCompletion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation, nativeStack?: ?string) {
-    super(value, location);
+  constructor(
+    value: Value,
+    precedingEffects: void | Effects,
+    location: ?BabelNodeSourceLocation,
+    nativeStack?: ?string
+  ) {
+    super(value, precedingEffects, location);
     this.nativeStack = nativeStack || new Error().stack;
     let realm = value.$Realm;
     if (realm.isInPureScope() && realm.reportSideEffectCallback !== undefined) {
@@ -68,20 +74,20 @@ export class ThrowCompletion extends AbruptCompletion {
 }
 
 export class ContinueCompletion extends AbruptCompletion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation, target: ?string) {
-    super(value, location, target || null);
+  constructor(value: Value, precedingEffects: void | Effects, location: ?BabelNodeSourceLocation, target: ?string) {
+    super(value, precedingEffects, location, target || null);
   }
 }
 
 export class BreakCompletion extends AbruptCompletion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation, target: ?string) {
-    super(value, location, target || null);
+  constructor(value: Value, precedingEffects: void | Effects, location: ?BabelNodeSourceLocation, target: ?string) {
+    super(value, precedingEffects, location, target || null);
   }
 }
 
 export class ReturnCompletion extends AbruptCompletion {
-  constructor(value: Value, location: ?BabelNodeSourceLocation) {
-    super(value, location);
+  constructor(value: Value, precedingEffects: void | Effects, location: ?BabelNodeSourceLocation) {
+    super(value, precedingEffects, location);
   }
 }
 
@@ -94,7 +100,7 @@ export class ForkedAbruptCompletion extends AbruptCompletion {
     alternate: AbruptCompletion,
     alternateEffects: Effects
   ) {
-    super(realm.intrinsics.empty, consequent.location);
+    super(realm.intrinsics.empty, undefined, consequent.location);
     invariant(consequentEffects);
     invariant(alternateEffects);
     this.joinCondition = joinCondition;
@@ -121,19 +127,29 @@ export class ForkedAbruptCompletion extends AbruptCompletion {
   }
 
   updateConsequentKeepingCurrentEffects(newConsequent: AbruptCompletion): AbruptCompletion {
-    let effects = this.consequent.effects;
-    invariant(effects);
-    newConsequent.effects = effects;
-    newConsequent.effects.result = newConsequent;
+    let e = this.consequent.effects;
+    invariant(e);
+    newConsequent.effects = new Effects(
+      newConsequent,
+      e.generator,
+      e.modifiedBindings,
+      e.modifiedProperties,
+      e.createdObjects
+    );
     this.consequent = newConsequent;
     return this;
   }
 
   updateAlternateKeepingCurrentEffects(newAlternate: AbruptCompletion): AbruptCompletion {
-    let effects = this.alternate.effects;
-    invariant(effects);
-    newAlternate.effects = effects;
-    newAlternate.effects.result = newAlternate;
+    let e = this.alternate.effects;
+    invariant(e);
+    newAlternate.effects = new Effects(
+      newAlternate,
+      e.generator,
+      e.modifiedBindings,
+      e.modifiedProperties,
+      e.createdObjects
+    );
     this.alternate = newAlternate;
     return this;
   }
@@ -200,7 +216,7 @@ export class PossiblyNormalCompletion extends NormalCompletion {
     invariant(consequent === consequentEffects.result);
     invariant(alternate === alternateEffects.result);
     invariant(consequent instanceof NormalCompletion || alternate instanceof NormalCompletion);
-    super(value, consequent.location);
+    super(value, undefined, consequent.location);
     this.joinCondition = joinCondition;
     consequent.effects = consequentEffects;
     alternate.effects = alternateEffects;

src/evaluators/BreakStatement.js

@@ -21,5 +21,5 @@ export default function(
   env: LexicalEnvironment,
   realm: Realm
 ): Value {
-  throw new BreakCompletion(realm.intrinsics.empty, ast.loc, ast.label && ast.label.name);
+  throw new BreakCompletion(realm.intrinsics.empty, undefined, ast.loc, ast.label && ast.label.name);
 }

src/evaluators/ContinueStatement.js

@@ -21,5 +21,5 @@ export default function(
   env: LexicalEnvironment,
   realm: Realm
 ): Value {
-  throw new ContinueCompletion(realm.intrinsics.empty, ast.loc, ast.label && ast.label.name);
+  throw new ContinueCompletion(realm.intrinsics.empty, undefined, ast.loc, ast.label && ast.label.name);
 }

src/evaluators/ForOfStatement.js

@@ -167,7 +167,7 @@ export function ForInOfHeadEvaluation(
     // a. If exprValue.[[Value]] is null or undefined, then
     if (exprValue instanceof NullValue || exprValue instanceof UndefinedValue) {
       // i. Return Completion{[[Type]]: break, [[Value]]: empty, [[Target]]: empty}.
-      throw new BreakCompletion(realm.intrinsics.empty, expr.loc, null);
+      throw new BreakCompletion(realm.intrinsics.empty, undefined, expr.loc, null);
     }
 
     // b. Let obj be ToObject(exprValue).

src/evaluators/ForStatement.js

@@ -211,7 +211,7 @@ function ForBodyEvaluation(
     // Incorporate the savedCompletion (we should only get called if there is one).
     invariant(realm.savedCompletion !== undefined);
     if (valueOrCompletionAtLoopContinuePoint instanceof Value)
-      valueOrCompletionAtLoopContinuePoint = new ContinueCompletion(valueOrCompletionAtLoopContinuePoint);
+      valueOrCompletionAtLoopContinuePoint = new ContinueCompletion(valueOrCompletionAtLoopContinuePoint, undefined);
     let abruptCompletion = Functions.incorporateSavedCompletion(realm, valueOrCompletionAtLoopContinuePoint);
     invariant(abruptCompletion instanceof AbruptCompletion);
 
@@ -251,7 +251,7 @@ function ForBodyEvaluation(
 
     // Incorporate the savedCompletion if there is one.
     if (valueOrCompletionAtUnconditionalExit instanceof Value)
-      valueOrCompletionAtUnconditionalExit = new BreakCompletion(valueOrCompletionAtUnconditionalExit);
+      valueOrCompletionAtUnconditionalExit = new BreakCompletion(valueOrCompletionAtUnconditionalExit, undefined);
     let abruptCompletion = Functions.incorporateSavedCompletion(realm, valueOrCompletionAtUnconditionalExit);
     invariant(abruptCompletion instanceof AbruptCompletion);
 

src/evaluators/Program.js

@@ -273,7 +273,7 @@ export default function(ast: BabelNodeProgram, strictCode: boolean, env: Lexical
         // Join e with the remaining completions
         let normalGenerator = e.generator;
         e.generator = new Generator(realm, "dummy", normalGenerator.pathConditions); // This generator comes after everything else.
-        let r = (e.result = new ThrowCompletion(realm.intrinsics.empty));
+        let r = new ThrowCompletion(realm.intrinsics.empty, e);
         let fc = Join.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, res, r, e);
         let allEffects = Join.extractAndJoinCompletionsOfType(ThrowCompletion, realm, fc);
         realm.applyEffects(allEffects, "all code", true);

src/evaluators/ReturnStatement.js

@@ -28,5 +28,5 @@ export default function(
   } else {
     arg = realm.intrinsics.undefined;
   }
-  throw new ReturnCompletion(arg, ast.loc);
+  throw new ReturnCompletion(arg, undefined, ast.loc);
 }

src/evaluators/ThrowStatement.js

@@ -24,5 +24,5 @@ export default function(
 ): Value {
   let exprRef = env.evaluate(ast.argument, strictCode);
   let exprValue = Environment.GetValue(realm, exprRef);
-  throw new ThrowCompletion(exprValue, ast.loc);
+  throw new ThrowCompletion(exprValue, undefined, ast.loc);
 }

src/intrinsics/ecma262/GeneratorPrototype.js

@@ -30,7 +30,7 @@ export default function(realm: Realm, obj: ObjectValue): void {
     let g = context;
 
     // 2. Let C be Completion{[[Type]]: return, [[Value]]: value, [[Target]]: empty}.
-    let C = new ReturnCompletion(value, realm.currentLocation);
+    let C = new ReturnCompletion(value, undefined, realm.currentLocation);
 
     // 3. Return ? GeneratorResumeAbrupt(g, C).
     return GeneratorResumeAbrupt(realm, g, C);
@@ -42,7 +42,7 @@ export default function(realm: Realm, obj: ObjectValue): void {
     let g = context;
 
     // 2. Let C be Completion{[[Type]]: throw, [[Value]]: exception, [[Target]]: empty}.
-    let C = new ReturnCompletion(exception, realm.currentLocation);
+    let C = new ReturnCompletion(exception, undefined, realm.currentLocation);
 
     // 3. Return ? GeneratorResumeAbrupt(g, C).
     return GeneratorResumeAbrupt(realm, g, C);

src/methods/call.js

@@ -323,7 +323,7 @@ function callNativeFunctionValue(
     }
   };
 
-  const wrapInReturnCompletion = contextVal => new ReturnCompletion(contextVal, realm.currentLocation);
+  const wrapInReturnCompletion = contextVal => new ReturnCompletion(contextVal, undefined, realm.currentLocation);
 
   if (context instanceof AbstractObjectValue && context.kind === "conditional") {
     let [condValue, consequentVal, alternateVal] = context.args;
@@ -379,7 +379,7 @@ export function OrdinaryCallEvaluateBody(
       GeneratorStart(realm, G, code);
 
       // 4. Return Completion{[[Type]]: return, [[Value]]: G, [[Target]]: empty}.
-      return new ReturnCompletion(G, realm.currentLocation);
+      return new ReturnCompletion(G, undefined, realm.currentLocation);
     } else {
       // TODO #1586: abstractRecursionSummarization is disabled for now, as it is likely too limiting
       // (as observed in large internal tests).
@@ -455,7 +455,7 @@ export function OrdinaryCallEvaluateBody(
           // converge into a single flow using their joint effects to update the post join point state.
           if (!(c instanceof ReturnCompletion)) {
             if (!(c instanceof AbruptCompletion)) {
-              c = new ReturnCompletion(realm.intrinsics.undefined, realm.currentLocation);
+              c = new ReturnCompletion(realm.intrinsics.undefined, undefined, realm.currentLocation);
             }
           }
           invariant(c instanceof AbruptCompletion);

src/methods/join.js

@@ -453,9 +453,9 @@ export class JoinImplementation {
     if (ce.result instanceof CompletionType) {
       // Erase completions of type CompletionType and prepare for transformation of c to a possibly normal completion
       if (c.consequent instanceof CompletionType) {
-        c = c.updateConsequentKeepingCurrentEffects(new SimpleNormalCompletion(realm.intrinsics.empty));
+        c.updateConsequentKeepingCurrentEffects(new SimpleNormalCompletion(realm.intrinsics.empty, undefined));
       } else if (c.consequent instanceof ForkedAbruptCompletion && c.consequent.containsCompletion(NormalCompletion)) {
-        c = c.updateConsequentKeepingCurrentEffects((c.consequent.transferChildrenToPossiblyNormalCompletion(): any));
+        c.updateConsequentKeepingCurrentEffects((c.consequent.transferChildrenToPossiblyNormalCompletion(): any));
       }
     } else {
       ce.result = new CompletionType(realm.intrinsics.empty);
@@ -469,9 +469,9 @@ export class JoinImplementation {
     if (ae.result instanceof CompletionType) {
       // Erase completions of type CompletionType and prepare for transformation of c to a possibly normal completion
       if (c.alternate instanceof CompletionType) {
-        c = c.updateAlternateKeepingCurrentEffects(new SimpleNormalCompletion(realm.intrinsics.empty));
+        c.updateAlternateKeepingCurrentEffects(new SimpleNormalCompletion(realm.intrinsics.empty, undefined));
       } else if (c.alternate instanceof ForkedAbruptCompletion && c.alternate.containsCompletion(NormalCompletion)) {
-        c = c.updateAlternateKeepingCurrentEffects((c.alternate.transferChildrenToPossiblyNormalCompletion(): any));
+        c.updateAlternateKeepingCurrentEffects((c.alternate.transferChildrenToPossiblyNormalCompletion(): any));
       }
     } else {
       ae.result = new CompletionType(realm.intrinsics.empty);
@@ -480,7 +480,7 @@ export class JoinImplementation {
     let e = this.joinForkOrChoose(realm, c.joinCondition, ce, ae);
     if (e.result instanceof ForkedAbruptCompletion) {
       if (e.result.consequent instanceof CompletionType && e.result.alternate instanceof CompletionType) {
-        e.result = this.collapseResults(realm, e.result.joinCondition, e.result.consequent, e.result.alternate);
+        e.result = this.collapseResults(realm, e.result.joinCondition, e, e.result.consequent, e.result.alternate);
       }
     }
     return e;
@@ -552,7 +552,7 @@ export class JoinImplementation {
       let e1 = this.joinNestedEffects(realm, c.consequent, c.consequentEffects);
       let e2 = this.joinNestedEffects(realm, c.alternate, c.alternateEffects);
       let e3 = this.joinForkOrChoose(realm, c.joinCondition, e1, e2);
-      e3.result = this.collapseResults(realm, c.joinCondition, e1.result, e2.result);
+      this.collapseResults(realm, c.joinCondition, e3, e1.result, e2.result);
       return e3;
     }
     if (precedingEffects !== undefined) return precedingEffects;
@@ -564,6 +564,7 @@ export class JoinImplementation {
   collapseResults(
     realm: Realm,
     joinCondition: AbstractValue,
+    precedingEffects: Effects,
     result1: EvaluationResult,
     result2: EvaluationResult
   ): Completion {
@@ -575,19 +576,24 @@ export class JoinImplementation {
     if (result1 instanceof BreakCompletion && result2 instanceof BreakCompletion && result1.target === result2.target) {
       let val = this.joinValues(realm, result1.value, result2.value, getAbstractValue);
       invariant(val instanceof Value);
-      return new BreakCompletion(val, joinCondition.expressionLocation, result1.target);
+      return new BreakCompletion(val, precedingEffects, joinCondition.expressionLocation, result1.target);
     }
     if (
       result1 instanceof ContinueCompletion &&
       result2 instanceof ContinueCompletion &&
       result1.target === result2.target
     ) {
-      return new ContinueCompletion(realm.intrinsics.empty, joinCondition.expressionLocation, result1.target);
+      return new ContinueCompletion(
+        realm.intrinsics.empty,
+        precedingEffects,
+        joinCondition.expressionLocation,
+        result1.target
+      );
     }
     if (result1 instanceof ReturnCompletion && result2 instanceof ReturnCompletion) {
       let val = this.joinValues(realm, result1.value, result2.value, getAbstractValue);
       invariant(val instanceof Value);
-      return new ReturnCompletion(val, joinCondition.expressionLocation);
+      return new ReturnCompletion(val, precedingEffects, joinCondition.expressionLocation);
     }
     if (result1 instanceof ThrowCompletion && result2 instanceof ThrowCompletion) {
       getAbstractValue = (v1: void | Value, v2: void | Value) => {
@@ -595,10 +601,10 @@ export class JoinImplementation {
       };
       let val = this.joinValues(realm, result1.value, result2.value, getAbstractValue);
       invariant(val instanceof Value);
-      return new ThrowCompletion(val, result1.location);
+      return new ThrowCompletion(val, precedingEffects, result1.location);
     }
     if (result1 instanceof SimpleNormalCompletion && result2 instanceof SimpleNormalCompletion) {
-      return new SimpleNormalCompletion(getAbstractValue(result1.value, result2.value));
+      return new SimpleNormalCompletion(getAbstractValue(result1.value, result2.value), precedingEffects);
     }
     AbstractValue.reportIntrospectionError(joinCondition);
     throw new FatalError();

src/partial-evaluators/BreakStatement.js

@@ -21,6 +21,6 @@ export default function(
   env: LexicalEnvironment,
   realm: Realm
 ): [BreakCompletion, BabelNodeBreakStatement, Array<BabelNodeStatement>] {
-  let result = new BreakCompletion(realm.intrinsics.empty, ast.loc, ast.label && ast.label.name);
+  let result = new BreakCompletion(realm.intrinsics.empty, undefined, ast.loc, ast.label && ast.label.name);
   return [result, ast, []];
 }

src/partial-evaluators/ContinueStatement.js

@@ -21,6 +21,6 @@ export default function(
   env: LexicalEnvironment,
   realm: Realm
 ): [ContinueCompletion, BabelNodeContinueStatement, Array<BabelNodeStatement>] {
-  let result = new ContinueCompletion(realm.intrinsics.empty, ast.loc, ast.label && ast.label.name);
+  let result = new ContinueCompletion(realm.intrinsics.empty, undefined, ast.loc, ast.label && ast.label.name);
   return [result, ast, []];
 }

src/partial-evaluators/ReturnStatement.js

@@ -27,6 +27,6 @@ export default function(
   } else {
     result = realm.intrinsics.undefined;
   }
-  if (!(result instanceof AbruptCompletion)) result = new ReturnCompletion(result, ast.loc);
+  if (!(result instanceof AbruptCompletion)) result = new ReturnCompletion(result, undefined, ast.loc);
   return [result, ast, []];
 }

src/partial-evaluators/ThrowStatement.js

@@ -24,7 +24,7 @@ export default function(
 ): [Completion | Value, BabelNode, Array<BabelNodeStatement>] {
   let [argValue, argAst, io] = env.partiallyEvaluateCompletionDeref(ast.argument, strictCode);
   if (argValue instanceof Value) {
-    let c = new ThrowCompletion(argValue, ast.loc);
+    let c = new ThrowCompletion(argValue, undefined, ast.loc);
     let s = t.throwStatement((argAst: any)); // will be an expression because argValue is a Value
     return [c, s, io];
   }

src/realm.js

@@ -88,13 +88,13 @@ let effects_uid = 0;
 
 export class Effects {
   constructor(
-    result: EvaluationResult,
+    result: Completion,
     generator: Generator,
     bindings: Bindings,
     propertyBindings: PropertyBindings,
     createdObjects: CreatedObjects
   ) {
-    this.result = result;
+    this._result = result;
     this.generator = generator;
     this.modifiedBindings = bindings;
     this.modifiedProperties = propertyBindings;
@@ -102,9 +102,18 @@ export class Effects {
 
     this.canBeApplied = true;
     this._id = effects_uid++;
+    if (result.effects === undefined) result.effects = this; //todo: require callers to ensure this
+  }
+
+  _result: Completion;
+  get result(): Completion {
+    return this._result;
+  }
+  set result(completion: Completion): void {
+    if (completion.effects === undefined) completion.effects = this; //todo: require callers to ensure this
+    this._result = completion;
   }
 
-  result: EvaluationResult;
   generator: Generator;
   modifiedBindings: Bindings;
   modifiedProperties: PropertyBindings;
@@ -194,7 +203,7 @@ export class ExecutionContext {
 
 export function construct_empty_effects(
   realm: Realm,
-  c: Completion = new SimpleNormalCompletion(realm.intrinsics.empty)
+  c: Completion = new SimpleNormalCompletion(realm.intrinsics.empty, undefined)
 ): Effects {
   // TODO #2222: Check if `realm.pathConditions` is always correct here.
   // The path conditions here should probably be empty.
@@ -1203,9 +1212,7 @@ export class Realm {
   }
 
   composeEffects(priorEffects: Effects, subsequentEffects: Effects): Effects {
-    let result = construct_empty_effects(this);
-
-    result.result = subsequentEffects.result;
+    let result = construct_empty_effects(this, subsequentEffects.result);
 
     result.generator = Join.composeGenerators(
       this,
@@ -1733,7 +1740,7 @@ export class Realm {
     if (typeof message === "string") message = new StringValue(this, message);
     invariant(message instanceof StringValue);
     this.nextContextLocation = this.currentLocation;
-    return new ThrowCompletion(Construct(this, type, [message]), this.currentLocation);
+    return new ThrowCompletion(Construct(this, type, [message]), undefined, this.currentLocation);
   }
 
   appendGenerator(generator: Generator, leadingComment: string = ""): void {

src/types.js

@@ -764,6 +764,7 @@ export type JoinType = {
   collapseResults(
     realm: Realm,
     joinCondition: AbstractValue,
+    precedingEffects: Effects,
     result1: EvaluationResult,
     result2: EvaluationResult
   ): Completion,

src/utils/parse.js

@@ -67,7 +67,7 @@ export default function(
         loc: e.loc,
         stackDecorated: false,
       };
-      throw new ThrowCompletion(error, e.loc);
+      throw new ThrowCompletion(error, undefined, e.loc);
     } else {
       throw e;
     }

src/values/NativeFunctionValue.js

@@ -119,6 +119,7 @@ export default class NativeFunctionValue extends ECMAScriptFunctionValue {
     }
     return new ReturnCompletion(
       this.callback(context, argsList, originalLength, newTarget),
+      undefined,
       this.$Realm.currentLocation
     );
   }