Summary:
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.
Pull Request resolved: https://github.com/facebook/prepack/pull/2257
Differential Revision: D8862827
Pulled By: sebmarkbage
fbshipit-source-id: 7e6e98f8b85b6fceaa5131136cc6163d94f5d289
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: https://github.com/facebook/prepack/pull/2263
Reviewed By: trueadm
Differential Revision: D8859210
Pulled By: hermanventer
fbshipit-source-id: 724dcde05927cc914f6f9517f14dc230b8b0ad2e
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: https://github.com/facebook/prepack/pull/2256
Differential Revision: D8850583
Pulled By: trueadm
fbshipit-source-id: 2d2aaec25c6a1ccd1ec0c08c5e7e2a71f78ac2d8
Summary:
I generated the following program (now added to test cases) and while I would expect its evaluation in Prepack to terminate reasonably quickly, I did not see the program terminate after five minutes of execution:
https://gist.github.com/calebmer/2bf1d84a5b7849fa732bce69811df10b
After this change that same program’s Prepack evaluation terminates in about two seconds. This change also saves about 2.8s on the evaluation of our internal bundle which is about 3% of the total time.
What was happening? Why did my program fail to terminate execution in five minutes? Why was the internal bundle relatively ok compared to my extreme case?
In my program, I would [expect the component `Mu`](https://gist.github.com/calebmer/2bf1d84a5b7849fa732bce69811df10b#file-program-js-L289-L291) to be inlined about 60 times. Which means there should only be about 60 calls to `ReactElementSet#add` for each of `Mu`’s `div`s. However, after some basic instrumentation I observed way over ten thousand visits to these React elements.
This pair of method calls happens a couple times in `ReactEquivalenceSet` and friends.
5f7256f17e/src/react/ReactEquivalenceSet.js (L233-L234)
Both `getEquivalentPropertyValue`…
5f7256f17e/src/react/ReactEquivalenceSet.js (L255)
…and `getValue`…
5f7256f17e/src/react/ReactEquivalenceSet.js (L104)
…end up calling `ReactElementSet#add` with the same React element. Then `ReactElementSet#add` ends up recursing into children. So the root element is added 2 times, so the root’s children are added 4 times each, so each child of those elements are added 8 times each, and so on. So if a leaf element is `n` deep in the tree it will be added `2^n` times. The most nested `Mu` child was 9 elements deep. `Mu` appeared 60 times at many levels of nesting.
I’m not entirely sure why my generated case was so much worse then our internal bundle. My two best guesses are: lots of repeated components or deeper nesting?
My fix was to move the `getValue` call into `getEquivalentPropertyValue`. This way if `getEquivalentPropertyValue` already finds an equivalent value we can short-circuit and not run `getValue` which will perform the same operation.
Pull Request resolved: https://github.com/facebook/prepack/pull/2243
Reviewed By: trueadm
Differential Revision: D8811463
Pulled By: calebmer
fbshipit-source-id: 4f30a75e96c6592456f5b21ee9c2ee3e40b56d81
Summary:
Release notes: none
The previous way we deal with side-effect callbacks was confusing and hard to understand. This PR now introduces a Set to store callbacks in and the old logic has been removed.
Pull Request resolved: https://github.com/facebook/prepack/pull/2254
Differential Revision: D8840154
Pulled By: trueadm
fbshipit-source-id: ce5b03f186414285d5ca6d2d0c2c385f3c56f981
Summary:
Release note: none
When joining a SimpleNormalCompletion with a PossiblyNormalCompletion, do not move the join condition to a leaf position.
Pull Request resolved: https://github.com/facebook/prepack/pull/2250
Differential Revision: D8838821
Pulled By: hermanventer
fbshipit-source-id: 6e102307af31e2ff2f3fbc7ec7f5ad516df5f7a5
Summary:
stop abstract loops from getting stuck in infinite loops when body contains a return, throw or break completion
Object.assign should no longer lose values when snapshotting in certain cases
Allow __optimize to work in conditional contexts
Move abstract Object.assign temporals into a helper function
new color scheme for the website
Add ability to rewrite generated code via global.__output
adds an optimization to Object.assign that attempts to merge calls together where possible
Added --expectedCounts parameter to test262-runner so that success can depend on the value of the time-out and the version of the test suite that is used.
Updated the test262 submodule to latest version
Reviewed By: cblappert
Differential Revision: D8837068
fbshipit-source-id: 218879046dbb0e307cbf3a13ba9a66d11e99a3f0
Summary:
Release notes: stop abstract loops from getting stuck in infinite loops when body contains a return, throw or break completion
Fixes https://github.com/facebook/prepack/issues/2053. This PR adds logic to "for loops" where they might get stuck in an infinite loop when there is no incrementor but there is a return, throw or break completion in the loop body. We check this via a new function `andfailIfContainsBreakOrReturnOrThrowCompletion` and only do this after `100` iterations of evaluating the loop body. I was thinking of making the iteration count configurable but wanted feedback on the approach first. I tried `1000` iterations, but it actually ended up taking about 2 minutes to evaluate the test case :/
Pull Request resolved: https://github.com/facebook/prepack/pull/2247
Differential Revision: D8827425
Pulled By: trueadm
fbshipit-source-id: d05f539e2c8a6dce15b7f23c1b76e89087437738
Summary:
Release notes: Object.assign should no longer lose values when snapshotting in certain cases
Fixes https://github.com/facebook/prepack/issues/2240. This PR does a few things:
- Improves the value of Object.assign when snapshots are involved. Previously, the `removeProperties` property passed to `getSnapshot` was being called in all cases, now it only does it when it makes sense – i.e. the next "source" is partial. I've explained this in comments and broken this logic out into its own function.
- Removed the duplicate Object.assign logic for `react/utils`, we no longer need this as the fallback route never gets triggered anymore and it's just dead code.
- Updated the dead code tests with assertions to ensure we check the $ variable matches the Object.assign and also updated a few tests that were affected by this PR (as the Object.assign variable changed).
Pull Request resolved: https://github.com/facebook/prepack/pull/2246
Differential Revision: D8827443
Pulled By: trueadm
fbshipit-source-id: ec600baf7d08916b6c62f0aeaa09d739fba4415b
Summary:
Release note: none
I'm trying to move from a world where we pass arounds effects and completions separately to a world where only completions are passed around. To get there, we need to get a one to one correspondence between completions and the effects that they complete. This is not currently the case and getting there seems to be quite a bit of work.
To make things more reviewable, I'm breaking out the first part of that. Hence, the todos in the code.
This bit only sets up Effects to always make itself be the value of the effects property of its completion (result). Because of existing cases where a single completion can complete several Effects objects, there is an exemption for such cases, along with todos.
Pull Request resolved: https://github.com/facebook/prepack/pull/2244
Differential Revision: D8825625
Pulled By: hermanventer
fbshipit-source-id: 7fb835d68e806ffd96845983f4d3db6837d1ad5a
Summary:
Release note: none
This just removes code that became redundant because of earlier PRs. To put it another way: ErasedAbruptCompletion is now gone!!!
Pull Request resolved: https://github.com/facebook/prepack/pull/2237
Differential Revision: D8811164
Pulled By: hermanventer
fbshipit-source-id: c7ca973f5ab6f3f7b5d4d78f70e2a3590cbe52e6
Summary:
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.
Pull Request resolved: https://github.com/facebook/prepack/pull/2219
Differential Revision: D8809146
Pulled By: sebmarkbage
fbshipit-source-id: 9fbfa87f5079b60f234ce4c1b2c745ac72a6b2af
Summary:
Release note: none
I'd like to get rid of ErasedAbruptCompletion and restore the invariant that any subclass of AbruptCompletion is strictly abrupt. To do that, I need to relax some invariants in PossiblyNormalCompletion.
This is also a first step towards fixing the code for composing possibly normal completions correctly.
I've pulled this out into a relatively small PR to make reviewing easier. Hence, the remaining TODO in the code.
Even this smallish change results in some regression because previously different code paths were followed that allowed simplification to succeed that will now fail. Fixing that is too ambitious for this PR and will have to wait.
Pull Request resolved: https://github.com/facebook/prepack/pull/2230
Differential Revision: D8808922
Pulled By: hermanventer
fbshipit-source-id: 4afce5e27bc30290b2b237b40c1e92f8c22bd100
Summary:
This will fix CI on master. I forgot it's possible to land a change internally without it passing the CI checks.
Pull Request resolved: https://github.com/facebook/prepack/pull/2242
Differential Revision: D8801741
Pulled By: gaearon
fbshipit-source-id: 454fcb29e023a502557b67e0ed12a304c4e5fabc
Summary:
Release note: none
Resolves#2238
This is a quick fix for the above issue. The underlying cause is that we do not construct completions with effects, but rely on incremental updates in special situations, such as the constructor for ForkedAbruptCompletions.
I'll try to fix the underlying cause in a subsequent PR.
Pull Request resolved: https://github.com/facebook/prepack/pull/2239
Differential Revision: D8795886
Pulled By: hermanventer
fbshipit-source-id: 677f2481ae3d6463a73f36a7e7106afbd92b068e
Summary:
Release notes: None
Found and fixed some minor issues.
This fixes#2177.
Pull Request resolved: https://github.com/facebook/prepack/pull/2231
Differential Revision: D8789870
Pulled By: NTillmann
fbshipit-source-id: 4ea3de2cbac08dbf6538af344eade1c45d9b9afd
Summary:
…local state
The logic was already there for the React Compiler's invocations of evaluatePure, this PR just generalizes it a little to be used for optmized functions as well.
Additionally, adds the ability to check for compiler info/warning logs in test-runner + updates test-error-handler.
See #1589
Pull Request resolved: https://github.com/facebook/prepack/pull/2175
Differential Revision: D8786744
Pulled By: cblappert
fbshipit-source-id: 110a4732dd6bd129b4d91047c3c9a24f5249a5e9
Summary:
Release Notes: None
Before `__optimize` wouldn't work when called in a conditional context, now if `__optimize` is called in some conditional context, we extract the FunctionValue from the AbstractValue in the `__optimizedFunctions` map.
Also fixes a bug where __optimize would silently noop when given something other than an `ECMAScriptSourceFunctionValue` causing two tests to fail with `TypeError`. Issue #2213 is setup to track this.
Pull Request resolved: https://github.com/facebook/prepack/pull/2214
Differential Revision: D8785643
Pulled By: cblappert
fbshipit-source-id: 3db992d693dd431aa8a2c5e6eb7ad0a1ecb6b79e
Summary:
This will fail CI if we forgot to run `yarn prettier` before committing.
We do the same in React repo. It prevents committing stale files that later cause unexpected changes.
Pull Request resolved: https://github.com/facebook/prepack/pull/2212
Differential Revision: D8784406
Pulled By: gaearon
fbshipit-source-id: ca948b8e088be8886c8ba865f280ba8d72750f69
Summary:
Release notes: none
As per Nikolai's comment https://github.com/facebook/prepack/pull/2206#discussion_r201142434, this PR moves the creation of the `Object.assign` abstract temporals into their own static abstract value creation method `AbstractValue.createTemporalObjectAssign`.
Pull Request resolved: https://github.com/facebook/prepack/pull/2232
Differential Revision: D8782639
Pulled By: trueadm
fbshipit-source-id: 9a41a66d5b7e41982de96684e627ef5e103cc638
Summary:
Release notes: None
It used to be a case that the Generator constructor simply
harvested whatever was currently set in the realm as the current
path condition.
This makes it explicit, and fixes some places where likely the wrong
path condition was picked up. At least one place caused an issue in
InstantRender that is now fixed (but I couldn't extract a small repro).
Leaving behind a number of `TODO #2222`s in the code to review once more.
Pull Request resolved: https://github.com/facebook/prepack/pull/2224
Differential Revision: D8779258
Pulled By: NTillmann
fbshipit-source-id: a0f3dea4b03eaa71b12943f813eb7fc440d29fca
Summary:
Release notes: Add ability to rewrite generated code via global.__output
When a special global variable __output is set to an object,
then all global generator entries are forgotten
and replaced by a series of assignments to reflecting the key-value
pairs of the __output object.
This is needed for instant render to emit single render functions only.
Pull Request resolved: https://github.com/facebook/prepack/pull/2218
Differential Revision: D8778269
Pulled By: NTillmann
fbshipit-source-id: 1dbc5ddd68aa5fd7845da8a7f551c8c7ba2b8919
Summary:
Release notes: None
I manually reviewed and annotated all exported function return types.
Still TODO: Same for class methods, and then we need some enforcement mechanism.
Pull Request resolved: https://github.com/facebook/prepack/pull/2227
Differential Revision: D8775279
Pulled By: NTillmann
fbshipit-source-id: 31aca088384657d7b398c758ab7eb42d3c4fa001
Summary:
Release notes: adds an optimization to Object.assign that attempts to merge calls together where possible
I frequently see `Objet.assign` calls litter our internal bundle. Many of them can actually be merged together to reduce bloat and runtime overhead (on that matter, `Object.assign` isn't a cheap call). We do this by adding a new `TemporalObjectAssignEntry` entry, that allows us to add some fine-tuning of Object.assign temporal entries we create.
The new `TemporalObjectAssignEntry` entry has an optimization that aims to merge entries by checking if linked nodes, specifically the `Object.assign` calls `to` field, to see if it can literally merge the arguments of many `Object.assign`s together. If a `to` is visited and can't be omitted, it doesn't try to apply this optimization. What we end up with, is a single `Object.assign` call and the others all get dead-code eliminated away because of the `mutatesOnly` logic from earlier.
Pull Request resolved: https://github.com/facebook/prepack/pull/2206
Reviewed By: NTillmann
Differential Revision: D8775391
Pulled By: trueadm
fbshipit-source-id: 41e5d6bb1d51fcfff66b2fe758bd51492ec472d9
Summary:
Release note: Added --expectedCounts parameter to test262-runner so that success can depend on the value of the time-out and the version of the test suite that is used.
Pull Request resolved: https://github.com/facebook/prepack/pull/2228
Differential Revision: D8775828
Pulled By: hermanventer
fbshipit-source-id: 284bdb3526467f634f41a151e3995719af751e49
Summary:
> I’m learning the Prepack/React Fusion codebase, so incorrect assumptions and limited a understanding of systems on my part are both very likely.
While I was experimenting with the React Compiler codebase I found the following invariant violation with the test case after that when using `create-element` as the `reactOutput`.
```
Invariant Violation: value must have been visited
```
```js
const React = require("react");
__evaluatePureFunction(() => {
function Foo(props) {
return <React.Fragment />;
}
__optimizeReactComponentTree(Foo);
module.exports = Foo;
});
```
[[Prepack REPL]](https://prepack.io/repl.html#MYewdgzgLgBASgUwIbFgXhgJwQRwK4CW2AFAETYpSkCUA3AFD0D6TCAbkgDZ5JQIAKebADE8YVAXDFi1GGgB8MAN70YMAGZiJ4GMJAhiAB0whDEWSrVrsUIWBgAeRJQB0wzEgDmAWwRhYAPTyDGoAvoxqLKZQBN4EAF4IzqgAwiDehuB+UAAq2AjEeiB0ETDeIAAmeJwILggAHpmYUBByuvoMoSVAA)
After digging in more, I discovered that this was the same issue which required some clowniness in the React Compiler tests to workaround.
e3f4262c9d/test/react/setupReactTests.js (L116-L119)
The problem with my example is that the React library was not being visited when a `React.Fragment` appeared even though the serializer needed the library to be visited to output code.
This diff makes sure we visit the React library when we see a `React.Fragment` and makes sure we don’t over visit the React library in `jsx` mode.
I also refactored this area of the code and added comments. Let me know if my refactors or comments don’t make sense.
Pull Request resolved: https://github.com/facebook/prepack/pull/2223
Reviewed By: trueadm
Differential Revision: D8774047
Pulled By: calebmer
fbshipit-source-id: 775a626b8a6bd33a5366e6dae6727350835b060e
Summary:
Release note: none
Resolves: #2207
When a PossiblyNormalCompletion has more than one normal path, it is not correct to push only the path leading to the nominally normal completion. Surprisingly, this has not caused all sorts of havoc so far, but clearly this needs fixing.
Interestingly, these changes did uncover some latent bugs, hence the additional changes here.
Testing this more thoroughly will be much easier once PNC composition is fixed. That will take a while, but this PR will help facilitate that.
Pull Request resolved: https://github.com/facebook/prepack/pull/2201
Differential Revision: D8754966
Pulled By: hermanventer
fbshipit-source-id: 220ddc80eaff69b8ee93f75c1943ee1a16adcdee
Summary:
Release note: Updated the test262 submodule to latest version
Also updated test262-runner.js to deal with time out errors showing up as diagnostics.
Filter out new tests that depend on errors being generated during the parsing phase. Since we depend on Babel for parsing, such issues are out of scope for us.
Pull Request resolved: https://github.com/facebook/prepack/pull/2216
Differential Revision: D8755005
Pulled By: hermanventer
fbshipit-source-id: 0c929904984d13efccbd3ad1ca125137ca275ef0
Summary:
Release notes: None
If concrete, it can only be an object.
Pull Request resolved: https://github.com/facebook/prepack/pull/2210
Differential Revision: D8752067
Pulled By: NTillmann
fbshipit-source-id: 207c3a6e055924290029e770528dc24df1023ffe
Summary:
I found this working on another PR but might as well fix it here first.
This wasn't previously enforced and now we are forced to deal with the whether something is a primitive or object as well as some other places we had bugs covered up by `any`.
Closes https://github.com/facebook/prepack/pull/2211
Differential Revision: D8751674
Pulled By: sebmarkbage
fbshipit-source-id: d1445530e53d88a08f3d2652dffd26bcf339d4f3
Summary:
Improved source location information
Only havoc abstract value args when really necessary
Debugger recognizes sourcemaps and opens the correct file/line when debugging in Nuclide.
Make React tests fast
Special case an expression simplification for Instant Render
Fix temporal assignments to intrinsics that happen inside non deterministic loops
Enhanced handling of Array.map applied to abstract arrays
Support dynamic invariants via __assume
We got a logo!
Add React Native mocks to Prepack
Reviewed By: simonhj
Differential Revision: D8739156
fbshipit-source-id: d535dfe0c5bcbee4aaed28f75d45cbedcc51344b
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
Summary:
Release notes: none
In an attempt to pull out some of the work from https://github.com/facebook/prepack/pull/2148 to make things easier to consume and review. This PR is one part, where we check the temporal alias values (if an object value has it) and see if we can dedupe the trees. Example below:
Closes https://github.com/facebook/prepack/pull/2193
Differential Revision: D8732542
Pulled By: trueadm
fbshipit-source-id: 352d0048acfef69b5a257c0fe280435fa7baaa7f
Summary:
Release notes: none
We were missing logic for handling abstract arrays with unknown numeric properties when we attempt to get a partial value on the array. I've added a regression test too.
Closes https://github.com/facebook/prepack/pull/2205
Differential Revision: D8732181
Pulled By: trueadm
fbshipit-source-id: 6002eac4b7d4d0b842407a3bb9ea9bcec19f486b
Summary:
Release notes: none
Make it easier to do local development testing of our internal bundle without having to hack the settings each time.
Closes https://github.com/facebook/prepack/pull/2202
Differential Revision: D8731922
Pulled By: trueadm
fbshipit-source-id: 71d92c3b90f448667c4a9ada1f1b0848ece5003b
Summary:
Release notes: adds React Native mocks to Prepack
This adds React Native mocks to Prepack and a few basic tests to demonstrate inlining of `View` and `Text`.
Closes https://github.com/facebook/prepack/pull/2096
Differential Revision: D8723932
Pulled By: NTillmann
fbshipit-source-id: 38bd265cd8935ebdf30266ec337378b4ea5b09d6
Summary:
Release notes: We got a logo!
Updating README to include logo.
Will create an issue to update website accordingly.
Closes https://github.com/facebook/prepack/pull/2189
Differential Revision: D8725906
Pulled By: NTillmann
fbshipit-source-id: 2f9b3b1d4afbd4fc42f137fcda9e4edac390d272
Summary:
Release notes: Support dynamic invariants via __assume
Resolves#2171. Implements an __assume primitive that injects dynamic invariants into the code.
Source:
```js
x = __abstract("number", "5");
__invariant(x==5, "x should be 5");
```
Generated:
```js
(function () {
var _$0 = this;
var _0 = 5;
_$0.x = _0;
var _1 = _0 == 5;
if (!_1) {
throw new Error("x should be 5");
}
}).call(this);
```
Closes https://github.com/facebook/prepack/pull/2195
Reviewed By: hermanventer
Differential Revision: D8725726
Pulled By: sb98052
fbshipit-source-id: 28db3272199cdb9ae669062344eb82f0ef619bd4
Summary:
Release notes: none
As pointed out by Herman in https://github.com/facebook/prepack/pull/2193#discussion_r199628160. We don't need `temporalAliasArgs`, so this PR removes it from the codebase and uses the existing way via the preludeGenerator.
This also adds a helper function to make it easier to relate derived abstract values to their temporal args. Furthermore, `derivedIds` now stores the `TemporalBuildNodeEntryArgs` rather than `Array<Value>`, as the follow up PRs for doing the React related logic need to access more than just the array of args, it also needs access to `isPure`, `skipInvariant` etc so it can properly clone derived abstract values.
Closes https://github.com/facebook/prepack/pull/2199
Differential Revision: D8724017
Pulled By: trueadm
fbshipit-source-id: 3ad72d3b230f1d2dc7f81c2a8d07dd33d0c7aa5c
