Don’t evaluate catch block with an infeasible JoinedNormalAndAbruptCo…

…mpletions (#2507)

Summary:
The following example fails with an invariant:

```js
try {
  try {
    const b = __abstract("boolean", "false");
    if (b) throw new Error("throw");
  } catch (error) {}
} catch (error) {
  console.log(error.message);
}
```

```
Invariant Violation: assuming that false equals true is asking for trouble
debug-fb-www.js:208
This is likely a bug in Prepack, not your code. Feel free to open an issue on GitHub.
    at invariant (/Users/calebmer/prepack/src/invariant.js:18:15)
    at PathImplementation.withCondition (/Users/calebmer/prepack/src/utils/paths.js:132:17)
    at joinTryBlockWithHandlers (/Users/calebmer/prepack/src/evaluators/TryStatement.js:81:35)
    at _default (/Users/calebmer/prepack/src/evaluators/TryStatement.js:30:47)
    at LexicalEnvironment.evaluateAbstract (/Users/calebmer/prepack/src/environment.js:1379:20)
    at LexicalEnvironment.evaluate (/Users/calebmer/prepack/src/environment.js:1367:20)
    at LexicalEnvironment.evaluateCompletion (/Users/calebmer/prepack/src/environment.js:1102:19)
    at LexicalEnvironment.evaluateCompletionDeref (/Users/calebmer/prepack/src/environment.js:1095:23)
    at _default (/Users/calebmer/prepack/src/evaluators/Program.js:235:17)
    at LexicalEnvironment.evaluateAbstract (/Users/calebmer/prepack/src/environment.js:1379:20)
```

What happens is we end up with a completion structure as follows when going into the outer `catch`:

```
- JoinedNormalAndAbrubtCompletions (joinCondition = x)
  - SimpleNormalCompletion
  - JoinedNormalAndAbrubtCompletions (joinCondition = x)
    - ThrowCompletion
    - SimpleNormalCompletion
```

The inner `JoinedNormalAndAbrubtCompletions` is the inner `try` block completion. The outer `JoinedNormalAndAbrubtCompletions` is the join of the inner `try`/`catch` blocks. Notably the `ThrowCompletion` is completely unreachable. However it is still there.

Ideally we would refine the completions. I tried this, but realized the `composedWith`, `pathConditionsAtCreation`, and `savedEffects` on `JoinedNormalAndAbrubtCompletions` were more to handle then I originally bargained for to fix my original test case. Instead I picked a simple fix for this specific case of `try`/`catch`. When we `AbstractValue.createJoinConditionForSelectedCompletions` we get a concrete `false` value. So I check if it is false and don’t execute the catch block if it is. The condition can never be concretely true. Otherwise we’d unconditionally catch an error in the block above.

Happy to take suggestions for a more general fix.
Pull Request resolved: #2507

Differential Revision: D9583244

Pulled By: calebmer

fbshipit-source-id: 7693efef5e967c90d5a4c54f10ef2c137f264ef8

src/evaluators/TryStatement.js

@@ -77,21 +77,23 @@ function joinTryBlockWithHandlers(
       invariant(blockRes instanceof JoinedAbruptCompletions || blockRes instanceof JoinedNormalAndAbruptCompletions);
       // put the handler under a guard that excludes normal paths from entering it.
       let joinCondition = AbstractValue.createJoinConditionForSelectedCompletions(selector, blockRes);
-      try {
-        let handlerEffects = Path.withCondition(joinCondition, () => {
-          invariant(blockRes instanceof Completion);
-          let joinedThrow = new ThrowCompletion(Join.joinValuesOfSelectedCompletions(selector, blockRes));
-          let handlerEval = () => env.evaluateCompletionDeref(handler, strictCode, joinedThrow);
-          return realm.evaluateForEffects(handlerEval, undefined, "joinTryBlockWithHandlers");
-        });
-        Completion.makeSelectedCompletionsInfeasible(selector, blockRes);
-        let emptyEffects = construct_empty_effects(realm, blockRes);
-        handlerEffects = Join.joinEffects(joinCondition, handlerEffects, emptyEffects);
-        realm.applyEffects(handlerEffects);
-        result = handlerEffects.result;
-      } catch (e) {
-        if (!(e instanceof InfeasiblePathError)) throw e;
-        // It turns out that the handler is not reachable after all so just do nothing and carry on
+      if (joinCondition.mightNotBeFalse()) {
+        try {
+          let handlerEffects = Path.withCondition(joinCondition, () => {
+            invariant(blockRes instanceof Completion);
+            let joinedThrow = new ThrowCompletion(Join.joinValuesOfSelectedCompletions(selector, blockRes));
+            let handlerEval = () => env.evaluateCompletionDeref(handler, strictCode, joinedThrow);
+            return realm.evaluateForEffects(handlerEval, undefined, "joinTryBlockWithHandlers");
+          });
+          Completion.makeSelectedCompletionsInfeasible(selector, blockRes);
+          let emptyEffects = construct_empty_effects(realm, blockRes);
+          handlerEffects = Join.joinEffects(joinCondition, handlerEffects, emptyEffects);
+          realm.applyEffects(handlerEffects);
+          result = handlerEffects.result;
+        } catch (e) {
+          if (!(e instanceof InfeasiblePathError)) throw e;
+          // It turns out that the handler is not reachable after all so just do nothing and carry on
+        }
       }
     }
   }

test/serializer/abstract/ThrowInDoubleCatch.js

@@ -0,0 +1,8 @@
+try {
+  try {
+    const b = __abstract("boolean", "false");
+    if (b) throw new Error("throw");
+  } catch (error) {}
+} catch (error) {
+  console.log(error.message);
+}