-
Notifications
You must be signed in to change notification settings - Fork 425
Special case conditionals for $Set operations on abstract object #2219
Special case conditionals for $Set operations on abstract object #2219
Conversation
This lets the simplifier do it work and allows intermediate objects to be dead code eliminated. If this looks good, I'll do $SetPartial and $Delete too.
src/values/AbstractObjectValue.js
Outdated
let d1 = ob1.$GetOwnProperty(P); | ||
let d2 = ob2.$GetOwnProperty(P); | ||
let oldVal1 = | ||
d1 === undefined ? this.$Realm.intrinsics.undefined : IsDataDescriptor(this.$Realm, d1) ? d1.value : undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not right. Should be empty, not undefined. I'll add a test.
@@ -0,0 +1,13 @@ | |||
// does not contain:unnecessaryIndirection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what is different with this diff. The intermediate objects are now eliminated.
invariant(ob1 instanceof ObjectValue || ob1 instanceof AbstractObjectValue); | ||
invariant(ob2 instanceof ObjectValue || ob2 instanceof AbstractObjectValue); | ||
let d1 = ob1.$GetOwnProperty(P); | ||
let d2 = ob2.$GetOwnProperty(P); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found an existing bug. These don't consider setters on the prototype chain. #2226
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sebmarkbage is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This lets the simplifier do it work and allows intermediate objects to be dead code eliminated.
If this looks good, I'll do $SetPartial and $Delete too.