This repository has been archived by the owner on Feb 12, 2022. It is now read-only.
Impure abstract getters on prototype chain #2257
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
trueadm
approved these changes
Jul 14, 2018
Summary: Release notes: upgrades Prepack to use Babel 7.0.0-beta.53 This is a big PR that updates all of Prepack to Babel 7. Babylon is now `babel/parser` and pretty much all of the the previous Babel packages are now located in scoped packages. I had to make a bunch of changes around Jest/Flow/Webpack to get this all working. The build times of building Prepack itself seem considerably faster (easily twice as fast locally). I followed most of the Babel 6 -> 7 upgrade guide from the Babel site in terms of changing nodes and type definitions to match the new ones. Pull Request resolved: facebookarchive#2256 Differential Revision: D8850583 Pulled By: trueadm fbshipit-source-id: 2d2aaec25c6a1ccd1ec0c08c5e7e2a71f78ac2d8
Summary: Release notes: none Follow up to facebookarchive#2256. This re-enables Babel traverse cache clearing, but with the Babel 7 API. Pull Request resolved: facebookarchive#2260 Differential Revision: D8850784 Pulled By: trueadm fbshipit-source-id: 9d61feff152400262a050d72b9ee3567ddcf11e2
hermanventer
approved these changes
Jul 15, 2018
Summary: Release note: fix test262 to fail CircleCI test if not enough tests pass The check for the number of tests that pass returned 1 to its caller, who just ignored it and then returned 0. Instead of that, now just call process.exit(1) when the check fails. Also updated the expected number of ES6 tests that pass. It appears that updating Babel had a positive effect on those. Possibly it also causes one more ES5 test to fail when running locally (but not on Circle). That might be because of an ES5 test that now times out and an ES6 test that now does not. I have not investigated this as it seems of little importance right now. Pull Request resolved: facebookarchive#2263 Reviewed By: trueadm Differential Revision: D8859210 Pulled By: hermanventer fbshipit-source-id: 724dcde05927cc914f6f9517f14dc230b8b0ad2e
This fixes an issue where get on abstract top val didn't consider Receiver. This also illustrates two common patterns of getters that are not pure. One is reading a mutable value known by Prepack and one is lazily initializing. I believe we'll want to continue supporting these patterns.
There was a problem hiding this comment.
@sebmarkbage is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue where get on abstract top val didn't consider Receiver.
This also illustrates two common patterns of getters that are not pure. One is reading a mutable property known by Prepack and one is lazily initializing a shared mutable property.
I believe we'll want to continue supporting these patterns since they almost always work anyway. Note that supporting these don't add much negative consequence. We can still eliminate unused getters. Most of the time these patterns doesn't happen where the prototype is unknown or havoced.
Usually the getter is abstract because the Receiver itself is unknown or havoced (not its prototype). So havocing even more doesn't have any further negative downside.