Nested abstract conditional fixes (#2255)

Summary:
Fixes #1867 and fixes #2058 which are issues blocking the React compiler.

Test case from #1867 copied below:

```js
(function() {
  function fn(arg) {
    if (arg.foo) {
      if (arg.bar) {
        return 42;
      }
    }
  }

  if (global.__optimize) {
    __optimize(fn);
  }

  global.inspect = function() {
    return JSON.stringify([
        fn(null),
        fn({}),
        fn({foo: true}),
    ]);
  };
})();
```

The fix is almost entirely in `joinOrForkResults` where I remove the call to `updatePossiblyNormalCompletionWithConditionalSimpleNormalCompletion` and replace it with a new `PossiblyNormalCompletion`. Here’s my understanding of the issue and the previous strategy. Hopefully hermanventer can correct me if I’m wrong or explain why it needs to be the way it was.

In our program above when we are joining the `arg.foo` consequent with its alternate we have the following completion for the `arg.foo` consequent:

- `PossiblyNormalCompletion` (join condition is `arg.bar`)
  - `ReturnCompletion(42)` (value of `42`)
  - `SimpleNormalCompletion(empty)` (there is no `else`)

We are joining this with an alternate of `SimpleNormalCompletion` since there is no `else` on the `arg.foo` if-statement. Before we would try and “sneak into” the two branches and add an abstract conditional to the result. However, the implementation was limited so we we only updated `SimpleNormalCompletion`. This meant we ended up changing our above completion to:

- `PossiblyNormalCompletion` (join condition is `arg.bar`)
  - `ReturnCompletion(42)`
  - `SimpleNormalCompletion(arg.foo ? empty : empty)`

The only change being the second `SimpleNormalCompletion` is now `arg.foo ? empty : empty`. This would serialize into roughly:

```js
function f() {
  return _$1 ? 42 : undefined;
}
```

Where `_$1` is not declared! Since we were serializing a `PossiblyNormalCompletion` with a join condition of `arg.bar` and the `arg.foo ? empty : empty` abstract conditional simplified to just `empty` which means we never emit `arg.foo` so never emit `arg.bar` which has a dependency on `arg.foo`.

My strategy was to nest the `PossiblyNormalCompletion` in another `PossiblyNormalCompletion`. So the final completion for the code sample at the top of this PR is:

- `PossiblyNormalCompletion` (join condition is `arg.foo`)
  - consequent: `PossiblyNormalCompletion` (join condition is `arg.bar`)
    - consequent: `ReturnCompletion(42)`
    - alternate: `SimpleNormalCompletion(empty)`
  - alternate: `SimpleNormalCompletion(empty)`

I’m confused as to why this was not done in the first place and instead an update strategy was used.

I then wrote a test (now in `test/serializer/optimized-functions/NestedConditions.js`) to enumerate a bunch of nested conditional cases. The rest of the fixes come from there. Notably the refactor of `replacePossiblyNormalCompletionWithForkedAbruptCompletion` which did not produce the same output when `consequent` and `alternate` were flipped.
Pull Request resolved: https://github.com/facebook/prepack/pull/2255

Differential Revision: D8865130

Pulled By: calebmer

fbshipit-source-id: 4964acc11b6403b9956cf29decefb1971340516d
This commit is contained in:
Caleb Meredith 2018-07-17 15:14:28 -07:00 committed by Facebook Github Bot
parent 5f626ebdbf
commit b0516b49ea
2 changed files with 558 additions and 39 deletions

View File

@ -294,73 +294,111 @@ export class JoinImplementation {
// effects collected after pnc was constructed
e: Effects
): ForkedAbruptCompletion {
let recurse = (xpnc, xe, nac, ne): [ForkedAbruptCompletion, Effects] => {
let nx = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, xpnc, nac, ne);
let nxe = new Effects(nx, xe.generator, xe.modifiedBindings, xe.modifiedProperties, xe.createdObjects);
return [nx, nxe];
};
let cloneEffects = () => {
let nac = ac.shallowCloneWithoutEffects();
let ne = new Effects(nac, e.generator, e.modifiedBindings, e.modifiedProperties, e.createdObjects);
return [nac, ne];
};
ac = ac.shallowCloneWithoutEffects();
e.result = ac;
ac.effects = e;
// # match (pncc, pnca)
let pncc = pnc.consequent;
let pnca = pnc.alternate;
// * case (AbruptCompletion, SimpleNormalCompletion)
// * case (AbruptCompletion, PossiblyNormalCompletion)
if (pncc instanceof AbruptCompletion) {
// todo: simplify with implied path condition
e = realm.composeEffects(pnc.alternateEffects, e);
invariant(e.result instanceof AbruptCompletion);
ac = e.result;
if (pnc.alternate instanceof SimpleNormalCompletion) {
if (pnca instanceof SimpleNormalCompletion) {
// todo: simplify with implied path condition
e = realm.composeEffects(pnc.alternateEffects, e);
invariant(e.result instanceof AbruptCompletion);
ac = e.result;
return new ForkedAbruptCompletion(realm, pnc.joinCondition, pncc, pnc.consequentEffects, ac, e);
}
invariant(pnc.alternate instanceof PossiblyNormalCompletion);
let na = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, pnc.alternate, ac, e);
let ae = pnc.alternateEffects;
let nae = new Effects(na, ae.generator, ae.modifiedBindings, ae.modifiedProperties, ae.createdObjects);
invariant(pnca instanceof PossiblyNormalCompletion);
let [na, nae] = recurse(pnca, pnc.alternateEffects, ac, e);
return new ForkedAbruptCompletion(realm, pnc.joinCondition, pncc, pnc.consequentEffects, na, nae);
} else {
let nc, nce;
if (pncc instanceof PossiblyNormalCompletion) {
nc = this.replacePossiblyNormalCompletionWithForkedAbruptCompletion(realm, pncc, ac, e);
let ce = pnc.consequentEffects;
nce = new Effects(nc, ce.generator, ce.modifiedBindings, ce.modifiedProperties, ce.createdObjects);
ac = ac.shallowCloneWithoutEffects();
e = new Effects(ac, e.generator, e.modifiedBindings, e.modifiedProperties, e.createdObjects);
} else {
invariant(pncc instanceof SimpleNormalCompletion);
nce = realm.composeEffects(pnc.consequentEffects, e);
invariant(nce.result instanceof AbruptCompletion);
nc = nce.result;
ac = ac.shallowCloneWithoutEffects();
e = new Effects(ac, e.generator, e.modifiedBindings, e.modifiedProperties, e.createdObjects);
}
// * case (SimpleNormalCompletion, AbruptCompletion)
// * case (PossiblyNormalCompletion, AbruptCompletion)
if (pnca instanceof AbruptCompletion) {
if (pncc instanceof SimpleNormalCompletion) {
// todo: simplify with implied path condition
e = realm.composeEffects(pnc.consequentEffects, e);
invariant(e.result instanceof AbruptCompletion);
ac = e.result;
return new ForkedAbruptCompletion(realm, pnc.joinCondition, ac, e, pnca, pnc.alternateEffects);
}
let pnca = pnc.alternate;
invariant(pncc instanceof PossiblyNormalCompletion);
let [nc, nce] = recurse(pncc, pnc.consequentEffects, ac, e);
return new ForkedAbruptCompletion(realm, pnc.joinCondition, nc, nce, pnca, pnc.alternateEffects);
}
// * case (SimpleNormalCompletion, SimpleNormalCompletion)
// * case (SimpleNormalCompletion, PossibleNormalCompletion)
if (pncc instanceof SimpleNormalCompletion) {
let nce = realm.composeEffects(pnc.consequentEffects, e);
invariant(nce.result instanceof AbruptCompletion);
let nc = nce.result;
[ac, e] = cloneEffects();
let na, nae;
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) {
if (pnca instanceof SimpleNormalCompletion) {
nae = realm.composeEffects(pnc.alternateEffects, e);
invariant(nae.result instanceof AbruptCompletion);
na = nae.result;
} else {
invariant(pnca instanceof AbruptCompletion);
na = pnca;
nae = pnc.alternateEffects;
invariant(pnca instanceof PossiblyNormalCompletion);
[na, nae] = recurse(pnca, pnc.alternateEffects, ac, e);
}
return new ForkedAbruptCompletion(realm, pnc.joinCondition, nc, nce, na, nae);
}
// * case (PossibleNormalCompletion, SimpleNormalCompletion)
if (pnca instanceof SimpleNormalCompletion) {
let nae = realm.composeEffects(pnc.alternateEffects, e);
invariant(nae.result instanceof AbruptCompletion);
let na = nae.result;
invariant(pncc instanceof PossiblyNormalCompletion);
[ac, e] = cloneEffects();
let [nc, nce] = recurse(pncc, pnc.consequentEffects, ac, e);
return new ForkedAbruptCompletion(realm, pnc.joinCondition, nc, nce, na, nae);
}
// * case (PossibleNormalCompletion, PossibleNormalCompletion)
invariant(pncc instanceof PossiblyNormalCompletion);
invariant(pnca instanceof PossiblyNormalCompletion);
let [nc, nce] = recurse(pncc, pnc.consequentEffects, ac, e);
[ac, e] = cloneEffects();
let [na, nae] = recurse(pnca, pnc.alternateEffects, ac, e);
return new ForkedAbruptCompletion(realm, pnc.joinCondition, nc, nce, na, nae);
// Impossible cases:
// * case (AbruptCompletion, AbruptCompletion)
}
joinNormalCompletions(
realm: Realm,
joinCondition: AbstractValue,
c: NormalCompletion,
a: NormalCompletion
ce: Effects,
a: NormalCompletion,
ae: Effects
): PossiblyNormalCompletion {
let getAbstractValue = (v1: void | Value, v2: void | Value): Value => {
if (v1 instanceof EmptyValue) return v2 || realm.intrinsics.undefined;
if (v2 instanceof EmptyValue) return v1 || realm.intrinsics.undefined;
return AbstractValue.createFromConditionalOp(realm, joinCondition, v1, v2);
};
c = c.shallowCloneWithoutEffects();
let ce = construct_empty_effects(realm, c);
a = a.shallowCloneWithoutEffects();
let ae = construct_empty_effects(realm, a);
let rv = this.joinValues(realm, c.value, a.value, getAbstractValue);
invariant(rv instanceof Value);
a.value = rv;
@ -582,7 +620,7 @@ export class JoinImplementation {
return new ForkedAbruptCompletion(realm, joinCondition, result1, e1, result2, e2);
}
if (result1 instanceof NormalCompletion && result2 instanceof NormalCompletion) {
return this.joinNormalCompletions(realm, joinCondition, result1, result2);
return this.joinNormalCompletions(realm, joinCondition, result1, e1, result2, e2);
}
if (result1 instanceof AbruptCompletion) {
let completion = result2;

View File

@ -0,0 +1,481 @@
(function() {
function fn1(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
// return 2;
}
} else {
// return 3;
}
}
function fn2(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
return 2;
}
} else {
// return 3;
}
}
function fn3(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
// return 2;
}
} else {
return 3;
}
}
function fn4(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
return 2;
}
} else {
// return 3;
}
}
function fn5(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
// return 2;
}
} else {
return 3;
}
}
function fn6(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
return 2;
}
} else {
return 3;
}
}
function fn7(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
return 2;
}
} else {
return 3;
}
}
function fn8(arg) {
if (arg.foo) {
// return 3;
} else {
if (arg.bar) {
return 1;
} else {
// return 2;
}
}
}
function fn9(arg) {
if (arg.foo) {
// return 3;
} else {
if (arg.bar) {
// return 1;
} else {
return 2;
}
}
}
function fn10(arg) {
if (arg.foo) {
return 3;
} else {
if (arg.bar) {
// return 1;
} else {
// return 2;
}
}
}
function fn11(arg) {
if (arg.foo) {
// return 3;
} else {
if (arg.bar) {
return 1;
} else {
return 2;
}
}
}
function fn12(arg) {
if (arg.foo) {
return 3;
} else {
if (arg.bar) {
return 1;
} else {
// return 2;
}
}
}
function fn13(arg) {
if (arg.foo) {
return 3;
} else {
if (arg.bar) {
// return 1;
} else {
return 2;
}
}
}
function fn14(arg) {
if (arg.foo) {
return 3;
} else {
if (arg.bar) {
return 1;
} else {
return 2;
}
}
}
function fn15(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
// return 4;
}
}
}
function fn16(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
// return 4;
}
}
}
function fn17(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
// return 4;
}
}
}
function fn18(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
return 4;
}
}
}
function fn19(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
// return 4;
}
}
}
function fn20(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
// return 4;
}
}
}
function fn21(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
return 4;
}
}
}
function fn22(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
// return 4;
}
}
}
function fn23(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
return 4;
}
}
}
function fn24(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
return 4;
}
}
}
function fn25(arg) {
if (arg.foo) {
if (arg.bar) {
// return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
return 4;
}
}
}
function fn26(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
// return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
return 4;
}
}
}
function fn27(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
// return 3;
} else {
return 4;
}
}
}
function fn28(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
// return 4;
}
}
}
function fn29(arg) {
if (arg.foo) {
if (arg.bar) {
return 1;
} else {
return 2;
}
} else {
if (arg.qux) {
return 3;
} else {
return 4;
}
}
}
if (global.__optimize) {
__optimize(fn1);
__optimize(fn2);
__optimize(fn3);
__optimize(fn4);
__optimize(fn5);
__optimize(fn6);
__optimize(fn7);
__optimize(fn8);
__optimize(fn9);
__optimize(fn10);
__optimize(fn11);
__optimize(fn12);
__optimize(fn13);
__optimize(fn14);
__optimize(fn15);
__optimize(fn16);
__optimize(fn17);
__optimize(fn18);
__optimize(fn19);
__optimize(fn20);
__optimize(fn21);
__optimize(fn22);
__optimize(fn23);
__optimize(fn24);
__optimize(fn25);
__optimize(fn26);
__optimize(fn27);
__optimize(fn28);
__optimize(fn29);
}
global.inspect = function() {
const cases = [
{ foo: true, bar: true },
{ foo: false, bar: true },
{ foo: true, bar: false },
{ foo: false, bar: false },
];
return JSON.stringify([
...cases.map(fn1),
...cases.map(fn2),
...cases.map(fn3),
...cases.map(fn4),
...cases.map(fn5),
...cases.map(fn6),
...cases.map(fn7),
...cases.map(fn8),
...cases.map(fn9),
...cases.map(fn10),
...cases.map(fn11),
...cases.map(fn12),
...cases.map(fn13),
...cases.map(fn14),
...cases.map(fn15),
...cases.map(fn16),
...cases.map(fn17),
...cases.map(fn18),
...cases.map(fn19),
...cases.map(fn20),
...cases.map(fn21),
...cases.map(fn22),
...cases.map(fn23),
...cases.map(fn24),
...cases.map(fn25),
...cases.map(fn26),
...cases.map(fn27),
...cases.map(fn28),
...cases.map(fn29),
]);
};
})();