Allow PossiblyNormalCompletion to have both forks be normal (#2230)

Summary:
Release note: none

I'd like to get rid of ErasedAbruptCompletion and restore the invariant that any subclass of AbruptCompletion is strictly abrupt. To do that, I need to relax some invariants in PossiblyNormalCompletion.

This is also a first step towards fixing the code for composing possibly normal completions correctly.

I've pulled this out into a relatively small PR to make reviewing easier. Hence, the remaining TODO in the code.

Even this smallish change results in some regression because previously different code paths were followed that allowed simplification to succeed that will now fail. Fixing that is too ambitious for this PR and will have to wait.
Pull Request resolved: https://github.com/facebook/prepack/pull/2230

Differential Revision: D8808922

Pulled By: hermanventer

fbshipit-source-id: 4afce5e27bc30290b2b237b40c1e92f8c22bd100

src/completions.js

@@ -131,7 +131,7 @@ export class ForkedAbruptCompletion extends AbruptCompletion {
     newConsequent.effects = effects;
     newConsequent.effects.result = newConsequent;
     this.consequent = newConsequent;
-    return newConsequent;
+    return this;
   }
 
   updateAlternateKeepingCurrentEffects(newAlternate: AbruptCompletion): AbruptCompletion {
@@ -140,7 +140,7 @@ export class ForkedAbruptCompletion extends AbruptCompletion {
     newAlternate.effects = effects;
     newAlternate.effects.result = newAlternate;
     this.alternate = newAlternate;
-    return newAlternate;
+    return this;
   }
 
   toDisplayString(): string {
@@ -205,20 +205,7 @@ export class PossiblyNormalCompletion extends NormalCompletion {
     invariant(consequent === consequentEffects.result);
     invariant(alternate === alternateEffects.result);
     invariant(consequent instanceof NormalCompletion || alternate instanceof NormalCompletion);
-    invariant(consequent instanceof AbruptCompletion || alternate instanceof AbruptCompletion);
+    super(value, consequent.location);
-    invariant(
-      consequent instanceof AbruptCompletion || (consequent instanceof NormalCompletion && value === consequent.value)
-    );
-    invariant(
-      alternate instanceof AbruptCompletion || (alternate instanceof NormalCompletion && value === alternate.value)
-    );
-    let loc =
-      consequent instanceof AbruptCompletion
-        ? consequent.location
-        : alternate instanceof Completion
-          ? alternate.location
-          : alternate.expressionLocation;
-    super(value, loc);
     this.joinCondition = joinCondition;
     consequent.effects = consequentEffects;
     alternate.effects = alternateEffects;
@@ -247,24 +234,22 @@ export class PossiblyNormalCompletion extends NormalCompletion {
     return this.alternate.effects;
   }
 
-  // TODO blappert: these functions are a copy of those in ForkedAbruptCompletion, but the two classes will be unified
+  updateConsequentKeepingCurrentEffects(newConsequent: Completion): PossiblyNormalCompletion {
-  // soon
-  updateConsequentKeepingCurrentEffects(newConsequent: Completion): Completion {
     if (newConsequent instanceof NormalCompletion) this.value = newConsequent.value;
     let effects = this.consequentEffects;
     newConsequent.effects = effects;
     newConsequent.effects.result = newConsequent;
     this.consequent = newConsequent;
-    return newConsequent;
+    return this;
   }
 
-  updateAlternateKeepingCurrentEffects(newAlternate: Completion): Completion {
+  updateAlternateKeepingCurrentEffects(newAlternate: Completion): PossiblyNormalCompletion {
     if (newAlternate instanceof NormalCompletion) this.value = newAlternate.value;
     let effects = this.alternateEffects;
     newAlternate.effects = effects;
     newAlternate.effects.result = newAlternate;
     this.alternate = newAlternate;
-    return newAlternate;
+    return this;
   }
 
   toDisplayString(): string {

src/methods/join.js

@@ -20,7 +20,6 @@ import {
   BreakCompletion,
   Completion,
   ContinueCompletion,
-  ErasedAbruptCompletion,
   PossiblyNormalCompletion,
   ForkedAbruptCompletion,
   SimpleNormalCompletion,
@@ -184,7 +183,6 @@ export class JoinImplementation {
         pnc.savedEffects
       );
     } else {
-      invariant(pnc.alternate instanceof AbruptCompletion);
       if (pnc.consequent instanceof SimpleNormalCompletion) {
         let { generator, modifiedBindings, modifiedProperties, createdObjects } = pnc.consequentEffects;
         let newConsequentEffects = new Effects(c, generator, modifiedBindings, modifiedProperties, createdObjects);
@@ -253,7 +251,7 @@ export class JoinImplementation {
         if (v instanceof AbstractValue) v = realm.simplifyAndRefineAbstractValue(v);
         if (pnc.alternate instanceof SimpleNormalCompletion) {
           nc.value = v;
-          pnc.updateAlternateKeepingCurrentEffects(nc);
+          pnc = pnc.updateAlternateKeepingCurrentEffects(nc);
           pnc.value = v;
         } else {
           invariant(pnc.alternate instanceof PossiblyNormalCompletion);
@@ -267,7 +265,7 @@ export class JoinImplementation {
         if (v instanceof AbstractValue) v = realm.simplifyAndRefineAbstractValue(v);
         if (pnc.consequent instanceof SimpleNormalCompletion) {
           nc.value = v;
-          pnc.updateConsequentKeepingCurrentEffects(nc);
+          pnc = pnc.updateConsequentKeepingCurrentEffects(nc);
           pnc.value = v;
         } else {
           invariant(pnc.consequent instanceof PossiblyNormalCompletion);
@@ -302,17 +300,37 @@ export class JoinImplementation {
       let nae = new Effects(na, ae.generator, ae.modifiedBindings, ae.modifiedProperties, ae.createdObjects);
       return new ForkedAbruptCompletion(realm, pnc.joinCondition, pncc, pnc.consequentEffects, na, nae);
     } else {
-      let pnca = pnc.alternate;
+      let nc, nce;
-      invariant(pnca instanceof AbruptCompletion);
+      if (pncc instanceof PossiblyNormalCompletion) {
-      e = realm.composeEffects(pnc.consequentEffects, e);
+        nc = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, pncc, ac, e);
-      if (pnc.consequent instanceof SimpleNormalCompletion) {
+        let ce = pnc.consequentEffects;
-        return new ForkedAbruptCompletion(realm, pnc.joinCondition, ac, e, pnca, pnc.alternateEffects);
+        nce = new Effects(nc, ce.generator, ce.modifiedBindings, ce.modifiedProperties, ce.createdObjects);
+      } else {
+        invariant(pncc instanceof SimpleNormalCompletion);
+        nc = ac;
+        nce = realm.composeEffects(pnc.consequentEffects, e);
       }
-      invariant(pnc.consequent instanceof PossiblyNormalCompletion);
+      let pnca = pnc.alternate;
-      let nc = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, pnc.consequent, ac, e);
+      let na, nae;
-      let ce = pnc.consequentEffects;
+      // TODO (hermanv) if we use e as is, it ends up being applied twice when the join of the normal
-      let nce = new Effects(nc, ce.generator, ce.modifiedBindings, ce.modifiedProperties, ce.createdObjects);
+      // path is applied to the current state. Follow up with a PR that allows Effects to get deeply cloned
-      return new ForkedAbruptCompletion(realm, pnc.joinCondition, nc, nce, pnca, pnc.alternateEffects);
+      // and then clone e at this point.
+      e = construct_empty_effects(realm);
+      // Note that ac may depend on the effects in the original e, so use e.result until the cloning is done.
+      ac = (e.result: any);
+      if (pnca instanceof PossiblyNormalCompletion) {
+        na = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, pnca, ac, e);
+        let ae = pnc.alternateEffects;
+        nae = new Effects(na, ae.generator, ae.modifiedBindings, ae.modifiedProperties, ae.createdObjects);
+      } else if (pnca instanceof SimpleNormalCompletion) {
+        na = ac;
+        nae = realm.composeEffects(pnc.alternateEffects, e);
+      } else {
+        invariant(pnca instanceof AbruptCompletion);
+        na = pnca;
+        nae = pnc.alternateEffects;
+      }
+      return new ForkedAbruptCompletion(realm, pnc.joinCondition, nc, nce, na, nae);
     }
   }
 
@@ -366,7 +384,7 @@ export class JoinImplementation {
     if (pnc.consequent instanceof AbruptCompletion) {
       if (pnc.alternate instanceof SimpleNormalCompletion) {
         nc.value = AbstractValue.createFromConditionalOp(realm, joinCondition, v, pnc.alternate.value);
-        pnc.updateAlternateKeepingCurrentEffects(nc);
+        pnc = pnc.updateAlternateKeepingCurrentEffects(nc);
       } else {
         invariant(pnc.alternate instanceof PossiblyNormalCompletion);
         this.updatePossiblyNormalCompletionWithInverseConditionalSimpleNormalCompletion(
@@ -379,7 +397,7 @@ export class JoinImplementation {
     } else {
       if (pnc.consequent instanceof SimpleNormalCompletion) {
         nc.value = AbstractValue.createFromConditionalOp(realm, joinCondition, v, pnc.consequent.value);
-        pnc.updateConsequentKeepingCurrentEffects(nc);
+        pnc = pnc.updateConsequentKeepingCurrentEffects(nc);
       } else {
         invariant(pnc.consequent instanceof PossiblyNormalCompletion);
         this.updatePossiblyNormalCompletionWithInverseConditionalSimpleNormalCompletion(
@@ -403,15 +421,12 @@ export class JoinImplementation {
       if (v2 instanceof EmptyValue) return v1 || realm.intrinsics.undefined;
       return AbstractValue.createFromConditionalOp(realm, joinCondition, v1, v2);
     };
-    let ac = new ErasedAbruptCompletion(realm.intrinsics.empty);
+    let ce = construct_empty_effects(realm, c);
-    let ee = construct_empty_effects(realm, ac);
-    let fc = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, c, ac, ee);
-    let ce = construct_empty_effects(realm, fc);
     let ae = construct_empty_effects(realm, a);
     let rv = this.joinValues(realm, c.value, a.value, getAbstractValue);
     invariant(rv instanceof Value);
     a.value = rv;
-    return new PossiblyNormalCompletion(rv, joinCondition, fc, ce, a, ae, []);
+    return new PossiblyNormalCompletion(rv, joinCondition, c, ce, a, ae, []);
   }
 
   // Join all effects that result in completions of type CompletionType.
@@ -419,12 +434,7 @@ export class JoinImplementation {
   // Also erase any generators that appears in branches resulting in completions of type CompletionType.
   // Note that c is modified in place and should be replaced with a PossiblyNormalCompletion by the caller
   // if either of its branches cease to be an AbruptCompletion.
-  extractAndJoinCompletionsOfType(
+  extractAndJoinCompletionsOfType(CompletionType: typeof AbruptCompletion, realm: Realm, c: AbruptCompletion): Effects {
-    CompletionType: typeof AbruptCompletion,
-    realm: Realm,
-    c: AbruptCompletion,
-    convertToPNC: boolean = true
-  ): Effects {
     let emptyEffects = construct_empty_effects(realm);
     if (c instanceof CompletionType) {
       emptyEffects.result = c;
@@ -433,9 +443,8 @@ export class JoinImplementation {
     if (!(c instanceof ForkedAbruptCompletion)) {
       return emptyEffects;
     }
-    let erasedAbruptCompletion = new ErasedAbruptCompletion(realm.intrinsics.empty);
     // Join up the consequent and alternate completions and compose them with their prefix effects
-    let ce = this.extractAndJoinCompletionsOfType(CompletionType, realm, c.consequent, convertToPNC);
+    let ce = this.extractAndJoinCompletionsOfType(CompletionType, realm, c.consequent);
     // ce will be applied to the global state before any non joining branches in c.consequent, so move
     // the generator from c.consequentEffects to ce.generator so that all branches will see its effects.
     ce = realm.composeEffects(c.consequentEffects, ce);
@@ -444,22 +453,14 @@ 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.updateConsequentKeepingCurrentEffects(
+        c = c.updateConsequentKeepingCurrentEffects(new SimpleNormalCompletion(realm.intrinsics.empty));
-          convertToPNC ? new SimpleNormalCompletion(realm.intrinsics.empty) : erasedAbruptCompletion
+      } else if (c.consequent instanceof ForkedAbruptCompletion && c.consequent.containsCompletion(NormalCompletion)) {
-        );
+        c = c.updateConsequentKeepingCurrentEffects((c.consequent.transferChildrenToPossiblyNormalCompletion(): any));
-        convertToPNC = false;
-      } else if (
-        convertToPNC &&
-        c.consequent instanceof ForkedAbruptCompletion &&
-        c.consequent.containsCompletion(NormalCompletion)
-      ) {
-        c.updateConsequentKeepingCurrentEffects((c.consequent.transferChildrenToPossiblyNormalCompletion(): any));
-        convertToPNC = false;
       }
     } else {
       ce.result = new CompletionType(realm.intrinsics.empty);
     }
-    let ae = this.extractAndJoinCompletionsOfType(CompletionType, realm, c.alternate, convertToPNC);
+    let ae = this.extractAndJoinCompletionsOfType(CompletionType, realm, c.alternate);
     // ae will be applied to the global state before any non joining branches in c.alternate, so move
     // the generator from c.alternateEffects to ae.generator so that all branches will see its effects.
     ae = realm.composeEffects(c.alternateEffects, ae);
@@ -468,15 +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.updateAlternateKeepingCurrentEffects(
+        c = c.updateAlternateKeepingCurrentEffects(new SimpleNormalCompletion(realm.intrinsics.empty));
-          convertToPNC ? new SimpleNormalCompletion(realm.intrinsics.empty) : erasedAbruptCompletion
+      } else if (c.alternate instanceof ForkedAbruptCompletion && c.alternate.containsCompletion(NormalCompletion)) {
-        );
+        c = c.updateAlternateKeepingCurrentEffects((c.alternate.transferChildrenToPossiblyNormalCompletion(): any));
-      } else if (
-        convertToPNC &&
-        c.alternate instanceof ForkedAbruptCompletion &&
-        c.alternate.containsCompletion(NormalCompletion)
-      ) {
-        c.updateAlternateKeepingCurrentEffects((c.alternate.transferChildrenToPossiblyNormalCompletion(): any));
       }
     } else {
       ae.result = new CompletionType(realm.intrinsics.empty);

src/types.js

@@ -755,12 +755,7 @@ export type JoinType = {
     nc: SimpleNormalCompletion
   ): void,
 
-  extractAndJoinCompletionsOfType(
+  extractAndJoinCompletionsOfType(CompletionType: typeof AbruptCompletion, realm: Realm, c: AbruptCompletion): Effects,
-    CompletionType: typeof AbruptCompletion,
-    realm: Realm,
-    c: AbruptCompletion,
-    convertToPNC?: boolean
-  ): Effects,
 
   joinForkOrChoose(realm: Realm, joinCondition: Value, e1: Effects, e2: Effects): Effects,
 

test/serializer/abstract/PathConditions.js

@@ -1,4 +1,4 @@
-// does not contain:3 ===
+// does not contain:|| 3 ===
 let n1 = global.__abstract ? __abstract("number", "1") : 1;
 
 function f1() {