Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Commit

Permalink
Add evaluateWithPossibleThrowCompletion
Browse files Browse the repository at this point in the history
Summary:
I'm going to need to add a lot of these things that might throw so I wanted to start preparing the infrastructure for #1264. See that issue for more info on the approach.

The idea is that code inside this scope might emit temporal operations that might throw. In the future we can wrap these in a try/catch to extract a caught error and a flag. These can be used to generate a PossiblyNormalCompletion.

However, for now I just keep the existing strategy of erroring in this case while I add more callers of this helper.
Closes #1326

Differential Revision: D6735259

Pulled By: sebmarkbage

fbshipit-source-id: c16a4ed2d4eaf9120dc38d60a4922c7cab0ba59c
  • Loading branch information
sebmarkbage authored and facebook-github-bot committed Jan 17, 2018
1 parent e5097e9 commit 9081d3b
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 31 deletions.
17 changes: 5 additions & 12 deletions src/evaluators/CallExpression.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { AbruptCompletion, PossiblyNormalCompletion } from "../completions.js";
import type { Realm } from "../realm.js";
import type { LexicalEnvironment } from "../environment.js";
import { EnvironmentRecord } from "../environment.js";
import { TypesDomain, ValuesDomain } from "../domains/index.js";
import { Value } from "../values/index.js";
import { AbstractValue, BooleanValue, ConcreteValue, FunctionValue } from "../values/index.js";
import { Reference } from "../environment.js";
Expand Down Expand Up @@ -139,8 +140,7 @@ function EvaluateCall(
if (!Value.isTypeCompatibleWith(func.getType(), FunctionValue)) {
if (!realm.isInPureScope()) {
// If this is not a function, this call might throw which can change the state of the program.
// If this is called from a pure function it is safe, unless in a try/catch which has a
// separate error below.
// If this is called from a pure function we handle it using evaluateWithPossiblyAbruptCompletion.
let error = new CompilerDiagnostic("might not be a function", loc, "PP0005", "RecoverableError");
if (realm.handleError(error) === "Fail") throw new FatalError();
}
Expand All @@ -149,16 +149,9 @@ function EvaluateCall(
} else {
// Assume that it is a safe function. TODO #705: really?
}
if (realm.isInPureTryStatement) {
// TODO(1264): We should be able to preserve error handling on abstract throw
// but currently we just issue a recoverable error instead.
let diag = new CompilerDiagnostic(
"Possibly throwing function call inside try/catch",
loc,
"PP0021",
"RecoverableError"
);
if (realm.handleError(diag) !== "Recover") throw new FatalError();
if (realm.isInPureScope()) {
// In pure functions we allow abstract functions to throw, which this might.
return realm.evaluateWithPossibleThrowCompletion(generateRuntimeCall, TypesDomain.topVal, ValuesDomain.topVal);
}
return generateRuntimeCall();
}
Expand Down
28 changes: 28 additions & 0 deletions src/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
UndefinedValue,
Value,
} from "./values/index.js";
import type { TypesDomain, ValuesDomain } from "./domains/index.js";
import { LexicalEnvironment, Reference, GlobalEnvironmentRecord, DeclarativeEnvironmentRecord } from "./environment.js";
import type { Binding } from "./environment.js";
import { cloneDescriptor, Construct } from "./methods/index.js";
Expand Down Expand Up @@ -427,6 +428,33 @@ export class Realm {
return !!this.createdObjectsTrackedForLeaks;
}

// Evaluate some code that might generate temporal values knowing that it might end in an abrupt
// completion. We only need to support ThrowCompletion for now but this can be expanded to support other
// abrupt completions.
evaluateWithPossibleThrowCompletion(f: () => Value, thrownTypes: TypesDomain, thrownValues: ValuesDomain): Value {
// The cases when we need this are only when we might invoke unknown code such as abstract
// funtions, getters, custom coercion etc. It is possible we can use this in other cases
// where something might throw a built-in error but can never issue arbitrary code such as
// calling something that might not be a function. For now we only use it in pure functions.
invariant(this.isInPureScope(), "only abstract abrupt completion in pure functions");

// TODO(1264): We should create a new generator for this scope and wrap it in a try/catch.
// We could use the outcome of that as the join condition for a PossiblyNormalCompletion.
// We should then compose that with the saved completion and move on to the normal route.
// Currently we just issue a recoverable error instead if this might matter.
let value = f();
if (this.isInPureTryStatement) {
let diag = new CompilerDiagnostic(
"Possible throw inside try/catch is not yet supported",
this.currentLocation,
"PP0021",
"RecoverableError"
);
if (this.handleError(diag) !== "Recover") throw new FatalError();
}
return value;
}

// Evaluate the given ast in a sandbox and return the evaluation results
// in the form of a completion, a code generator, a map of changed variable
// bindings and a map of changed property bindings.
Expand Down
31 changes: 14 additions & 17 deletions src/values/AbstractObjectValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

/* @flow */

import { CompilerDiagnostic, FatalError } from "../errors.js";
import { FatalError } from "../errors.js";
import type { Realm } from "../realm.js";
import type { Descriptor, PropertyKeyValue } from "../types.js";
import { AbstractValue, ArrayValue, ObjectValue, StringValue, Value } from "./index.js";
Expand Down Expand Up @@ -313,22 +313,7 @@ export default class AbstractObjectValue extends AbstractValue {
$Get(P: PropertyKeyValue, Receiver: Value): Value {
if (P instanceof StringValue) P = P.value;
if (this.values.isTop()) {
if ((this.isSimpleObject() && this.isIntrinsic()) || this.$Realm.isInPureScope()) {
if (!this.isSimpleObject()) {
if (this.$Realm.isInPureTryStatement) {
// TODO(1264): We should be able to preserve error handling on abstract throw
// but currently we just issue a recoverable error instead.
let diag = new CompilerDiagnostic(
"Possibly throwing getter inside try/catch",
this.$Realm.currentLocation,
"PP0021",
"RecoverableError"
);
if (this.$Realm.handleError(diag) !== "Recover") throw new FatalError();
}
// This object might have leaked to a getter.
Leak.leakValue(this.$Realm, this);
}
let generateAbstractGet = () => {
let type = Value;
if (P === "length" && Value.isTypeCompatibleWith(this.getType(), ArrayValue)) type = NumberValue;
let object = this.kind === "sentinel ToObject" ? this.args[0] : this;
Expand All @@ -344,6 +329,18 @@ export default class AbstractObjectValue extends AbstractValue {
skipInvariant: true,
}
);
};
if (this.isSimpleObject() && this.isIntrinsic()) {
return generateAbstractGet();
} else if (this.$Realm.isInPureScope()) {
// This object might have leaked to a getter.
Leak.leakValue(this.$Realm, this);
// The getter might throw anything.
return this.$Realm.evaluateWithPossibleThrowCompletion(
generateAbstractGet,
TypesDomain.topVal,
ValuesDomain.topVal
);
}
AbstractValue.reportIntrospectionError(this, P);
throw new FatalError();
Expand Down
2 changes: 1 addition & 1 deletion test/error-handler/try-and-access-abstract-property.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// additional functions
// abstract effects
// recover-from-errors
// expected errors: [{location: {"start":{"line":14,"column":11},"end":{"line":14,"column":15},"identifierName":"obj1","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possibly throwing getter inside try/catch"},{location: {"start":{"line":22,"column":11},"end":{"line":22,"column":15},"identifierName":"obj2","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possibly throwing getter inside try/catch"}]
// expected errors: [{location: {"start":{"line":14,"column":11},"end":{"line":14,"column":15},"identifierName":"obj1","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possible throw inside try/catch is not yet supported"},{location: {"start":{"line":22,"column":11},"end":{"line":22,"column":15},"identifierName":"obj2","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possible throw inside try/catch is not yet supported"}]

let obj1 = global.__abstract ? __abstract('object', '({get foo() { return "bar"; }})') : {get foo() { return "bar"; }};
let obj2 = global.__abstract ? __abstract('object', '({foo:{bar:"baz"}})') : {foo:{bar:"baz"}};
Expand Down
2 changes: 1 addition & 1 deletion test/error-handler/try-and-call-abstract-function.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// additional functions
// abstract effects
// expected errors: [{location: {"start":{"line":26,"column":11},"end":{"line":26,"column":21},"identifierName":"abstractFn","source":"test/error-handler/try-and-call-abstract-function.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possibly throwing function call inside try/catch"}]
// expected errors: [{location: {"start":{"line":26,"column":11},"end":{"line":26,"column":21},"identifierName":"abstractFn","source":"test/error-handler/try-and-call-abstract-function.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possible throw inside try/catch is not yet supported"}]

let abstractFn = global.__abstract ? __abstract('function', '(function() { return true; })') : function() { return true; };

Expand Down

0 comments on commit 9081d3b

Please sign in to comment.