facebookarchive/prepack
1
1
Fork 0
mirror of https://github.com/facebookarchive/prepack.git synced 2024-09-11 14:46:37 +03:00
Code Issues Projects Releases Wiki Activity

Abstract empty values should fail RequireObjectCoercible (#2515)

Browse Source 
Summary:
This fixes a bug it took me a while to track down. Unfortunately creating a repro was really difficult. I was observing the following invariant in `Reference` being triggered.

fddfc6f0c5/src/environment.js (L1425-L1431)

Digging in, the bad base was a deep conditional `AbstractValue` where all the leaf nodes were `EmptyValue` and `__bottomValue`. (Constructed from a number of `try`/`catch`/`throw`s.) This `AbstractValue` _should_ have simplified to just `EmptyValue` then `RequireObjectCoercible()` should have thrown a runtime `TypeError` after seeing the `EmptyValue`. This is what I observed in local tests when trying to reproduce this issue.

After looking at the error in the large React Native bundle I’m working in I discovered that the simplifier was throwing an error since the simplification count was exceeded. This is fine and normal. The issue was that `RequireObjectCoercible()` does not handle complex empty `AbstractValue`s well. If failing simplification is a normal part of Prepack operation we should make sure methods like `RequireObjectCoercible()` can handle complex `AbstractValue`s as well as simple ones.

This PR does that.
Pull Request resolved: https://github.com/facebook/prepack/pull/2515

Reviewed By: trueadm

Differential Revision: D9602146

Pulled By: calebmer

fbshipit-source-id: 359c74c646eeb619ba2e78b50eafe10b2e2fb133
This commit is contained in:
Caleb Meredith 2018-08-31 02:58:23 -07:00 committed by Facebook Github Bot
parent b3bcd73102
commit 8a9457d844
2 changed files with 6 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/methods/abstract.js
View File

@ -74,6 +74,9 @@ export function RequireObjectCoercible(
arg: Value,
argLoc?: ?BabelNodeSourceLocation
): AbstractValue | ObjectValue | BooleanValue | StringValue | SymbolValue | NumberValue {
if (!arg.mightNotBeNull() || !arg.mightNotBeUndefined()) {
throw realm.createErrorThrowCompletion(realm.intrinsics.TypeError, "null or undefined");
}
if (arg instanceof AbstractValue && (arg.mightBeNull() || arg.mightBeUndefined())) {
if (realm.isInPureScope()) {
// In a pure function it is ok to throw if this happens to be null or undefined.
@ -91,11 +94,7 @@ export function RequireObjectCoercible(
}
arg.throwIfNotConcrete();
}
if (arg instanceof NullValue || arg instanceof UndefinedValue) {
throw realm.createErrorThrowCompletion(realm.intrinsics.TypeError, "null or undefined");
} else {
return (arg: any);
}
return (arg: any);
}
export function HasSameType(x: ConcreteValue, y: ConcreteValue): boolean {

2
src/values/AbstractValue.js
View File

@ -32,6 +32,7 @@ import {
StringValue,
SymbolValue,
UndefinedValue,
EmptyValue,
Value,
} from "./index.js";
import { hashString, hashBinary, hashCall, hashTernary, hashUnary } from "../methods/index.js";
@ -507,6 +508,7 @@ export default class AbstractValue extends Value {
mightNotBeUndefined(): boolean {
let valueType = this.getType();
if (valueType === UndefinedValue) return false;
if (valueType === EmptyValue) return false;
if (valueType !== PrimitiveValue && valueType !== Value) return true;
if (this.kind === "abstractConcreteUnion") {
for (let arg of this.args) if (arg.mightNotBeUndefined()) return true;