Summary:
Release notes: opt out of optimizing a React component in a tree with `__doNotRender`
This PR makes it possible to opt out of a single component in a React component tree by detecting a `__doNotRender` flag. For example:
```js
function MyComponent(props) {
...
}
MyComponent.__doNotRender = true;
class MyComponent extends React.Component {
render() {
...
}
}
MyComponent.__doNotRender = true;
```
Closes https://github.com/facebook/prepack/pull/2004
Differential Revision: D8123009
Pulled By: trueadm
fbshipit-source-id: d5fc54aea2ea1253e260e41c1fb7d4f2d4717ffd
Summary:
Release notes: none
This fixes an issue that we were experiencing, that turned out to be the correct fix after looking at https://github.com/facebook/prepack/pull/1997. We shouldn't mutate arrays in the reconciliation process, instead we should create new ones when we have new values. Also, we should ensure that we don't mutate ReactElements during reconciliation too, instead we should just create new ones. Mutating existing objects and values breaks down completely when we enter nested effects that need to re-evaluate the same original values, so this fixes a bunch of internal issues we were seeing on our internal FB bundle.
Closes https://github.com/facebook/prepack/pull/1999
Differential Revision: D8105499
Pulled By: trueadm
fbshipit-source-id: 4e6ceb8b02c886c42156a8903c347765f101c840
Summary:
This line that refers to a missing variable slipped though a previous commit.
Closes https://github.com/facebook/prepack/pull/1988
Reviewed By: trueadm
Differential Revision: D8105162
Pulled By: sb98052
fbshipit-source-id: 6e35b882f0af94c2acd8096f955bd9ab7275ed77
Summary:
Release notes: all serializer tests now get linted via ESlint for correct output
This PR is pretty gigantic, but almost 99% of it is changing all the serializer tests so they pass the linting rules – which means that there must be no undefined variables or if they are, they are explicitly accesses via `global`. Furthermore, the linting also checks to ensure that all variables are defined before they are used.
Also I added a regression test `conditions3` that further validates some complex cases I was seeing fail in other cases. The ESlint config is also in its own module so it can be shared between `debug-fb-www`.
Closes https://github.com/facebook/prepack/pull/1998
Differential Revision: D8099445
Pulled By: trueadm
fbshipit-source-id: 4a03c57beb51e394bc7b334728090b9fc515eca8
Summary:
Release notes: all globals emitted reference the global identifier
This is a PR to replace https://github.com/facebook/prepack/pull/1894 that fixes#1890. I found a much simpler fix was to ensure we always treat emitted globals as safe for strict mode regardless. I had to change a test that went against this assumption, but we end up with code that better conforms to strict mode without large changes under the hood to Prepack.
Closes https://github.com/facebook/prepack/pull/1911
Differential Revision: D8095772
Pulled By: trueadm
fbshipit-source-id: d4ed65ba8d52cdb0c8f2171d3f3c02bdd265c32d
Summary:
Release note: none
When forking the normal branch of a PossiblyNormalCompletion into yet another PossiblyNormalCompletion, push the current effects onto a stack of effects, rather than just continuing to use the currently active effects. Not doing so, causes mismatched undo operations.
The modified test case would fail and print 2 for y, without this change.
Closes https://github.com/facebook/prepack/pull/1993
Differential Revision: D8087175
Pulled By: hermanventer
fbshipit-source-id: 6b857b9885aa79a8e90b90e365a2b9688a34152d
Summary:
Release notes: Support for --delayInitializations for optimized functions
Supporting this removes a TODO and is in response to #1973 (although more of a work-around than the actual fix).
Added tests.
Closes https://github.com/facebook/prepack/pull/1994
Differential Revision: D8087001
Pulled By: NTillmann
fbshipit-source-id: 572f41ffc024bef7954ebd88068f350fe08a84dc
Summary:
Release Notes: Fix abstract values not being coerced into strings for the Error constructor
Fixes#1827 NTillmann
The exact example from the issue now prepacks to
```javascript
(function () {
var _0 = function () {
return 42;
};
inspect = _0;
})();
```
I'm new here, so if this approach is completely wrong, please do notify me and guide me the way to a better one ;)
Closes https://github.com/facebook/prepack/pull/1960
Differential Revision: D8043199
Pulled By: NTillmann
fbshipit-source-id: 9c67fc2ab882398a20e6313b2894b5357b5ef038
Summary:
Release notes: adds experimental `react-dom/server` ahead-of-time rendering to string
This PR was part of a small 1-2 day hackathton to see the applicability of creating a server-side renderer based almost entirely to be a 1:1 of `ReactDOMServer` from `react-dom/server` package. This is by no means a full, complete server renderer but is the foundations for us to do further work on this path in the future. Currently, it consists of a single Jest test, to ensure the output of the Hacker News benchmark matches that of the current ReactDOMServer `renderToString` output.
The performance results look very promising (there are some `console.time` lines you can comment out in the test to see the performance for yourself). This implantation essentially compiles away `react` and `react-dom/server` and instead injects runtime helper functions that do things that cannot be statically determined ahead of time.
Lots of features are missing behind `invariant` TODOs. This was done intentionally.
Closes https://github.com/facebook/prepack/pull/1940
Differential Revision: D8075964
Pulled By: trueadm
fbshipit-source-id: 33b3c7ba26b41871ccd15ad8bde4ad257009fed6
Summary:
Release note: none
When debugging another issue, I noticed that member expressions do explicit conversions of abstract base objects to object and that call expressions obtain a temporal value for the function but then go ahead without it, leaving a useless assignment in the timeline.
Closes https://github.com/facebook/prepack/pull/1983
Reviewed By: trueadm
Differential Revision: D8055038
Pulled By: hermanventer
fbshipit-source-id: 4dd394bb0c4df2f9656be453b8feae57cc0db516
Summary:
Release notes: none
This PR fixes a bug where prototype methods on unknown arrays with widened numeric properties were incorrectly being used instead of following the route to filter which methods were safe or not. The functionality and logic for this was pulled out, into its own function so it can be re-used in places and tidied up.
Closes https://github.com/facebook/prepack/pull/1981
Differential Revision: D8043347
Pulled By: trueadm
fbshipit-source-id: 05415e5bdcb85818681a72323df16711a89f7c5c
Summary:
Release note: fix numerous problems with code ordering
Closes: #1914
We've been seeing an endless stream of bugs where code is either missing or in the wrong place. A lot of these were due to the complexity of joinEffectsAndPromoteNested and some more are due to a fundamental flaw in its algorithm: it rotates branches of the effects tree in order to group together all completions of a given type. That can cause generators to end up in the wrong place in the temporal sequence.
We also had a problem with generators being duplicated in the tree.
This pull request introduces a new algorithm for doing what joinEffectsAndPromoteNested does. This too is complicated and I spent days on debugging and fixing many subtle issues. So, for now, I'm only doing it for calls. More will come in later pull requests.
I am not super confident that I've found and addressed all subtle bugs. I think we can expect to run into quite a few more of those. I am, however, reasonably confident that we now have the right approach to dealing with abrupt control flow.
Please have a careful look, but let's get this landed as soon as possible because it is on the critical path.
Closes https://github.com/facebook/prepack/pull/1979
Differential Revision: D8044482
Pulled By: hermanventer
fbshipit-source-id: 317402c5601fdea7366c9daf7762af0217302b54
Summary:
When either name or msg of Error is abstract, we do not want to partially evaluate it because the checks on string emptiness can be overwhelming, and better done in the VM than in JS.
Added a test to check these cases.
(I messed up my previous pull request. Sending a new one here.)
Closes https://github.com/facebook/prepack/pull/1974
Differential Revision: D8028508
Pulled By: lxfind
fbshipit-source-id: 8e1d1f2151e0b08b519935387298d4ffcd579f9b
Summary:
Release notes: none
This is a follow up to the changes in https://github.com/facebook/prepack/pull/1926. This PR correctly fixes the derived values for waiting and ensures that the unknown array objects have their dependencies properly resolved. This fixes part of the issue in https://github.com/facebook/prepack/issues/1962, but it brought about another issue that this PR doesn't aim to solve – dependencies on an abstract function call's unbound/bound bindings.
Closes https://github.com/facebook/prepack/pull/1970
Differential Revision: D8024402
Pulled By: trueadm
fbshipit-source-id: b9f9e725b7b22241209ef5a6f3b29dc0f2eeddeb
Summary:
Release notes: More simplification rules
The existing simplifier would never go into relational expressions
and distribute conditional expressions. It does now.
With this change, `AbstractValue.createFromBinaryOp` joins many other
value factory functions that may return non-abstract values.
This caused various changes downstream in types, and dealing with concrete
instead of abstract values.
Adding regression test.
Closes https://github.com/facebook/prepack/pull/1972
Differential Revision: D8023363
Pulled By: NTillmann
fbshipit-source-id: a3cd6f7752ba24d039c6085bec2ee93f7587af26
Summary:
Release notes: None
It used to bail out for conditional abstract values when accessing
a prototype that existed on neither side of the condition. No more.
Also, things used to be wrong in the presence of object prototypes.
That's also getting fixed.
Adding regression tests.
Closes https://github.com/facebook/prepack/pull/1963
Differential Revision: D8000539
Pulled By: NTillmann
fbshipit-source-id: 5214f213ea49c63e9637126360d3c90aace92d20
Summary:
Release notes: None
This fixes#1957.
Turns out that the special case is no longer needed, and just removing that code fixes the issue now.
Adding regression test.
Closes https://github.com/facebook/prepack/pull/1958
Differential Revision: D7995563
Pulled By: NTillmann
fbshipit-source-id: 6822c5433679fd88bd5a052ca23824cb3f3eabba
Summary:
Release note: Fixes problem with result of Array.filter (and similar) applied to arrays with unknown properties
Closes: #1924
Generator.deriveConcrete can result in temporal object values. These must be marked with _isScopedTemplate to prevent them from being hoisted.
Closes https://github.com/facebook/prepack/pull/1926
Reviewed By: trueadm
Differential Revision: D7949463
Pulled By: hermanventer
fbshipit-source-id: 2f15117725c37e9b09cae8ac469ee5b2eb481b6c
Summary:
Release notes: None
The `Object.freeze` command (or similar) was put into the body
where the object was allocated, not where it got initialized.
This would cause the initialization code to silently fail later on when the object was used for the first time.
This fixes it, and adds a regression test.
Closes https://github.com/facebook/prepack/pull/1936
Differential Revision: D7994911
Pulled By: NTillmann
fbshipit-source-id: 6ec0706357706f7bf80f62690be9aea1f30f8a72
Summary:
Fixes https://github.com/facebook/prepack/issues/1935.
The issue was that we'd always put `EmptyValue` into the right branch even if it happened in the left one.
Adds a regression test.
Closes https://github.com/facebook/prepack/pull/1937
Reviewed By: trueadm
Differential Revision: D7967344
Pulled By: gaearon
fbshipit-source-id: ece8550787679e2fc33f040989ceae0cf3a7c2a2
Summary:
Release notes: None
This fixes#1856, at least in the sense that Prepack won't silently generate wrong code.
Instead, Prepack will now issue an error indicating that a Prepack limitation was hit.
Adding error-handler regression test.
Also fixed reference to PP1023 (no wiki page) to be PP0023 (matching wiki page) instead.
Closes https://github.com/facebook/prepack/pull/1929
Differential Revision: D7954903
Pulled By: NTillmann
fbshipit-source-id: f9676f33e74ff36067832333a7ee9a26ec77126d
Summary:
Release notes: the React reconciler now throw a FatalError upon encountering side-effects in a render
This PR revamps the current React system's restrictions for what you can and can't do during the React reconcilation phase. This is a pretty large update but provides much better boundaries between what is "safe" and not "safe", thus reducing the constraints.
1. A new error `ReconcilerRenderBailOut` is thrown if something occurs in the React reconciliation that causes the render to fail and it's a hard fail – no recovering and continuing.
2. If you mutate a binding/object outside the render phase, given the React component render phase is meant to be "pure", a `ReconcilerRenderBailOut` will be thrown.
3. If you `throw` during the React reconciliation phase, again a `ReconcilerRenderBailOut` will be thrown.
In the future, we should maybe loosen the constraints around all this and maybe allow `throw`, but right now it's causing too much friction. We should attempt to make React components render phase as pure as possible – as it results in much better optimizations by a compiler because we can assert far more without things tripping us up.
Another point, regarding (1), is that we should ideally be able to recover from the error thrown in Prepack. The reason we can't and is something that might be a very deep issue in Prepack, is that effects don't properly restore when we have nested PossiblyNormalCompletions at work. Bindings get mutated on records from changes made within `evaluateForEffects` but never get reset when in nested PossiblyNormalCompletion. If I remove all the things that can cause `PossiblyNormalCompletion`s then everything works fine and bindings do get restored. We can remove the constraint on (1) once we've found and fixed that issue.
Closes https://github.com/facebook/prepack/pull/1860
Differential Revision: D7950562
Pulled By: trueadm
fbshipit-source-id: 4657e68b084c7069622e88c9655823b5f1f9386f
Summary:
Release note: none
Resolves issue #1865
Abstract values that are known to result in objects (of unknown identity) should not be eligible for Common Subexpression Elimination (CSE).
Closes https://github.com/facebook/prepack/pull/1925
Differential Revision: D7941893
Pulled By: hermanventer
fbshipit-source-id: 454e26c6d042093b9fbae06d73110202083731b6
Summary:
Release notes: fixes an issue with getOwnPropertyDescriptor when dealing with a native prototype function
This PR fixes#1285, specifically when using `Object.getOwnPropertyDescriptor` on a native prototype function, we have to resort to returning a temporal abstract as this function isn't available at build time. I also renamed all the tests with typos in them.
cc anilanar
Closes https://github.com/facebook/prepack/pull/1913
Differential Revision: D7935215
Pulled By: trueadm
fbshipit-source-id: d7e33cf8bd73aa2c1156966ad86e9da9fb0404e4
Summary:
Release note: Do not generate code for unreachable global code statements
Resolves issue #1906
If an unconditional throw is encountered, it does not make sense to keep processing top level statements.
Closes https://github.com/facebook/prepack/pull/1908
Differential Revision: D7934963
Pulled By: hermanventer
fbshipit-source-id: d6e6b26d7ad7c54a065e43a74ad1b27959159c7a
Summary:
Release notes: None
This fixes#1821.
Improve tracking of target bodies in Emitter,
which is necessary now that generators can have effects
and one should not attempt to emit into a body
with the wrong effects in place; removed some dubious code there.
It turned out that the exact placement of `Object.freeze` and similar statements
was always a bit fragile; this issue came out after fixing the Emitter, and so
this is made robust by keeping track of the
emission of all other object property mutations, and only emitting the
`Object.freeze` after they are all done.
When visiting modified property bindings, re-visit object in case it was
previously visited from a different scope.
For objects visited (only) in scopes where additional generator effects have been
applied, record additional information for serialization to ensure that these
objects get serialized in their creation scopes, so that the initial values
of the object properties are not polluted by the additional generator effects
of some nested scope.
Adding regression test.
Closes https://github.com/facebook/prepack/pull/1850
Differential Revision: D7928970
Pulled By: NTillmann
fbshipit-source-id: e8587be393fb078cc503718932057d7b10d0df72
Summary:
Release note: Fix problems with duplicated and missing statements
Resolves issue: #1842
Code duplication happened because the joined effects of two abrupt completions were applied first in aggregate and then again individually.
While debugging through this, I found it much simpler to understand Generator.fromEffects when applied to an Effects that has not been joined, so now things are closer to what they were before I started trying to hack past issues. The main difference is that a fully joined up set of effects is applied before serializing the result from Generator.fromEffects.
All of this caused an invariant to fail during a test. The invariant is complicated, fails in other new scenarios and is being reworked by Nikolai, so I've removed it for now in order to get these fixes in so that we are all on the same page again.
Closes https://github.com/facebook/prepack/pull/1903
Differential Revision: D7920610
Pulled By: hermanventer
fbshipit-source-id: 89b6ac9f1b8df776657c3a93550fdd3eb2ae4cd7
Summary:
Release notes: Removing --simpleClosures option.
This feature was never fully implemented and remained buggy.
Instead of having two distinct modes, let's rather try to improve
pointwise what we perceive as inacceptible for the regular way of
why closures work in the serializer.
Closes https://github.com/facebook/prepack/pull/1876
Differential Revision: D7889689
Pulled By: NTillmann
fbshipit-source-id: 782a6754408a9b250d45691274902c6bb2ed6924
Summary:
Release note: none
When joining PossiblyNormalCompletions/JoinedAbruptCompletions while promoting particular kinds of completions (such as return completions), do not compose normal effects since that has already happened.
Thanks to Sapan for finding the issue, making a nice test case and identifying the code that is at fault.
This PR supersedes #1872.
Closes https://github.com/facebook/prepack/pull/1875
Reviewed By: trueadm
Differential Revision: D7888926
Pulled By: hermanventer
fbshipit-source-id: 58071b80ce68e89e5a38c0b7b5a2061c5539a304
Summary:
Release notes: none
This fixes https://github.com/facebook/prepack/issues/1866. The React reconciler previously had somewhat broken logic when dealing with conditional that may require effects being joined together for things such as if statements. This re-uses the logic in `IfStatement` to ensure we get it right. Furthermore, I found a bug in `fb20.js` that failed unless we made sure we throw a `FatalError` in `AbstractObjectValue` where both consequent and alternate values are not found – like done in other methods in `AbstractObjectValue` joining together a value in this scenario actually generated broken output.
Closes https://github.com/facebook/prepack/pull/1874
Differential Revision: D7887489
Pulled By: trueadm
fbshipit-source-id: 64595296e5090b55a1670ab989e6737d7febab55
Summary:
Release note: none
While debugging various issues with generators, I noticed that joinEffectsAndPromoteNested does not quite live up to its advertising because it does not handle all the permutations of results that could contain abrupt completions of the given type.
Fixed this and added test cases. The tree manipulations are quite subtle. I went to some trouble to verify that they are correct, using diagrams, but did not want to put the diagrams in the code, so the reading may be a bit tough.
Closes https://github.com/facebook/prepack/pull/1873
Differential Revision: D7886038
Pulled By: hermanventer
fbshipit-source-id: df874ca5fad771932490428d54ff22b022fec74c
Summary:
BreakCompletions that were not joined previously fall through joinAllLoops. This change converts such cases to NormalCompletions. Without this change, prepacking fails with an invariant failure.
Closes https://github.com/facebook/prepack/pull/1863
Differential Revision: D7876263
Pulled By: sb98052
fbshipit-source-id: fed191c1163da412291c6a09674220b6133a99b1
Summary:
Small fix to value of joined BreakCompletions
Closes https://github.com/facebook/prepack/pull/1861
Differential Revision: D7867413
Pulled By: sb98052
fbshipit-source-id: 94ff51ae0225b8381a79d9fd53128b18448b65a4
Summary:
Release note: Support switch statements where the switch value is abstract and one or more cases throw
Resolves#1838
When only some of the cases in a switch statement throws, the join results in a possibly normal completion and this has to get composed with the saved completion.
The test case also revealed that the join was happening in the wrong order, so that is fixed as well.
Closes https://github.com/facebook/prepack/pull/1844
Differential Revision: D7861034
Pulled By: hermanventer
fbshipit-source-id: f4a8614e1f8769f942018345e2e1024d8013cf67
Summary:
Release note: Fixed bug with implicitly converting unknown values to Object
Resolves#1845
Joined values can be of type PrimitiveValue, so the invariant in the default case of the switch in _WrapAbstractInObject is actually not true.
Added handling for this case, throwing an error if the joined value might be null, undefined or missing and converting it lazily to Object otherwise.
Closes https://github.com/facebook/prepack/pull/1849
Differential Revision: D7853117
Pulled By: hermanventer
fbshipit-source-id: eb8e8d85c13f9eee4320badb2209c5581f27ef0c
Summary:
Follow up to #1839
It's not safe to mutate an expression in a build function. Those needs to be emitted as statements.
We don't have to deal with the assignment part of it though. We can just calculate the new value and deal with the fact that ToNumber might throw or mutate.
The new value we can just pass to PutValue which takes care of the rest.
Closes https://github.com/facebook/prepack/pull/1847
Differential Revision: D7853622
Pulled By: sebmarkbage
fbshipit-source-id: 1e370c2961048f5ce22becb7385a84b92ecc7045
Summary:
Release note: Fixed code duplication bug
Issue: #1829
When joining up a saved possibly abrupt completion with the current completion (in Functions.incorporateSavedCompletion), the state must be updated with the effects of the normal part of the join. The normal part of the join does not include the joined generator, so that must be explicitly excluded when applying the effects.
The subtle reason for this is that the type of join point is only known to the caller of incorporateSavedCompletion and, depending on the type of join point, some parts of the joined completion may be excised from the join and put back into a saved completion. This means that only the caller knows exactly which parts of the abrupt completions should have their effects (and generators) reflected in the current state.
Closes https://github.com/facebook/prepack/pull/1834
Differential Revision: D7848979
Pulled By: hermanventer
fbshipit-source-id: 80e61692ea8f3b57930b5125178b560fa3d47af5
Summary:
Release notes: adds ReactDOM mocks to fb-www compatibility mode
This PR aims to do a bunch of things:
- Tidy up the spaghetti React mock logic by adding in some helper functions
- Add ReactDOM mocks, pathing the way for greater optimizations for first render
- Swap the `Get` to a `getProperty` in `isReactElement` function
- Make the fb-www mocks not ignored by eslint and fix the eslint issues after doing so
Closes https://github.com/facebook/prepack/pull/1835
Differential Revision: D7845112
Pulled By: trueadm
fbshipit-source-id: 7173c543da6af2c20a38fbc70f962825647f8f3e
Summary:
Release note: Support multiple assignments to unknown properties in optimized functions
Issue #1818.
The code in #1822 can only deal correctly with a single assignment. This generalizes that code to generate separate assignments for each different (key, value) pair found in the conditional value tree that is used to model a group of assignments to unknown properties of known objects.
Closes https://github.com/facebook/prepack/pull/1828
Differential Revision: D7840880
Pulled By: hermanventer
fbshipit-source-id: 8f2bb12cfd5246dc13d5a360ff9bb16a9326dd41
Summary:
Release note: Support temporal assignment to object properties using unknown property names
Resolves issue: #1818
There was no code for dealing with assignments using unknown property names that showed up in generator entries.
Closes https://github.com/facebook/prepack/pull/1822
Differential Revision: D7833068
Pulled By: hermanventer
fbshipit-source-id: 2c3e8d211bd4770ba736ff9be03a0f4d85adad34
Summary:
Release notes: none
This PR fixes https://github.com/facebook/prepack/issues/1819. It expands on the existing invariant check in `AbstractObjectValue` by also allowing abstract object values to pass through.
Closes https://github.com/facebook/prepack/pull/1824
Differential Revision: D7829938
Pulled By: trueadm
fbshipit-source-id: 7e198bc626b4fd2737d0ef9d2a7207bd17b0b02a
Summary:
Release notes: None
This fixes#1771.
The `PropertyBinding` `key` type used to be just `any`; not sure what the historical story here is,
but the visitor was unaware that this in fact could be an `AbstractValue` as well as a `string`.
This meant that some keys might not get visited when they should have been.
This refines the key type and accounts for the different possible types in all referencing places.
Closes https://github.com/facebook/prepack/pull/1816
Differential Revision: D7827049
Pulled By: NTillmann
fbshipit-source-id: 88fa2e036d1d4393d025af38954e309f6632a839
Summary:
Release note: Resolves issues with outer declarations moving into nested scopes
Issue: #1808
The fix identified in #1805, needs to be applied in a number of other contexts as well.
Closes https://github.com/facebook/prepack/pull/1813
Differential Revision: D7821306
Pulled By: hermanventer
fbshipit-source-id: dd08198f5d031c2e368bf5da179c2a11ac0f3b3a
Summary:
Release notes: none
This should fix https://github.com/facebook/prepack/issues/1803 and extends from the unsuccessful attempts made in https://github.com/facebook/prepack/pull/1809 to fix this.
When we apply the `priorEffects` inside of the `evaluateForEffects` on TryStatement, we have to ensure that the `canBeApplied` flag is reset (as the bindings on the Realm get restored by `evaluateForEffects`).
Closes https://github.com/facebook/prepack/pull/1811
Differential Revision: D7820907
Pulled By: trueadm
fbshipit-source-id: 248cc0f21be0044de8641e79c5985c7259a88629
Summary:
Release notes: none
This adds more validation that something definitely is a ReactElement, as found from the issue https://github.com/facebook/prepack/issues/1802. Unfortunately, the test in that case does not pass, it fails due to an invariant:
`Invariant Violation: an abstract value with an identifier "_$1" was referenced before being declared`
Closes https://github.com/facebook/prepack/pull/1810
Differential Revision: D7813932
Pulled By: trueadm
fbshipit-source-id: e431887002da83389da4062cfe00afbdc42d21c8
Summary:
Release notes: None
This fixes#1789.
The visitor already computed the right scopes for all objects,
which the serializer then used to figure out in which code block
to emit an object.
However, the serializer didn't take into account that the wrong
effects might be applied when serializing values in a simple
recursive manner, finding an object from an outer scope while
serializing an optimized function.
To fix this, (re-)introduce an additional roots set which is used
to ensure that all objects belonging to an outer scope get serialized
before we enter an additional function where effects might get applied.
Adding regression test case.
Closes https://github.com/facebook/prepack/pull/1794
Differential Revision: D7814388
Pulled By: NTillmann
fbshipit-source-id: 5709fcd676b9b86dd66b2952155039d09d873a70
Summary:
Release notes: none
When digging deeper into detecting write conflicts with React component trees, I found that PossiblyNormalCompletions were not handled properly, so this PR ensures that the effects tree is recursively traversed so all modified bindings are collected, even from PossiblyNormalCompletions.
Update:
With all the latest changes, all tests in this PR now pass, so it's ready to be reviewed again.
Closes https://github.com/facebook/prepack/pull/1742
Differential Revision: D7813778
Pulled By: trueadm
fbshipit-source-id: 425ec505709c83081c7eef3bec9bf95bd651299e
Summary:
Release notes: none
This PR fixes an issue where the React reconciler incorrectly assumed that `createFromConditionalOp` always returned an `AbstractValue`. This changes that and adds the failing test for a regression test.
Fixes https://github.com/facebook/prepack/issues/1804.
Closes https://github.com/facebook/prepack/pull/1807
Differential Revision: D7813026
Pulled By: trueadm
fbshipit-source-id: 5af963fb0e2351d073df554cda33b979d52c9e60
Summary:
Release notes: None
This fixes#1798, and it almost fixes#1797 by reducing it to #1799, and it unblocks #1794.
When the visitor looks at (initial) function bindings, it does so with additional functions still applied if that additional function happens to be the first thing that pull on the function value.
This is being fixed, as well as generally turning _withScope uses into _enqueueWithUnrelatedScope calls which ensure that the right effects are in place.
Added additional output to the serializer runner - it now shows a summary of which tests failed, ordered by their size.
Added regression test.
Closes https://github.com/facebook/prepack/pull/1806
Differential Revision: D7805532
Pulled By: NTillmann
fbshipit-source-id: 274859e9f6f74ebfc4decc37d984cedcbedf990c
Summary:
Release notes: none
This PR provides a mechanism for the React reconciler to understand newly created arrays from maps with unknown properties, in this this first case this is only for `Array.from`, `Object.keys`, `Array.prototype.map`, `Array.prototype.filter`, `Array.prototype.reduce` (with constraints), `Array.prototype.slice` and `Array.prototype.concat`.
When we have something completely abstract, like `props.items`, and it's been wrapped in `Array.from`, we can assume that (in a pure render) that it's safe to emit the `Array.from` and make the array with an unknown property that is an abstract widened hint. The reconciler can see this and perform a nested optimized closure to the map callback.
In cases where the third argument `thisVal` is passed to array methods, we bail-out.
Closes https://github.com/facebook/prepack/pull/1770
Differential Revision: D7795295
Pulled By: trueadm
fbshipit-source-id: 3396330da6d4ebe2ffdd1f1610173eb482df9081
Summary:
Release note: none
Addresses issue: #1763
If the abstract selector value of a switch statement has a known values domain and ranges over a small set of values, we can generate better code by not having to join up all the case blocks in the switch.
Closes https://github.com/facebook/prepack/pull/1786
Differential Revision: D7792997
Pulled By: hermanventer
fbshipit-source-id: 147f52ad666f2617a8e16c3d7363341742adf2f9
Summary:
Release note: none
Fixes issue: #1766
Generators that result from joining two blocks of code have lots of entries that are the joins of the generators of the individual blocks. When generating code for optimized functions, these entries need to get ignored since they will be visited while recursing through the top level result. That much was achieved with the PR #1758.
That turned out to be too simplistic since joined generator entries can get appended into generators with existing entries. This PR restores visiting generators from effects with joined results, but purges them of entries that resulted from the join.
This is somewhat brittle and we need to revisit the entire generator design at some point, but this PR gets us unblocked and does seem to make things strictly better.
Closes https://github.com/facebook/prepack/pull/1795
Differential Revision: D7787380
Pulled By: hermanventer
fbshipit-source-id: 23e8924282cfdabf77969ff716ac936c12daa6ad
Summary:
Release notes: None
This fixes#1772.
When an optimized function modifies properties of objects
created outside of its scope, we can omit such property modifications
if the modified object is not used.
This nicely fits in with the fixed point model of the visitor.
(The objects involved in object property modification used to complete bypass the whole visiting process, resulting in the crash.)
Note that as a positive side effect certain double-mutation errors won't be issued to the user.
Added simple regression test.
Manually verified that this also makes the larger React test mentioned in the issue pass.
Also fixed ordering ordering of effects applications when processing queue in residual heap visitor. This bug existed for a while, but was only brought out by the existing serializer tests after the more aggressive queueing of modified object property binding visitng.
Closes https://github.com/facebook/prepack/pull/1793
Differential Revision: D7784644
Pulled By: NTillmann
fbshipit-source-id: 3dd21cc89275a30c5312f7bdd94e7d0364d02959
Summary:
Fixing equals() on Values
Release notes: None
I found the following bugs in equals():
- For numbers, it didn't take into account NaN.
- For symbols, it was too optimistic.
And for objects, it was overly complicated.
Adding regression test for symbols. Could produce a regression test for numbers, but file a related issue for NaN values.
Closes https://github.com/facebook/prepack/pull/1791
Differential Revision: D7784665
Pulled By: NTillmann
fbshipit-source-id: 96bdcce6c771c8549dc52cd73c646d0e15b5b9b0
Summary:
Release note: none
EmptyValue is a signal that the property that (might) have the value could in fact not actually be there. This matters when constructing objects at runtime, since code has to be emitted to deal with the case where the property will not be created at all, due to a runtime switch.
On the other hand, when the value of a missing property is *used*, it should be the undefined value. For that scenario, there is a promoteEmptyToUndefined on Value and this should be called before serializing empty values that are actually used.
This was not done for values returned from optimized functions. This PR plugs that hole.
Closes https://github.com/facebook/prepack/pull/1787
Differential Revision: D7776331
Pulled By: hermanventer
fbshipit-source-id: ebf8f5335e8ed3f6c557e00f3c88b08ccbd9c14d
Summary:
Release notes: none
This PR came about from trying to fix https://github.com/facebook/prepack/issues/1771. This PR fails on the tests still, so I need some help getting them resolved.
- `_emitProperty` should account for the fact a `key` can be an `AbstractValue`, thus needs to serialize the value.
- `visitObjectPropertiesWithComputedNames` in the visitor and `_getNestedAbstractValues` in the serializer both need to take into account cases where `alternate` and `consequent` might not be `AbstractValue`s.
Closes https://github.com/facebook/prepack/pull/1781
Differential Revision: D7777483
Pulled By: trueadm
fbshipit-source-id: 1b94489ec9c800a68414031e0bc63b06061fe559
Summary:
Release note: Allow break/continue to be guarded by abstract conditions inside loops
This addresses the first example in issue: #1764
Loops represent join points for break and continue completions. They need to be handled much like the return completions that join at function end. The main difficulty is that break and continue can specify a target. For now, we punt on that and give a compile time error.
Closes https://github.com/facebook/prepack/pull/1778
Differential Revision: D7762824
Pulled By: hermanventer
fbshipit-source-id: d8d3699277b7bb20817a27ecc24f3c503904fc74
Summary:
Release notes: none
When doing `Object.assign` in `getDerivedStateFromProps`, we need to add back in the fallback for when it might fail otherwise we end up triggering invariants. This fixes https://github.com/facebook/prepack/issues/1773
Closes https://github.com/facebook/prepack/pull/1780
Differential Revision: D7773190
Pulled By: trueadm
fbshipit-source-id: 2539f01cfde3a65136a6e9947a4620dfc81a371a
Summary:
Release notes: none
There was a case where an invariant was firing in the hoisting logic due to a binding value being undefined. This PR stops the invariant from firing in such cases, and includes the regression test. I also found another issue in the evaluatedReactNode debugging output and fixed that too so it was easier to see what was going on with this and other issues.
This fixes https://github.com/facebook/prepack/issues/1775
Closes https://github.com/facebook/prepack/pull/1779
Differential Revision: D7763057
Pulled By: trueadm
fbshipit-source-id: 61c866f7cb7322f5d884262adb85613c97cfa4ca
Summary:
Release notes: none
This fixes bugs around detecting independent dangling `this` keywords in methods of a React class component and correctly bailing out upon finding them. This is typically used in cases where we'd want to bail out, for example:
```js
let self = this;
let handleClick = function () { console.log(self.x) };
return <div onClick={handleClick} />
```
or:
```js
let handleClick = function () { console.log(this.x) };
return <div onClick={handleClick.bind(this)} />
```
Closes https://github.com/facebook/prepack/pull/1777
Differential Revision: D7757435
Pulled By: trueadm
fbshipit-source-id: b06fe29a3ac4a44bdf2b8a7c6e9457e5801b32b7
Summary:
Release note: none
When composing a newly constructed PossiblyNormalCompletion with saved one, the resulting completion needs to capture the effects that happened on the normal branch, in between the saved fork point and the fork point of the newly constructed completion, because these effects also apply to the abrupt part of the new completion.
When joining up such a composed completion, the effects need to be applied (and later reversed), otherwise the nested joins might capture values as they were before the first fork, rather than as there were at the time of the nested fork.
Closes https://github.com/facebook/prepack/pull/1769
Differential Revision: D7746284
Pulled By: hermanventer
fbshipit-source-id: d2396f29f93c676e330ac6460f5ac9e638329528
Summary:
Release notes: none
This adds the functionality to fold/inline ReactRelay components via `createFragmentContainer`. This is for first-render only for now.
Closes https://github.com/facebook/prepack/pull/1745
Differential Revision: D7686225
Pulled By: trueadm
fbshipit-source-id: 7eb546ff70e92b60bcc1abb69c0cc5091926c012
Summary:
Release notes: None
The serializer used simply invoke the serialization
machinery for the special empty value when it was
trying to generate code for a possibly-deleted property.
However, there was no matching visiting of the special empty value.
This fixes issue #1747.
Adding regression test case.
Closes https://github.com/facebook/prepack/pull/1751
Differential Revision: D7672464
Pulled By: NTillmann
fbshipit-source-id: 60b2e4eb4246abb61d2361b8ef88d293de021ef5
Summary:
Release note: none
Joining effects results in joined generators that contain all of the I/O operations of the argument effects, but does not change the arguments.
Normally, this does not matter because only the joined generator is used. In the case of an optimized function, however, the generator tree is traversed via the effects tree and this resulted in duplicate I/O operations.
To fix this, the tree walk now ignores joined generators. Also included in this PR, some fixes to make sure that only the (joined) effects resulting from _createAdditionalEffects are used, and not the effects that went into it.
Closes https://github.com/facebook/prepack/pull/1758
Differential Revision: D7687328
Pulled By: hermanventer
fbshipit-source-id: c3a333369bc403bed5f986e5d9d59cc56ab62ced
Summary:
Release notes: none
When a `null` or `undefined` value is passed as the `config` for `createElement`, then we need to create an empty object to be used instead.
Closes https://github.com/facebook/prepack/pull/1759
Differential Revision: D7686087
Pulled By: trueadm
fbshipit-source-id: a8eb917f38ca3afe881af08d07fe8be0943b0a1f
Summary:
Release notes: none
Ensure we apply the nested closure effects in the right order. This was the underlying bug after doing much digging.
Furthermore, I added case where this fails. I also found that the ReactElement code was causing issues at one point (I forgot to add the props, type, ref and key as dependencies), but that wasn't needed in the end for this fix, but I thought it best to keep those fixes in.
I also had to make a change to the JSON tester as it was incorrectly failing a test because the function closures were not 1:1, which they're not meant to be.
Closes https://github.com/facebook/prepack/pull/1743
Differential Revision: D7686048
Pulled By: trueadm
fbshipit-source-id: 8cf7faed51c86720e519abbbad209820e7e02dc4
Summary:
Release note: none
I noticed that the stack properties for Error objects were being set to the same value even when the objects were constructed in separate lines.
It turns out that some recent caching logic used the containing function as key and thus unified all error location descriptions encountered in the same function to a single value. So part of this PR is just a fix that uses the actual location, if available, as the cache key.
This unmasked another recently introduced problem: There are now invariant checks that happen if you get a property from the global object. The function where the checks are triggered is called as part of the describe logic. This logic was largely disabled because of the caching bug, so the checks did not fire in any test case and we missed out on the fact that the check don't work if the property get logic is not part of the source program.
To fix that, all calls to $Get that are done for internal purposes have now been replaced by calls to _SafeGetDataPropertyValue.
Closes https://github.com/facebook/prepack/pull/1755
Differential Revision: D7683687
Pulled By: hermanventer
fbshipit-source-id: 1c811fffc6ecf275f5fabd75afbd9e8cfc051247
Summary:
Release note: none
The code for disentangling return completions from throw completions uses a recursive algorithm that caused the throw completions to get reversed. Fix this to prepend rather than append so that the order comes out right.
This is also an opportunity to not mess with the path conditions.
Closes https://github.com/facebook/prepack/pull/1752
Differential Revision: D7676676
Pulled By: hermanventer
fbshipit-source-id: f10326e21bdf3f15c2ddf7eac02ef02cf8d155d3
Summary:
Release note: none
The effects constructed for an optimized function must join all paths that may leave the function in order to provide a full accounting of what the function might do to the global environment. Without this, the global environment can get polluted by side effects in the abrupt branches, because these effects are applied to the environment when constructing the generator from the overall effects, but are not reversed when backing out the overall effects.
This can break things in many different ways, one of which is illustrated in the modification of return-or-throw-simple.js, where the last return x is serialized as undefined rather than the abstract value, because x got polluted during the construction of the effects for func1.
Closes https://github.com/facebook/prepack/pull/1749
Differential Revision: D7664313
Pulled By: hermanventer
fbshipit-source-id: 3b061d16a4f38918f461b792b660954332ae6fa7
Summary:
Release notes: Improved code generation in the presence of branching
This is fully implemented now:
- The visitor keeps track of whether the consequent or alternate of conditional values are feasible. If something is known to be infeasible, it is never visited.
- This in turn allows the serializer to skip serializing infeasible branches of conditional values.
- The emitter also knows that those infeasible branches are no dependencies.
- Added a test case for the basic functionality, one that shows that complex objects get eliminated, and one that tests the dependency code.
Closes https://github.com/facebook/prepack/pull/1697
Differential Revision: D7606266
Pulled By: NTillmann
fbshipit-source-id: 09bfe8c2a2838ebd252a93a98013b31158909d2f
Summary:
Release note: none
Generator._deconstruct did not handle PossiblyNormalCompletion and was asymmetrical for ReturnCompletion. Fixed this and added a test case.
Also deleted a dead method.
Closes https://github.com/facebook/prepack/pull/1744
Differential Revision: D7624647
Pulled By: hermanventer
fbshipit-source-id: edf72696767cf23eaadfb27b5951f172c7a8275a
Summary:
Release notes: none
When working on internal projects, I found cases where the `getDerivedStateFromProps` would throw a FatalError because the partial state wasn't an abstract object value or object value. In this case we can see that the abstract value is a conditional and handle the paths to ensure we get the correct results without failing.
Closes https://github.com/facebook/prepack/pull/1737
Differential Revision: D7615094
Pulled By: trueadm
fbshipit-source-id: 5c868a46936a108c277ac52c2675feabc1f4df8c
Summary:
Release note: Execute Do/While loop condition in the proper place
Fixes issue: #1677
The fixpoint computation computed the while condition of the first iteration as part of the second, and so on.
Refactored the code to make the fixed point computation independent of where the loop condition is executed. This simplifies things and will come in handy when doing while/do.
Closes https://github.com/facebook/prepack/pull/1741
Differential Revision: D7610522
Pulled By: hermanventer
fbshipit-source-id: 5a86b1ea179b5e03c773b642375b91ad3837d868
Summary:
Release notes: None
This fixes an internal test failure.
Changes:
- (Re)Visit bindings in the right function scope
- No longer mutate the binding's value field after it's initialized once
- Don't visit/serialize old binding value when it's not needed - adding test case for that
- Delete unnecessary getModifiedBindingOldValues function; whatever it was needed for can be done as part of visitModifiedBinding.
- Delete some code that performed special handling for bindings referring to functions. Doesn't seem to be needed.
Closes https://github.com/facebook/prepack/pull/1733
Differential Revision: D7607978
Pulled By: NTillmann
fbshipit-source-id: faa45963753408a35015ce8c57009cc287d53624
Summary:
Release notes: None
Don't generate redundant calls of the form
`var __captured__scope_2 = __scope_0[0] || __scope_1(0);`
Adding regression test.
This addresses one of the observations of #1732.
Also deleted unneeded left-over code and comment that used to refer #989.
Closes https://github.com/facebook/prepack/pull/1734
Differential Revision: D7602949
Pulled By: NTillmann
fbshipit-source-id: 77d926b236a903eb7927a1471411e8397a0d192a
Summary:
Release notes: Introducting new option --invariantLevel NUMBER and dropping --omitInvariants
The new default is invariant level 0, which corresponds to the old --omitInvariants.
Level 1 is roughly what we used to do: Check invariants on (derived) abstract values.
Level 2 implements #1180: Checking that all accesses to built-ins do what Prepack expects them to do, including checks for when properties on built-ins are absent.
Level 3 adds checks for internal consistency, basically an internal debug level.
The serializer tests now run with the highest invariant level by default. The added invariants found a few issues that got addressed, including:
- Prepack exposes a TypedArray prototype, which is not a real thing. It's now marked as `__refuseSerialization`, and no invariants are emitted for such things.
- Global variables / properties on the global object are special. Those are not yet handled at level 2.
- Accesses to Prepack magic functions that generally start with `__` should not produce invariant checks.
- Magic code generation for loops should not take into account objects that are `__refuseSerialization`.
- All invariant statements get a unique id to make it easier to locate them.
- I marked some test cases which depend on counting occurrences in the output as "// omit invariants", as the additional invariants increased some such counts.
As part of testing, I also found it necessary to make the --invariantMode more useful; it now also allows specifying nativeLoggingHook which is the preferred way of logging in React Native.
To reduce the number of checks by a few orders of magnitude in practice, each property is only checked on first access. This is tracked by a global variable `__checkedBindings`.
This pull requests incorporates all aspects of the #1709 (which I abandoned).
Closes https://github.com/facebook/prepack/pull/1724
Reviewed By: simonhj
Differential Revision: D7575091
Pulled By: NTillmann
fbshipit-source-id: 585cd224ce66882f8e5f27d88c1ad08afeb96ee1
Summary:
Release notes: none
This PR adds some sanity around React components roots that when render, mutate the same of bindings as other React component roots. This is currently not supported and thus safely bails out and continues.
Closes https://github.com/facebook/prepack/pull/1736
Differential Revision: D7600373
Pulled By: trueadm
fbshipit-source-id: fe2b2b3055fa3c1b28668da6d67bca3826431eb6
Summary:
Release notes: Adds`createRef` and `forwardRef` to the React compiler
This PR adds both features from React 16.3, including full inlining of forwardRef functions.
Closes https://github.com/facebook/prepack/pull/1731
Differential Revision: D7599780
Pulled By: trueadm
fbshipit-source-id: 50458546c7a4374baea7fa80feb21262dd5692af
Summary:
Release notes: adds React 16.3 `getDerivedStateFromProps` support to the React compiler
This PR adds support for the React 16.3 `getDerivedStateFromProps` lifecycle event on class components. I've also updated the React testing dependencies to 16.3.1. I also extended the `componentWillMount` logic to support `UNSAFE_componentWillMount` too.
Closes https://github.com/facebook/prepack/pull/1730
Differential Revision: D7586378
Pulled By: trueadm
fbshipit-source-id: 895e74ca86eb9ff9ad04aa93f021c107f08a7b9f
Summary:
Dropped entries in WeakSet and WeakMap that are not referenced elsewhere from serialized output.
This addresses #680.
Closes https://github.com/facebook/prepack/pull/1710
Reviewed By: NTillmann
Differential Revision: D7573234
Pulled By: sb98052
fbshipit-source-id: 64c99b8ea979b3d39c77f80a50959f5534b65274
Summary:
Release note: none
The effects that get applied when serializing an optimized function should include the join of all the normal paths through the function, not just the common prefix.
Closes https://github.com/facebook/prepack/pull/1727
Differential Revision: D7578528
Pulled By: hermanventer
fbshipit-source-id: c2655bfaed30224eb63a27a3335fc239e4d3b86f
Summary:
Release notes: none
Some internal changes to ensure `Bootloader.loadModules()` global stays as is and doesn't get destructured. Also changed the incorrect usage of `context.$Realm.generator.derive` in the mocks.
Closes https://github.com/facebook/prepack/pull/1726
Differential Revision: D7578301
Pulled By: trueadm
fbshipit-source-id: dd7dce3e46ff01b10eb9206a1ed5d94871ae93fd
Summary:
This is the equivalent of #1680 but for assignments.
It's pretty much the same changes, except this one emits assignments.
I also had to implement assignment to topVal.
Closes https://github.com/facebook/prepack/pull/1703
Differential Revision: D7546923
Pulled By: sebmarkbage
fbshipit-source-id: d0e2c356496c009b7f5865ba69784556ab9753d8
Summary:
Release notes: None
We keep track of initialized modules via an internal `__initializedModules` object.
However, after state joins and conditional `require()` calls, some of the
initialized module values `mightHaveBeenDeleted()`, indicating that they
might or might not have been initialized.
In that case, we should treat them just as other uninitialized modules.
Added regression test.
Closes https://github.com/facebook/prepack/pull/1713
Differential Revision: D7528844
Pulled By: NTillmann
fbshipit-source-id: b418a61e53b851f3ab385718c536e4c43fbbd170
Summary:
Release note: Better support for o[p] where p is unknown and the runtime value denotes a property on the prototype.
Partially fixes issue: #1575
When a known object, say ob = { x: 1, y: 2}, is accessed with an abstract property name (p), we currently emit a cascade of conditions of the form "p === 'x' ? 1 : (p === 'y' ? 2 : undefined)". If there was previously a write the object with an abstract property name (q), say ob[q] = 3, the undefined case is replaced with "p === q ? 3 : undefined" and the 1 is replaced with "q === 'x' ? 3 : 1" and so on.
If this seems very painful and inefficient to you, I can only agree, but note that it handles intermediate object states because the expression is a-temporal and is derived from the state of the object at a particular point in time. This is kind of important.
Nevertheless, this is not exactly correct either, hence issue 1575. The problem is basically that the undefined case at the end is wrong if the object has a prototype and the prototype has a property with name: (runtime value of p) and the object itself has not.
Fixing this is proving to be tricky and I think it is best get there in baby steps.
The first such step is to replace the undefined value in the expression above with the temporal expression ob[p]. This will run up the prototype chain rather than just return undefined.
Note, however, that this is just less wrong and not an actual solution. The problem is that by the time ob[p] is evaluated, ob may have a different value for p than it had at the temporal point where the expression was encountered. (The reason being that the final version of ob is the only one that actually gets serialized.)
I have some solutions in mind (Sebastian talks about some of them in the issue) but they are sufficiently complicated that I do not want to include them in this pull request, so please bear with me for while longer.
Closes https://github.com/facebook/prepack/pull/1702
Differential Revision: D7519459
Pulled By: hermanventer
fbshipit-source-id: 6387ab0a2be893eec205373c2c53c2f636760bf0
Summary:
Release notes: None
The issue seems to have been fixed by pull request #1660.
This adds a regression test.
Closes https://github.com/facebook/prepack/pull/1706
Differential Revision: D7518715
Pulled By: NTillmann
fbshipit-source-id: eab46243919283409681a89ffaa162e7a9e4074e
Summary:
Release note: none
If the x and y part of a c ? x : y are simplified under the assumption that they will only be used contexts where they are converted to Booleans, then it is not safe to use them in equality comparisons. Hence the simplification rules that do so must be disabled in this situation.
Closes https://github.com/facebook/prepack/pull/1698
Differential Revision: D7511274
Pulled By: hermanventer
fbshipit-source-id: f0329c00523cb3c06de9e0dde3ab55104dbb07c7
Summary:
Steals test case from #1626.
`__optimize` calls now search for newly registered functions. This should allow more test cases, which are all included with the diff.
This also changes the serializer to serialize additional functions when they're first encountered in the serializer instead of at the end.
Closes https://github.com/facebook/prepack/pull/1660
Differential Revision: D7491497
Pulled By: cblappert
fbshipit-source-id: b4b88100d7aa4b8c25dddabb1f82189dd9848fc5
Summary:
Release note: Fix problem where the results of To.ToObject were lost during serialization
Fixes issue: #1595
The serializer will now serialize the abstract result of calling To.ToObjectPartial on an abstract value as "Object.assign(abstract_value)". In the one case where the serialized code is known to implicitly convert the abstract value to an object anyway, the caller of To.ToObjectPartial strips away the wrapper in order to avoid the redundant explicit conversion.
Since I ended up looking at every call of To.ToObjectPartial, I used this an opportunity to merge it into To.ToObject.
I also made AbstractObjectValue.$GetPrototypeOf aware of the wrapper and made it return concrete values if the type of the wrapped primitive is known.
Closes https://github.com/facebook/prepack/pull/1690
Differential Revision: D7490354
Pulled By: hermanventer
fbshipit-source-id: 75146883d8b949c4e3e7848bd2e030f7b7c4d147
Summary:
Release notes: None
This partially addresses #1640, allowing for read-access to this.
However, just as for any other incoming abstract values, write-accesses are not yet allowed.
Adding regression test.
Closes https://github.com/facebook/prepack/pull/1641
Differential Revision: D7473440
Pulled By: NTillmann
fbshipit-source-id: 140a3c97aa0e3c4f52ef09134b0e43e7f7f84f7d
Summary:
This adds havocing in the right places that was not done properly in the previous solution.
This also allows non-self Receivers. For simple objects, this is allowed because they can't possibly have any meaningful impact if the object is truly simple since only getters expose the receiver concept.
More than that we can also make an assumption for non-simple objects, in pure mode. I assume that if the receiver is not self then the only reason we got here is because we concluded that anything else on the prototype chain cannot have had this property key on it. For example, that happens with a ToObject coercion. The original values doesn't have any properties on it.
A fix to #1675 might need to account for this scenario because currently we never propagate up the prototype chain so this can never happen.
However, even if there might be something else shadowing this on the prototype chain, this property access will still be ok since the residual is serialized as a property access on the receiver - which is havoced.
Fixes#1676
Closes https://github.com/facebook/prepack/pull/1680
Differential Revision: D7460594
Pulled By: sebmarkbage
fbshipit-source-id: e71dee4a6edf9b8a32a09016e7ac455ad03ad792
Summary:
Release note: More aggressive simplification of !e expressions
Since only the truthiness of e matters, it can be simplified more aggressively than e can by itself. To do this, one uses Environment.GetConditionValue rather than Environment.GetValue.
Closes https://github.com/facebook/prepack/pull/1688
Differential Revision: D7460139
Pulled By: hermanventer
fbshipit-source-id: 1ac2b799107e9743832979e29f8a79954ac666a8
Summary:
Release note: none
The basic change here is to use path conditions when simplifying conditional expressions. Doing so led to an explosion of work in the simplifier, so I had to introduce more flags to reign in the simplifier and preventing it from trying to simplify things that have already been simplified.
Closes https://github.com/facebook/prepack/pull/1684
Differential Revision: D7457002
Pulled By: hermanventer
fbshipit-source-id: 30ecde01c23b52871cdf43b24bf9ab9c5eecff4f
Summary:
Release note: Simplification now works better when == and != are used.
The main goal of this PR is simplify away unnecessary object creations such as the one in the test case.
Working on this, however, I ran into cases where tweaks to the simplifier would cause "push false" invariant failures. This time around I could not find a way to avoid it, so the time has come to exploit it. Consequently, the biggest part of this PR is to recover when we detect a contradiction by reverting to behavior we would have had, had we known that assuming the condition is true (or false) is actually invalid.
While there is a lot of code, the transformations are pretty simple.
Closes https://github.com/facebook/prepack/pull/1679
Differential Revision: D7452855
Pulled By: hermanventer
fbshipit-source-id: 507f0a1a6dbe9432ad5654e488214b8da7f7197c
Summary:
Release note: Stop generator entries from being serialized twice
Fixes issue: #1627
When the effects of optimized function is reduced to a generator, care must be taken to avoid duplicating the normal path of a function that could terminate with an exception.
It is a bit unfortunate that the effects has this duplication in there in the first place, but I was not able to figure out a way to avoid this and there are more pressing matters to attend to.
Closes https://github.com/facebook/prepack/pull/1670
Differential Revision: D7431202
Pulled By: hermanventer
fbshipit-source-id: 081713419c7cd46b77c76068fba89c62f49dea70
Summary:
Fixes#1598
This is actually already allowed to be abstract since all the internal objectvalue properties can be abstract and there are tests for it. This PR just makes it a bit more explicitly handled and properly typed.
I also need this so I can write more tests around abstract prototype chains so I don't break the code paths that already are designed to support them.
I also did a pass to search for other instanceof ObjectValue checks but I didn't find any obviously similar issues.
Closes https://github.com/facebook/prepack/pull/1603
Differential Revision: D7440114
Pulled By: sebmarkbage
fbshipit-source-id: 45169e8eb5afc6de2de7a7f1f4a4c6076fe7e2ab
Summary:
Release notes: none
I've added more tests that used to fail, but now pass with recent changes to master. Let's keep these tests as regressions tests.
Closes https://github.com/facebook/prepack/pull/1580
Differential Revision: D7414055
Pulled By: trueadm
fbshipit-source-id: 0b53d711b7ffd1de75335071afe8eba14fcae2f8
Summary:
Release notes: none
Adds more tests to ensure we don't regress in some cases.
Closes https://github.com/facebook/prepack/pull/1649
Differential Revision: D7392527
Pulled By: trueadm
fbshipit-source-id: 0b34bfd741a5de7ea434f6e3c30f15dab983482e
Summary:
Release notes: none
This adds a really cool feature – dynamic ReactElement type. As per a conversation in Messenger yesterday by gaearon, it got me thinking about how we might "simplify" recursively on a conditional in complex cases and I decided it was probably worth adding to inline components. This PR makes it possible to inline component trees without having to break out when the type is an abstract conditional.
Closes https://github.com/facebook/prepack/pull/1647
Differential Revision: D7392497
Pulled By: trueadm
fbshipit-source-id: 170ae4829bd407410d8882c874144516cfd01190
Summary:
This fixes a React test to be deterministic.
Closes https://github.com/facebook/prepack/pull/1636
Reviewed By: trueadm
Differential Revision: D7366209
Pulled By: gaearon
fbshipit-source-id: c944d6ab638d4914d2a5eab9e341186755264ec1
Summary:
Release notes: none
This PR adds nested optimized function support and bug fixes around them to the React reconciler.
The tests added that fail have been skipped for now until we have the ability to properly serialize nested optimized functions.
Closes https://github.com/facebook/prepack/pull/1614
Differential Revision: D7366102
Pulled By: trueadm
fbshipit-source-id: c18c5a470a31b905a76155e006767f8ec7a73afa
Summary:
Release notes: None
This fix for #1250 ensures that Prepack properly emits global let bindings,
which was broken in particular in the presence of optimized functions.
(Looking at the code, the whole approach of rewriting residual bindings
while processing optimized values seems quite brittle, but changing that
is for a later pull request.)
Closes https://github.com/facebook/prepack/pull/1631
Differential Revision: D7359200
Pulled By: NTillmann
fbshipit-source-id: 3fcb84e2672dc913c8dd217375de7744cc361746
Summary:
Release note: Allow subsequent mutating of Object.assign() arguments by snapshotting them
Fixes issue: #1480
The normal mode for Prepack is to run to the end of global code and then serialize the objects as they are at that point. This causes problems if objects are passed to functions that must be called at particular temporal points, since the calls must receive the objects as they are at that point, rather than as they are at the time when Prepack serializes them.
To solve this conundrum, there is now a way to get a snapshot of an object at a particular temporal point. This works by creating a new object and transferring the properties from the original object to the new object. Things are arranged so that the two objects are always referred to by the same name in the generated code. The nice end result is that subsequent modifications to the object does not affect the object that is passed to the function call where the snapshot was taken.
Following the snapshot, the original object has no own properties and must be havoced or marked simple partial, depending on the circumstances at the temporal point where the snapshot is taken.
The latter case arises, for example, when the target object of a call to Object.assign has been snapshotted because one of the other arguments is partial or fully abstract. Following the call, the object can still be used and all property references result in abstract values with runtime lookup behavior. It is also possible to assign new properties to the object and Prepack will track the values of those properties.
In order to deal with the fact that multiple compile time objects now correspond to a single runtime object, the first snapshot gets its own named temporal abstract value and the original object is associated with this object and the name of the latter becomes the name of the original. Once an object gets a name, all further writes to the object results in runtime assignments at the temporal point of the write.
Consequently subsequent snapshots just return this first snapshot, since the runtime object get incrementally updated. The property references are still cleared out, since snapshotting an object implies that it going to be modified in unspecified ways.
Closes https://github.com/facebook/prepack/pull/1563
Differential Revision: D7347283
Pulled By: hermanventer
fbshipit-source-id: 618873abf99bcadc4c74794aa7804a5ac5d04c12
Summary:
Release notes: Removing --abstractEffectsInAdditionalFunctions option
It is now the new default, as nothing seems to depend on the old behavior.
Closes https://github.com/facebook/prepack/pull/1623
Differential Revision: D7342083
Pulled By: NTillmann
fbshipit-source-id: c02d13bddd0ff3d8a113a6a5a83c1bf35c96ba4d
Summary:
Solve #886
`ClosureRefVisitor` tracks the depth of the traversing, and sets the `usesArguments`/`useThis` flags only if they were used in the top level function (depth = 1).
Closes https://github.com/facebook/prepack/pull/1618
Differential Revision: D7342100
Pulled By: a8m
fbshipit-source-id: e12afec542a5afa67625b03ee615ebf9f065af74
Summary:
Release note: Fix bug that caused variables to get used before their definitions have been processed.
Resolves: #1585
Following an code sequence that may terminate abruptly, or alternatively carry on normally, the generator that preceded the fork must be emitted into its parent and replaced with a new generator, before carrying on with the normal execution, since the normal execution is effectively nested inside the normal branch of the fork.
Note that this only occurs after the second such fork, which is why the bug was so elusive.
Closes https://github.com/facebook/prepack/pull/1619
Differential Revision: D7338429
Pulled By: hermanventer
fbshipit-source-id: e7463d87d20bde4f13bd6e69e0afe20c548f07c0
Summary:
Additional Functions duplicates the Effects traversing logic in the serializer and visitor. Consolidate this logic as well as allowing for ordering dependencies by putting all these things needing visiting/serialization into the generator. Adds new `ModifiedPropertyEntry` and `ModifiedBindingEntry` types.
Closes https://github.com/facebook/prepack/pull/1559
Differential Revision: D7311062
Pulled By: cblappert
fbshipit-source-id: e789d48f76879fdee3dd1187b53ba15377d7dcb7
Summary:
Release note: Added more logic to simplifier
The simplifier was ignoring the "isCondition" flag in some cases, which broke symmetry.
Also added this rule: (c ? x : (c || false)) => c
Test cases to come later.
Closes https://github.com/facebook/prepack/pull/1594
Differential Revision: D7293426
Pulled By: hermanventer
fbshipit-source-id: 5d42c0dcb29161c8186ce0964714e6ba1b7c67ae
Summary:
Release notes: None.
Unfortunately, the abstract recursion check seems to conservative, as it triggered on some large internal tests even though there was no issue.
Disabled test test/serializer/abstract/Fibonacci.js that now fails.
I created an issue to track this.
Closes https://github.com/facebook/prepack/pull/1587
Differential Revision: D7282804
Pulled By: NTillmann
fbshipit-source-id: 4fc924b109c66c3237b14ace4d60d03f994b3715
Summary:
Release notes: none
It's common for a `new Something()` call to exist in pure wrappers where `Something` is an abstract value. Currently, this fails with a FatalError with no recovery. This PR adds a recover for this when in a pure wrapper so we can continue evaluating.
Closes https://github.com/facebook/prepack/pull/1590
Differential Revision: D7271876
Pulled By: trueadm
fbshipit-source-id: fc5192af6b356b7df9eb2f147bd8b53fe3790942
Summary:
Release note: Simplify conditions like c ? {} : null to just c if used in a condition context
Resolves: #1582
The simplifier already simplifies c ? true : false to just c. It can also do this for truthy values that are known to convert to true and false, if the result of the conditional is used as a condition.
Closes https://github.com/facebook/prepack/pull/1583
Differential Revision: D7261552
Pulled By: hermanventer
fbshipit-source-id: cf2c16ffd1dd99a23ea9cf4f8065d0f495b5d749
Summary:
Release notes: none
firstRender can fail when `Function.prototype.bind` is passed around because the firstRender removes the class component entirely, along with any concept of state etc. To prevent outputting the incorrect value, we now check for bind and if we're given something that is completely abstract as a child from a render, we bail out in that case too.
Closes https://github.com/facebook/prepack/pull/1573
Differential Revision: D7248829
Pulled By: trueadm
fbshipit-source-id: eebd7e6565352a37026928a20944377bb06129f5
Summary:
Release Notes: Allow additional functions to return PossiblyNormalCompletions.
Similar to the logic in `Program.js`, we need to emit an `if` statement at the end of an additional function that may return or throw.
In `functions.js` we process the PossiblyNormalCompletion, joining the consequent and alternate effects and creating a set of args + buildnode for emitting the conditional throw. Then where we would originally serialize/visit the return value, we will instead apply the joined effects and emit the conditional throw/return.
Closes https://github.com/facebook/prepack/pull/1523
Differential Revision: D7238882
Pulled By: cblappert
fbshipit-source-id: fd51bb4998394930960a70c8ef40b574e3244026
Summary:
Release notes: none
Found an issue on REPL: https://goo.gl/Qz46FJ
This fixes it along with a test.
Closes https://github.com/facebook/prepack/pull/1564
Differential Revision: D7231968
Pulled By: trueadm
fbshipit-source-id: c3ceff7d345f1a48e6fdeccfeaed6740383fbd40
Summary:
Release notes: none
First we need to ignore the differences in properties of newly created objects when we compare. Since widening doesn't preserve any knowledge of newly created objects (they just go to topVal), we don't have to be smarter than this.
Second, since all effects including property mutation of newly created objects gets reverted, this always yields empty objects. We need to apply the effects of newly created objects so that the serializer knows how to serialize these objects.
We should also avoid emitting property assignments to these objects since we've already applied them to the object value.
Closes https://github.com/facebook/prepack/pull/1560
Differential Revision: D7229726
Pulled By: sebmarkbage
fbshipit-source-id: f699028a01348934dc0e039263d48a8c3439870a
Summary:
Release notes: none
Adds React.createContext support to the React compiler. Updates Yarn dependencies to use React 16.3 pre-release version so tests can validate new API works with the compiler.
Closes https://github.com/facebook/prepack/pull/1548
Differential Revision: D7209302
Pulled By: trueadm
fbshipit-source-id: fff360ee19d4d9fee6e27b738658d9076449da4b
Summary:
Release notes: none
When we evaluate multiple additional functions, they might remove other additional functions that were defined as additional functions. This causes a serializer issue. If we check if they have info from the visitor we can avoid hitting an invariant. This fixes one of the issues flagged in https://github.com/facebook/prepack/pull/1550, thus pulling in the test that was failing from the PR.
This PR is rebased on https://github.com/facebook/prepack/pull/1561
Closes https://github.com/facebook/prepack/pull/1555
Differential Revision: D7220598
Pulled By: trueadm
fbshipit-source-id: 2d0b87e23ecd93b325f44daa3a7c21ff3591d733
Summary:
Release notes: none
When rendering a class component on first render, we need to ensure it's instance is intrinsic otherwise it will serialize out incorrectly.
Closes https://github.com/facebook/prepack/pull/1556
Differential Revision: D7220974
Pulled By: trueadm
fbshipit-source-id: 968afb5b7dbfc6f7714862d9cbedf6e793c9e504
Summary:
Release notes: `__registerAdditionalFunction` is now `__optimize`
This PR renames `__registerAdditionalFunction` to `__optimize` and removes all tests that use the `// additional functions` logic in favor of declaratively using `__optimize`. Also the serializer option for `additionalFunctions` is removed too.
Closes https://github.com/facebook/prepack/pull/1561
Differential Revision: D7219751
Pulled By: trueadm
fbshipit-source-id: 7984496b3a68db3194fb3696839cb11fdd0b7843
Summary:
Release notes: none
This PR fixes an issue where a StringLiteral that contained quotes, which resulted in being escaped on output, should have been wrapped in JSXExpressionContainer, otherwise the output becomes invalid.
Closes https://github.com/facebook/prepack/pull/1549
Reviewed By: trueadm
Differential Revision: D7207620
Pulled By: gaearon
fbshipit-source-id: d783475e507f65d0dc20678cc07e1b694397d0c1
Summary:
A WIP attempt that might not be right – but to help with the discovery of the right solution for #1537.
Closes https://github.com/facebook/prepack/pull/1538
Differential Revision: D7188125
Pulled By: trueadm
fbshipit-source-id: eed37afd3707feaffe779f50ff82b222d2abfd28