From 3e8c638071ca0bd429b4a45ebf464276dacd85c4 Mon Sep 17 00:00:00 2001 From: Christopher Blappert Date: Tue, 7 Aug 2018 16:48:05 -0700 Subject: [PATCH] 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 | 7 ++++-- src/serializer/functions.js | 25 +++++++++++-------- .../NestedOptimizeSameFunction.js | 2 +- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/intrinsics/prepack/global.js b/src/intrinsics/prepack/global.js index 63bea17bb..92b75c39b 100644 --- a/src/intrinsics/prepack/global.js +++ b/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; }), diff --git a/src/serializer/functions.js b/src/serializer/functions.js index 5288ddce7..7855167b5 100644 --- a/src/serializer/functions.js +++ b/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 { @@ -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(); } } diff --git a/test/serializer/optimized-functions/NestedOptimizeSameFunction.js b/test/serializer/optimized-functions/NestedOptimizeSameFunction.js index a1a9679a3..199a53122 100644 --- a/test/serializer/optimized-functions/NestedOptimizeSameFunction.js +++ b/test/serializer/optimized-functions/NestedOptimizeSameFunction.js @@ -1,4 +1,4 @@ -// expected FatalError: PP1009 +// expected RecoverableError: PP1009 // does not contain: = 5 function fn() {