Narrow type of PropertyBinding.object and eliminate unnecessary checks (#2208)

Summary:
Release notes: None

This came about as an offshoot of #2185. PropertyBinding.object cannot be of type AbstractObjectValue. This change narrows its type to ObjectValue, and eliminates checks that become unnecessary.

Outcome of a manual search via Grasp: PropertyBinding.object is assigned at two points - in methods/properties.js and react/util.js. There are also invariants at two places to guard this condition (object instanceof ObjectValue.) yarn flow does not report errors.
Closes https://github.com/facebook/prepack/pull/2208

Differential Revision: D8738889

Pulled By: sb98052

fbshipit-source-id: 52807e28a699b95934b8d53f0c81ccc6b06a79bb
This commit is contained in:
Sapan Bhatia 2018-07-05 13:57:51 -07:00 committed by Facebook Github Bot
parent 3cd3dd8431
commit 92c4db7483
5 changed files with 13 additions and 19 deletions

View File

@ -32,7 +32,7 @@ import { cloneDescriptor, equalDescriptors, IsDataDescriptor, StrictEqualityComp
import { construct_empty_effects } from "../realm.js";
import { Path } from "../singletons.js";
import { Generator } from "../utils/generator.js";
import { AbstractValue, ConcreteValue, EmptyValue, ObjectValue, Value } from "../values/index.js";
import { AbstractValue, ConcreteValue, EmptyValue, Value } from "../values/index.js";
import invariant from "../invariant.js";
@ -809,7 +809,7 @@ export class JoinImplementation {
let join = (b: PropertyBinding, d1: void | Descriptor, d2: void | Descriptor) => {
// If the PropertyBinding object has been freshly allocated do not join
if (d1 === undefined) {
if (b.object instanceof ObjectValue && c2.has(b.object)) return d2; // no join
if (c2.has(b.object)) return d2; // no join
if (b.descriptor !== undefined && m1.has(b)) {
// property was deleted
d1 = cloneDescriptor(b.descriptor);
@ -821,7 +821,7 @@ export class JoinImplementation {
}
}
if (d2 === undefined) {
if (b.object instanceof ObjectValue && c1.has(b.object)) return d1; // no join
if (c1.has(b.object)) return d1; // no join
if (b.descriptor !== undefined && m2.has(b)) {
// property was deleted
d2 = cloneDescriptor(b.descriptor);

View File

@ -19,7 +19,7 @@ import { AbruptCompletion, PossiblyNormalCompletion, SimpleNormalCompletion } fr
import { Reference } from "../environment.js";
import { cloneDescriptor, equalDescriptors, IsDataDescriptor, StrictEqualityComparison } from "../methods/index.js";
import { Generator } from "../utils/generator.js";
import { AbstractValue, ArrayValue, EmptyValue, ObjectValue, Value } from "../values/index.js";
import { AbstractValue, ArrayValue, EmptyValue, Value } from "../values/index.js";
import invariant from "../invariant.js";
import * as t from "babel-types";
@ -218,7 +218,7 @@ export class WidenImplementation {
if (d1 === undefined && d2 === undefined) return undefined;
// If the PropertyBinding object has been freshly allocated do not widen (that happens in AbstractObjectValue)
if (d1 === undefined) {
if (b.object instanceof ObjectValue && c2.has(b.object)) return d2; // no widen
if (c2.has(b.object)) return d2; // no widen
if (b.descriptor !== undefined && m1.has(b)) {
// property was present in (n-1)th iteration and deleted in nth iteration
d1 = cloneDescriptor(b.descriptor);
@ -235,7 +235,7 @@ export class WidenImplementation {
}
}
if (d2 === undefined) {
if (b.object instanceof ObjectValue && c1.has(b.object)) return d1; // no widen
if (c1.has(b.object)) return d1; // no widen
if (m2.has(b)) {
// property was present in nth iteration and deleted in (n+1)th iteration
d2 = cloneDescriptor(d1);
@ -406,13 +406,13 @@ export class WidenImplementation {
if (val1 === undefined) continue; // deleted
let val2 = m2.get(key1);
if (val2 === undefined) continue; // A key that disappears has been widened away into the unknown key
if (key1.object instanceof ObjectValue && c1.has(key1.object)) {
if (c1.has(key1.object)) {
continue;
}
if (!containsPropertyBinding(val1, val2)) return false;
}
for (const key2 of m2.keys()) {
if (key2.object instanceof ObjectValue && c2.has(key2.object)) {
if (c2.has(key2.object)) {
continue;
}
if (!m1.has(key2)) return false;

View File

@ -1051,7 +1051,7 @@ export class Realm {
) {
if (modifiedProperties === undefined) return;
modifiedProperties.forEach((desc, propertyBinding, m) => {
if (propertyBinding.object instanceof ObjectValue && newlyCreatedObjects.has(propertyBinding.object)) {
if (newlyCreatedObjects.has(propertyBinding.object)) {
propertyBinding.descriptor = desc;
}
});
@ -1108,10 +1108,7 @@ export class Realm {
let tvalFor: Map<any, AbstractValue> = new Map();
pbindings.forEach((val, key, map) => {
if (
key.object instanceof ObjectValue &&
(newlyCreatedObjects.has(key.object) || key.object.refuseSerialization)
) {
if (newlyCreatedObjects.has(key.object) || key.object.refuseSerialization) {
return;
}
let value = val && val.value;
@ -1138,10 +1135,7 @@ export class Realm {
}
});
pbindings.forEach((val, key, map) => {
if (
key.object instanceof ObjectValue &&
(newlyCreatedObjects.has(key.object) || key.object.refuseSerialization)
) {
if (newlyCreatedObjects.has(key.object) || key.object.refuseSerialization) {
return;
}
let path = key.pathNode;

View File

@ -156,7 +156,7 @@ export type FunctionBodyAstNode = {
export type PropertyBinding = {
descriptor?: Descriptor,
object: ObjectValue | AbstractObjectValue,
object: ObjectValue,
key: void | string | SymbolValue | AbstractValue, // where an abstract value must be of type String or Number or Symbol
// contains a build node that produces a member expression that resolves to this property binding (location)
pathNode?: AbstractValue,

View File

@ -448,7 +448,7 @@ export class Generator {
for (let propertyBinding of modifiedProperties.keys()) {
let object = propertyBinding.object;
if (object instanceof ObjectValue && createdObjects.has(object)) continue; // Created Object's binding
if (createdObjects.has(object)) continue; // Created Object's binding
if (object.refuseSerialization) continue; // modification to internal state
// modifications to intrinsic objects are tracked in the generator
if (object.isIntrinsic()) continue;