Fix FatalErrors that are actually Recoverable (#2371)

Summary:
Release Notes: None

When optimizing a function fails, we can usually continue to emit a correct program while ignoring that call to `__optimize`, thus most of these errors should be `Warning`s and not `FatalError`s.

A few times (like when we have multiple sets of `Effects` for one optimized function), we can sensibly continue Prepacking code, but our output may be incorrect. In these cases, we can issue a `RecoverableError` instead of a `FatalError`
Pull Request resolved: https://github.com/facebook/prepack/pull/2371

Differential Revision: D9186240

Pulled By: cblappert

fbshipit-source-id: c30fc63fb713a3bf504cc1ec4f02410a8566cc42

src/intrinsics/prepack/global.js

@@ -118,6 +118,7 @@ export default function(realm: Realm): void {
   // NB: If we interpret one of these calls in an evaluateForEffects context
   //     that is not subsequently applied, the function will not be registered
   //     (because prepack won't have a correct value for the FunctionValue itself)
+  // If we encounter an invalid input, we will emit a warning and not optimize the function
   global.$DefineOwnProperty("__optimize", {
     value: new NativeFunctionValue(realm, "global.__optimize", "__optimize", 1, (context, [value, argModelString]) => {
       let argModel;
@@ -134,9 +135,10 @@ export default function(realm: Realm): void {
               "__optimize called twice with different argModelStrings",
               realm.currentLocation,
               "PP1008",
-              "FatalError"
+              "Warning"
             );
             if (realm.handleError(argModelError) !== "Recover") throw new FatalError();
+            else return realm.intrinsics.undefined;
           }
         }
         realm.optimizedFunctions.set(value, argModel);
@@ -150,10 +152,11 @@ export default function(realm: Realm): void {
             `Optimized Function Value ${location} is an not a function or react element`,
             realm.currentLocation,
             "PP0033",
-            "FatalError"
+            "Warning"
           )
         );
         if (result !== "Recover") throw new FatalError();
+        else return realm.intrinsics.undefined;
       }
       return value;
     }),

src/serializer/functions.js

@@ -97,15 +97,16 @@ export class Functions {
     }
 
     let location = optionalStringOfLocation(value.expressionLocation);
-    realm.handleError(
+    let result = realm.handleError(
       new CompilerDiagnostic(
         `Optimized Function Value ${location} is an not a function or react element`,
         realm.currentLocation,
         "PP0033",
-        "FatalError"
+        "Warning"
       )
     );
-    throw new FatalError("Optimized Function Values must be functions or react elements");
+    // Here we can recover by ignoring the __optimize call and emit correct code
+    if (result !== "Recover") throw new FatalError("Optimized Function Values must be functions or react elements");
   }
 
   _generateInitialAdditionalFunctions(globalKey: string): Array<AdditionalFunctionEntry> {
@@ -253,11 +254,13 @@ export class Functions {
           "Trying to optimize a function with two parent optimized functions, which is not currently allowed.",
           functionValue.expressionLocation,
           "PP1009",
-          "FatalError"
+          "RecoverableError"
         );
+        // we can recover by assuming one set of effects to show further diagnostics
         if (realm.handleError(error) !== "Recover") throw new FatalError();
+      } else {
+        this.writeEffects.set(functionValue, additionalFunctionEffects);
       }
-      this.writeEffects.set(functionValue, additionalFunctionEffects);
 
       // Conceptually this will ensure that the nested additional function is defined
       // although for later cases, we'll apply the effects of the parents only.
@@ -292,13 +295,13 @@ export class Functions {
       invariant(e1 !== undefined);
       if (e1.result instanceof Completion && !e1.result instanceof PossiblyNormalCompletion) {
         let error = new CompilerDiagnostic(
-          `Additional function ${fun1Name} may terminate abruptly`,
+          `Additional function ${fun1Name} will terminate abruptly`,
           e1.result.location,
           "PP1002",
-          "FatalError"
+          "RecoverableError"
         );
-        this.realm.handleError(error);
-        throw new FatalError();
+        // We generate correct code in this case, but the user probably doesn't want us to emit an unconditional throw
+        if (this.realm.handleError(error) !== "Recover") throw new FatalError();
       }
       for (let fun2 of additionalFunctions) {
         if (fun1 === fun2) continue;
@@ -327,8 +330,8 @@ export class Functions {
       }
     }
     if (conflicts.size > 0) {
-      for (let diagnostic of conflicts.values()) this.realm.handleError(diagnostic);
-      throw new FatalError();
+      for (let diagnostic of conflicts.values())
+        if (this.realm.handleError(diagnostic) !== "Recover") throw new FatalError();
     }
   }
 

test/serializer/optimized-functions/NestedOptimizeSameFunction.js

@@ -1,4 +1,4 @@
-// expected FatalError: PP1009
+// expected RecoverableError: PP1009
 // does not contain: = 5
 
 function fn() {