Commit Graph

1568 Commits

Author SHA1 Message Date
Caleb Meredith
4027b168c3 Don’t record modified bindings for immutable bindings when havocing (#2443)
Summary:
Fixes #2442 by not marking immutable bindings as modified. Its very possible that my fix is incorrect. There’s a bunch of related logic for `binding.hasLeaked`/`binding.mutable`/`modifiedBindings` and I certainly didn’t find them all to get a holistic view of what’s happening. From the code I did look at, this fix seems to be safe since there’s no way for a havoced function to change an immutable binding.

I ran into this issue while trying to compile our internal React Native bundle with the React Compiler.

Let me walk through this repro to the invariant.

```js
const havoc = __abstract("function", "(() => {})");

global.result = __evaluatePureFunction(() => {
  const b = __abstract("boolean", "true");

  if (b) {
    var g = function f(i) {
      return i === 0 ? 0 : f(i - 1) + 1;
    };
    havoc(g);
  } else {
    return;
  }

  return g(5);
});
```

1. Create a function expression that refers to itself inside a construct that wraps the evaluation in `evaluateForEffects()`. (I used an if-statement.)
2. Havoc the function expression. This will add the function’s reference to itself to `modifiedBindings`. However, the original [function expression skipped (3rd arg)](3eb1e8e8ea/src/evaluators/FunctionExpression.js (L108)) adding its binding to `modifiedBindings`.
3. Outside of `evaluateForEffects()` call the function.
4. Observe the invariant firing:

3eb1e8e8ea/src/environment.js (L339)

According to `modifiedBindings`, we only initialize the function expression _after_ its definition. So if the function expression wants to recursively refer to itself, it can’t since the effects we’ve recorded/applied puts us in a state where the binding is not available.
Pull Request resolved: https://github.com/facebook/prepack/pull/2443

Reviewed By: trueadm

Differential Revision: D9447861

Pulled By: calebmer

fbshipit-source-id: de4460bd184012416e80eb65f86dae7ade9dc34e
2018-08-22 06:54:28 -07:00
Caleb Meredith
fcdb6ebbe6 typeof React.forwardRef is not function (#2425)
Summary:
`typeof React.forwardRef(...)` is `object` but Prepack was returning `function`. This is particularly important since React Native’s `Animated.createAnimatedComponent` crashes with an invariant if `typeof` is `function`. We can’t compile our React Native internal bundle without this fix.

22cf5dc566/Libraries/Animated/src/createAnimatedComponent.js (L20-L25)

f9358c51c8/packages/react/src/forwardRef.js (L12-L43)

Small Prepack core change in `src/evaluators/UnaryExpression.js`.
Pull Request resolved: https://github.com/facebook/prepack/pull/2425

Reviewed By: trueadm

Differential Revision: D9447593

Pulled By: calebmer

fbshipit-source-id: de1e9101f368ea52c8c17c31d04a2e4e0a38dd6b
2018-08-22 04:54:04 -07:00
Caleb Meredith
0b813f174e Fix nested for statement bailout with nested for-in (#2438)
Summary:
Fixes #2437.

The code to handle for-statement bailouts was failing on nested for-in/for-out statements since while they had a variable declaration node in their AST the variable declaration behaves very differently. Notably there must be exactly one declaration, the declaration may not have an initializer, and the declaration may only be replaced by a `LeftHandSideExpression` not a expression or statement.

This PR does two things:

1. Replace the variable declarations in for-in/for-of statements with a `LeftHandSideExpression` which will then perform `DestructuringAssignment` with loop values in the evaluator.
2. Recognize that a `LeftHandSideExpression` in a for-in/for-of expression modifies bindings in the serializer residual function visitor.
Pull Request resolved: https://github.com/facebook/prepack/pull/2438

Reviewed By: trueadm

Differential Revision: D9446786

Pulled By: calebmer

fbshipit-source-id: e7f13a3c722602270a87cf8d933de1283b8121d1
2018-08-22 03:24:50 -07:00
Caleb Meredith
18fc229d44 Add throw-away generator to evaluateWithoutEffects (#2436)
Summary:
Fixes #2432.

`evaluateWithoutEffects` sets the realm generator to `undefined`. However, I found a case in #2432 where a generator is required in `evaluateWithoutEffects`.

What happens is that we build an error stack in `evaluateWithoutEffects`.

3eb1e8e8ea/src/realm.js (L1607-L1612)

As a part of building that error stack we try to get `EventEmitter.name`. However, `EventEmitter` is havoced! So we want to create an abstract temporal value `EventEmitter.name`. But creating a temporal value requires a generator since we need to add a generator entry.

3eb1e8e8ea/src/methods/properties.js (L1361-L1367)

This PR solves the issue by adding a throw-away generator to `evaluateWithoutEffects`. Other strategies might be a bit complicated since `evaluateWithoutEffects` is a very uncommon case. Removing `evaluateWithoutEffects` is also not a great option since in this case we would add a temporal `EventEmitter.name` when we never actually use it.
Pull Request resolved: https://github.com/facebook/prepack/pull/2436

Reviewed By: trueadm

Differential Revision: D9446792

Pulled By: calebmer

fbshipit-source-id: 14b36da1fb39dc9ab2a166ecb02d0f062b2bdffe
2018-08-22 03:24:50 -07:00
Sapan Bhatia
3baf6e373f Simplify widened array index access test (#2453)
Summary:
This test bedeviled me for quite some time because I thought it was testing for aliasing effects - it isn't. This is my fault, since I posted this in my repro. Aliasing effects are still to be dealt with in nested optimized functions. Anyhow, simplifying to prevent somebody else from having this confusion.
Pull Request resolved: https://github.com/facebook/prepack/pull/2453

Reviewed By: trueadm

Differential Revision: D9420733

Pulled By: sb98052

fbshipit-source-id: 755ccd90b1e64afdfe6ec627a3306e8baf5952ea
2018-08-21 04:13:20 -07:00
Dominic Gannaway
d4a0c1ee48 Allow arrays with widened numeric properties to update index properties (#2450)
Summary:
Fixes #2447 by adding a separate codepath for when we encounter arrays with widened numeric properties, otherwise we hit an invariant when trying to deal the `length` property (which always remains abstract for arrays with widened numeric properties).
Pull Request resolved: https://github.com/facebook/prepack/pull/2450

Differential Revision: D9402265

Pulled By: trueadm

fbshipit-source-id: 772634bd0a69af23410e65c63b89cbfcc89d8b5b
2018-08-20 11:24:05 -07:00
Sapan Bhatia
7d0aa2dd87 Fixed duplicate uses of PP0038 (#2445)
Summary:
Fixes #2439.
Pull Request resolved: https://github.com/facebook/prepack/pull/2445

Differential Revision: D9397648

Pulled By: sb98052

fbshipit-source-id: 5d74d47efd0e1f5ea29ec877f0747119a0ed9694
2018-08-20 04:25:02 -07:00
Sapan Bhatia
6f7161ecb3 Clarify terminology for Leaked Value Analysis (#2444)
Summary:
This PR continues the break up of #2185 into smaller pieces, and makes the use of Leaking, Havocing and Materialization consistent with agreed upon terminology.

*Leaking* transitively adds mutable locations to the environment. Accessing such leaked locations is equivalent to accessing to the environment.

*Havocing* takes on its usual meaning in static analysis: to set an abstract value to "unknown" i.e. topVal.

*Materialization* snapshots locations into runtime assignments for the purpose of outlining functions.

Presently, leaking always leads to havocing, and materialization. But in addition, it also marks a location or an object as "leaked" implying that it may be aliased and/or no longer be "well behaved" (for example, a leaked object may leak any object passed to one of its methods).
Pull Request resolved: https://github.com/facebook/prepack/pull/2444

Differential Revision: D9392730

Pulled By: sb98052

fbshipit-source-id: 0ef987be8a71bb74e2def0526caf203bbfc34d82
2018-08-18 08:57:14 -07:00
Caleb Meredith
3eb1e8e8ea Fix havoced binding not in optimized function (#2420)
Summary:
Fixes #2419 and #2386. I ran into this issue while testing an internal React Native bundle. This invariant assumes that all property bindings emitted in a pure scope are also in an optimized function. This is not true for `__evaluatePureFunction` which the React Compiler wraps around initialization code so that globals outside of the closure are not havoced.
Pull Request resolved: https://github.com/facebook/prepack/pull/2420

Differential Revision: D9363247

Pulled By: calebmer

fbshipit-source-id: 4f1634165b6fe15f95b5b7332432fccacc596821
2018-08-16 11:26:16 -07:00
Nikolai Tillmann
c18bd7ee97 Adding TODO # (#2429)
Summary:
Release notes: None
Pull Request resolved: https://github.com/facebook/prepack/pull/2429

Differential Revision: D9352869

Pulled By: NTillmann

fbshipit-source-id: 50da5a16aed04dcd376974d433290159f9e698c8
2018-08-15 17:09:29 -07:00
Herman Venter
6a938efac1 New Prepack alpha
Summary: 0.2.49-alpha.0

Reviewed By: cblappert

Differential Revision: D9349878

fbshipit-source-id: b351b44ff3400a64ec19e8eaa0778ba718e525ef
2018-08-15 16:24:55 -07:00
Herman Venter
90c3aa28cc Weekly release v0.2.48: Fuzz tester and join logic refactor
Summary:
Fixes #2151 #2222 #2279 #2393 #2399 #2404 #2411 #2414 #2415
Added a fuzz testing tool
Added test cases
Turn crash in JSON.stringify into a diagnostic
Adds a `arrayNestedOptimizedFunctionsEnabled` flag to enable nested optimized functions derived from Array.prototype methods (like `map`) and Array.from
Refactor assignment on partial or possibly deleted property
Rewrote the joining logic to always do a full join at every join point
Removed last remnants of delayUnsupportedRequires

Reviewed By: cblappert

Differential Revision: D9349841

fbshipit-source-id: 74a16dbb015777d59d23fdfde77abbe2489c292a
2018-08-15 16:24:54 -07:00
Nikolai Tillmann
9f62eeabc6 Added TODO #. (#2427)
Summary:
Release notes: None

Did a little bit of prettying up the code in _getTarget,
renaming additional to optimized, and added a TODO #2426
annotation where issue #2426 happens in the code.
Pull Request resolved: https://github.com/facebook/prepack/pull/2427

Differential Revision: D9350285

Pulled By: NTillmann

fbshipit-source-id: 56f1c7ab1a62ca39091d06c91c0700d0cad447ad
2018-08-15 15:12:35 -07:00
Herman Venter
14bbf4e595 Make Prepack understand latest module polyfill
Summary: The require method is now __r.

Reviewed By: NTillmann

Differential Revision: D9329676

fbshipit-source-id: 0c3385d593abcf93c5bb3116273b1e128b9e2950
2018-08-15 12:39:24 -07:00
Nikolai Tillmann
a764933bc6 Making sure ThrowTypeError intrinsic has intrinsic name (#2424)
Summary:
Release notes: None

Every native function value needs an intrinsic name,
otherwise the serializer will fail with an invariant violation.

This gives an intrinsic "name" to `ThrowTypeError`
that meets the requirements of the ECMAScript spec.
Pull Request resolved: https://github.com/facebook/prepack/pull/2424

Differential Revision: D9344248

Pulled By: NTillmann

fbshipit-source-id: c0e00652c50a555e7c43009d1394c969fbfff9b1
2018-08-15 11:44:54 -07:00
Nikolai Tillmann
55c55f70c9 Removing last remnants of delayUnsupportedRequires
Summary:
The previous changes on "Do full joins" didn't remove all
uses of the delayUnsupportedRequires feature. This does it.

In particular, this fixes the internal prepack-test.

Reviewed By: hermanventer

Differential Revision: D9287509

fbshipit-source-id: f810f5e08a10cbd14272d776799a2a8f1442d1c8
2018-08-12 17:54:27 -07:00
Herman Venter
7157849a44 Do full joins (#2402)
Summary:
Release note: Rewrote the joining logic to always do a full join at every join point

Closes #2151 #2222 #2279

I've spent a lot of time in the last few months trying to sort out problems that arise from effects being applied too many or too few times. Fixing these feel a bit like playing wack a mole and in the end no fix goes unpunished.

Stepping back a bit from the fray, it seems to me that the root cause of all this pain is the fact that joins of different kinds of completions get delayed.

Before we had path conditions and the simplifier this seemed like a rather good thing since exceptional paths did not contribute values to the normal paths and we thus had fewer abstract values to deal with and fewer places where Prepack would grind to a halt.

In the current state of things, however, it seems perfectly possible to join in all branches at every join point. I've had to decrease some limits, in particular the number of times we go around a loop with conditional exits. I've also had to make the test runner impose a limit on how many times the simplifier can invoke Path.implies.

Nevertheless, the tests seem to pass and hopefully this will also fix quite a lot of bugs that have been unresolved for many months already.
Pull Request resolved: https://github.com/facebook/prepack/pull/2402

Differential Revision: D9236263

Pulled By: hermanventer

fbshipit-source-id: 92a25b591591297afeba536429226c5a0291f451
2018-08-11 20:53:43 -07:00
Sebastian Markbage
671ea300ba Refactor assignment on partial or possibly deleted property (#2389)
Summary:
The goal of this refactoring is to move more of the special case logic around properties as deeply as possible so that when we expand the object model, we don't have to add special cases through all the indirections. This will be clear in a follow up where I try to model the multiple identities.

This reverts the fix in #1183. That fix is no longer needed for its original purpose because those getters are now modeled as pure and the unnecessary accesses gets eliminated by the serializer.

I also remove the `Receiver.$Delete(P)` hack in `OrdinarySet` when a property might have been deleted. I'll need that because this hack doesn't work on objects that only allow weak deletions (such as object set templates). Instead, I model possibly deleted properties in ValidateAndApplyPropertyDescriptor.

It turns out that much of it doesn't matter if it has been deleted or not since you can't delete a non-configurable property.

The React environment calls Set/DefineOwnProperty on partials during initialization. These must read to get the current descriptor. However, a temporal read of the actual value can't be emitted at this point. So I emit a placeholder value that shouldn't be serialized. This is something the environment initialization must already consider. We can't have it read partial values before there is an environment to consider.
Pull Request resolved: https://github.com/facebook/prepack/pull/2389

Differential Revision: D9274895

Pulled By: sebmarkbage

fbshipit-source-id: b8e8aee697121552243f7a67b7c19c62768f4729
2018-08-10 13:24:35 -07:00
Dominic Gannaway
abe84227f5 Adds a flag for Array.prototype method nested optimized functions (#2404)
Summary:
Release notes: adds a `arrayNestedOptimizedFunctionsEnabled` flag to enable nested optimized functions derived from Array.prototype methods (like `map`) and Array.from

This PR puts the existing (unstable) work for Array prototype methods behind a flag. The flag is enabled by default in React and serializer tests.
Pull Request resolved: https://github.com/facebook/prepack/pull/2404

Differential Revision: D9272747

Pulled By: trueadm

fbshipit-source-id: d7e53656a12cd6cff680a9ef0e2580a93d56e34e
2018-08-10 11:40:19 -07:00
Dominic Gannaway
fbf3d0a639 Remove React pathCondition resetting (#2415)
Summary:
Release notes: none

During React reconciliation we wrongly reset `pathConditions`. This was done originally because of a pushing false invariant, that I incorrectly thought was a bug because of React specific code.

However, today, I re-worked the original test code to remove all React logic and I found that the invariant occurs even without React specific logic. This strongly suggests that this is still an existing bug in Prepack and adding `pathConditions` logic, which gets around it, was the wrong strategy and should be reverted.
Pull Request resolved: https://github.com/facebook/prepack/pull/2415

Differential Revision: D9264730

Pulled By: trueadm

fbshipit-source-id: 2d3703e638c894eb0929b9a30a7d74ccbd998ea3
2018-08-10 07:01:37 -07:00
Dominic Gannaway
47d074ca20 Ensure createdObjects is not broken (#2414)
Summary:
Release notes: none

This adds a check in the React utils component function caller to ensure that the `createObjects` are correct at evaluation time.
Pull Request resolved: https://github.com/facebook/prepack/pull/2414

Differential Revision: D9264727

Pulled By: trueadm

fbshipit-source-id: 6435bf1a4ccfaf98278377ad48147a557c1608d4
2018-08-10 06:40:45 -07:00
Sapan Bhatia
9fb1e34722 Turn crash in JSON.stringify into introspection error (#2412)
Summary:
Release notes: None

Fixes #2411
Pull Request resolved: https://github.com/facebook/prepack/pull/2412

Differential Revision: D9259046

Pulled By: sb98052

fbshipit-source-id: 392b6aa9bc840e72d61f26929e5cc110230c75d0
2018-08-09 18:39:32 -07:00
Chris Blappert
22a55f7585 Add test cases from 2358, 2398 that pass from previous PRs (#2410)
Summary:
Release Notes: None

Previous PRs fixed these issues, but didn't necessarily add the test case from the issue. This PR adds the exact test cases from the issues.

Closes #2358
Closes #2398
Pull Request resolved: https://github.com/facebook/prepack/pull/2410

Differential Revision: D9259118

Pulled By: cblappert

fbshipit-source-id: 3eb347044865a11e2ce5262b2338b7b6e1958c60
2018-08-09 18:39:32 -07:00
Dominic Gannaway
3503c793eb Fixes incorrect warning of side-effects in #2393 (#2394)
Summary:
Release notes: none

Fixes #2393. I created the test as a React test because I couldn't see any way of making a serializer test fail if there was a warning without making many other tests fail or without adding another `// some config` to the test (which I believe we said we would not add anymore as there's already too many). React tests will always fail on side-effect warnings, so this takes better advantage of this.
Pull Request resolved: https://github.com/facebook/prepack/pull/2394

Differential Revision: D9249714

Pulled By: trueadm

fbshipit-source-id: 0e11d13e575eca3b6bd75e437eb083b9294a3aee
2018-08-09 13:39:07 -07:00
Nikolai Tillmann
829bec26bf Updating Flow to .78 (#2400)
Summary:
Release notes: None
Pull Request resolved: https://github.com/facebook/prepack/pull/2400

Reviewed By: hermanventer

Differential Revision: D9244528

Pulled By: NTillmann

fbshipit-source-id: 159f38a0a7faa8e52ce87e489f66d940297c568d
2018-08-09 12:24:34 -07:00
Caleb Meredith
4319298f88 Fuzzer (#2374)
Summary:
This PR adds the fuzzer I’ve been working on to the Prepack codebase so other people can contribute, provide feedback, and run it against their changes. The new commands added are:

- `yarn fuzz`: Starts generating tests in the foreground and logs progress. If it finds an error it will try and shrink it before returning the shrunken program to you with the invalid results.
- `yarn fuzz-sample`: See a selection of the programs generated by the fuzzer.
- `yarn fuzz-overnight`: Spin up a worker for each CPU and try to find failing test cases. Your computer will be basically unusable while you run this, so leave it running overnight. Failed test cases will be saved in `fuzzer/overnight.sqlite` so in the morning you can use `sqlite3` to inspect the errors the fuzzer found.

The fuzzer generates programs with an optimized function and executes them twice:

1. In Node.js
2. In Node.js after running Prepack

Then compares the results. If the results are different then the fuzzer will attempt to shrink the program to something easier to debug and return this to you. See [this gist for a sample of generated programs](https://gist.github.com/calebmer/6a727c1f4aa8c08d51940e60d29d336a). Here’s an example of a function you might see:

```js
function f3(a1, a2, a3, a4, a5, a6) {
  2;
  var x2;

  if (0) {
    return a1 ? false : a2;
  } else {
    var x1 = a3;
    x2 = x1;
  }

  var x6;

  if (x2) {
    var x3;

    if (x2) {
      x3 = x2;
    } else {
      x3 = a4;
    }

    var x4 = x3;
    x6 = x4;
  } else {
    var x5;

    if (a5) {
      x5 = x2;
    } else {
      x5 = a6;
    }

    x6 = f2(x5);
  }

  return x6;
}
```

So far I’ve reported four bugs reduced from test cases found by this version of the fuzzer. I’ve reported a couple more from old fuzzers I used, but these four from the current version. The shrinking process is not that good and it takes a while as the generated program can get large, so you’ll usually have to do some manual shrinking to get good bug reports. I only ran `yarn fuzz-overnight` for about an hour. It found 28 failures and I reduced those down to these 4.

- #2354
- #2355
- #2361
- #2363

I expect I’ll find more bugs as these get fixed and I add more JavaScript features to the fuzzer. The features I currently have are:

- Scalar primitives (number, string, boolean, null/undefined)
- Functions
- Conditional expressions
- If statements
- Variable declarations

Not too many features, but enough to catch a handful of bugs.

> **Note:** If this PR is too much to review, I’ve created [`calebmer/prepack-fuzzer`](https://github.com/calebmer/prepack-fuzzer) in which my work is broken up into commits as I made them. I then copied these files over to the Prepack repo.

The fuzzer in this PR is a rewrite from the [fuzzer I started with](https://gist.github.com/calebmer/75dd75ebe556681d3a628e75eaffc403). The main lessons I learned from that one are that I should start with a general JS fuzzer instead of a React fuzzer (since adding JS features after the fact to fuzzer designed for React is difficult) and that all nested structures need to be generated with one recursive generator from the generator library I’m using.

To generate programs I use [`leebyron/testcheck`](https://github.com/leebyron/testcheck-js) which is actually a JS compiled version of the official Clojure library [`clojure/test.check`](https://github.com/clojure/test.check). `testcheck` is designed for generative property testing at a much smaller scale then program fuzzing. So I am abusing the library a bit to use it as a fuzzer. My reasoning is that I wanted to write the fuzzer in JS (to have access to the Babel AST) and I didn’t want to write my own case generating framework. If we outgrow `testcheck` then we can keep the DSL, but rewrite the generation/shrinking logic. Although its been working fine for me so far. (Yet I am using a forked version which simply uses some unpublished features in the `testcheck` repo.)

The generator code in `fuzzer/src/gen.js` may look odd to you. It uses immutable.js and a **state monad** implemented with a JS generator so the code looks imperative. I need state since generating various program components depends on things like “what variables are declared at a given point in time,” but because I’m limited to only using a single recursive generator (based on performance lessons I learned from my first fuzzer) I can’t pass around state at the generator level and must instead maintain state at the result level. At first I tried hacking together some imperative state, but when shrinking programs `testcheck` replays some generators to get new programs. So what do you do when you need a stateful process that needs to be replayed? You use a monad.

I could try to fix the bugs I found, but I’d like to find more bugs. I need to add back support for React components and I need to add more language features.

_Which JS language features are the most interesting to fuzz?_

I ranked all the kinds of AST nodes in our internal React bundle [and got this](https://gist.github.com/calebmer/be5e2bad4b12af683522096544fc9568). I’ll be starting with that, but the Prepack team has a better intuition around what’s good for fuzzing. I know there’s been a discussion around temporals recently that I haven’t really been following. What would be good ways to trigger this behavior?
Pull Request resolved: https://github.com/facebook/prepack/pull/2374

Differential Revision: D9180836

Pulled By: calebmer

fbshipit-source-id: 59d3fb59ecc1026a865672e2833f7482ed72139a
2018-08-09 10:39:23 -07:00
Chris Blappert
efc807f5f4 Fix tryGetOptimizedFunctionRoot to actually get optimized function root (#2401)
Summary:
Release Notes: None

Previously, `tryGetOptimizedFunctionRoot` would only get the root successfully if the root was one of the optimized functions or the parent of one of the residual functions. Now we look at the optimized functions as well as any of their parents. Added 2 test cases.

Fixes #2399
Pull Request resolved: https://github.com/facebook/prepack/pull/2401

Differential Revision: D9242153

Pulled By: cblappert

fbshipit-source-id: 9bdffce23d5dd19d9ce5a6b09e1ee9647ca6e5cb
2018-08-09 10:24:42 -07:00
Nikolai Tillmann
93d2026019 Alpha version bump
Summary: Alpha version bump

Reviewed By: cblappert

Differential Revision: D9230712

fbshipit-source-id: 0d0f8ad2ac27ef7bd7a3144a40b14ce1462b80b8
2018-08-08 16:09:37 -07:00
Nikolai Tillmann
13a5eeb30b Weekly release v0.2.47
Summary:
Weekly release v0.2.47:
- Many bug fixes
- Progress in support for nested optimized functions
- Fixing source map support
- Improved support for abstract behavior in switch statements
- Reduced memory usage of Prepack

Reviewed By: cblappert

Differential Revision: D9230689

fbshipit-source-id: 2094183fe4183089dfa584b865c9997147475e45
2018-08-08 16:09:37 -07:00
Dominic Gannaway
d6253b2680 Nested optimized function with residual functions (#2397)
Summary:
Release notes: none

This fixes a bug with nested optimized functions referencing nested residual functions and makes our internal bundle compile.

The issue was that we weren't checking if the function we get from `tryGetOptimizedFunctionRoot` was defined inside another optimized function, like we do already 5-6 lines up in the other if statement. This is an important thing, as the next line will result in `undefined` being returned. When `undefined` is returned, we use the `MainGenerator` rather than the `OptimizedFunction` generator, which means all declared values look at the wrong body.
Pull Request resolved: https://github.com/facebook/prepack/pull/2397

Differential Revision: D9223735

Pulled By: trueadm

fbshipit-source-id: 8e1cb1cc1b201b1ae1a804bc61a4bdc8790a3eea
2018-08-08 11:54:27 -07:00
Nikolai Tillmann
e042645620 Don't wait for identifiers declared in other optimized functions (#2395)
Summary:
Release notes: Progress in support for nested optimized functions

This fixes #2392.
Control flow of nested optimized functions is independent of
their parents. This change reflects that.

Added regression tests.
Pull Request resolved: https://github.com/facebook/prepack/pull/2395

Differential Revision: D9204607

Pulled By: NTillmann

fbshipit-source-id: 1c75ff7493871a4abc3c36dc63f00663c5f6e31b
2018-08-08 02:08:56 -07:00
Andres Suarez
5050226cb0 Ignore project node_modules
Reviewed By: pvdz

Differential Revision: D9196154

fbshipit-source-id: f285824d1ca4d825ae9808a6757b82b96debf849
2018-08-07 18:25:02 -07:00
Christopher Blappert
3e8c638071 Fix FatalErrors that are actually Recoverable (#2371)
Summary:
Release Notes: None

When optimizing a function fails, we can usually continue to emit a correct program while ignoring that call to `__optimize`, thus most of these errors should be `Warning`s and not `FatalError`s.

A few times (like when we have multiple sets of `Effects` for one optimized function), we can sensibly continue Prepacking code, but our output may be incorrect. In these cases, we can issue a `RecoverableError` instead of a `FatalError`
Pull Request resolved: https://github.com/facebook/prepack/pull/2371

Differential Revision: D9186240

Pulled By: cblappert

fbshipit-source-id: c30fc63fb713a3bf504cc1ec4f02410a8566cc42
2018-08-07 16:54:50 -07:00
Sebastian Markbage
be3779315e Fix default attributes in joined descriptor (#2390)
Summary:
When we create a joined descriptor in an abstract object we need to preserve any attributes of that property that were already there. To do that, we just need to read the first descriptor of the elements since all elements must have the same attributes (or we fatal).
Pull Request resolved: https://github.com/facebook/prepack/pull/2390

Differential Revision: D9193129

Pulled By: sebmarkbage

fbshipit-source-id: 32db3e88ae65de9a09ff6de72ed5b91ec02cf090
2018-08-06 22:56:26 -07:00
Nikolai Tillmann
9d02cbc575 Fixing fixup of source locations, mostly. (#2356)
Summary:
Release notes: Fixing source map support.

I kept getting seemingly garbage source locations in error messages, and looked into that.
I found various issues:
- We in-place update positions, but they are actually shared between locations, and thus we may revisit positions. When we do, we map them again, and so on...
- Locations are also shared between nodes, so we kept revisiting and rewriting yet again.
- The actual mapping doesn't pay attention to the filename, so we apply the wrong mapping altogether, especially in the presence of multiple input files. I am not fixing this, but added a TODO, and opened #2353.
- Another remaining but not blocking issue is that something goes wrong with end-positions: They are sometimes mapped to some seemingly random position in the right line. If anyone knows anything about this, please let me know...
Pull Request resolved: https://github.com/facebook/prepack/pull/2356

Differential Revision: D9141809

Pulled By: NTillmann

fbshipit-source-id: d765e99706d69e3c792fba4c553d4110963067eb
2018-08-06 18:54:38 -07:00
Greg Pataky
b965400a31 Preliminary work fixing PP0027 (#1547) (#2309)
Summary:
Attempting to fix #1547.

**Summary**:

I started work with supporting the `return` keyword in switch statements. I came to the method of "pushing" the return up through the `switch` statement by throwing the `ReturnCompletion`.

Next, I worked on the code snippet provided in #1547, which required supporting `ForkedAbruptCompletion`. I again came to the conclusion of just throwing the `Completion`, but I am not positive this is the correct way. Any guidance here would be appreciated.

Then it was adding support for `ThrowCompletion`, which ended up being the same method.

Since `continue` can only be used within a loop, I made a small test using static variables all known at optimization time, and prepack correctly evaluated the function with no additional changes necessary. However, using a variable not known at optimization time yielded an error with tag PP0037.

*Note*: I left the `if (r instanceof PossiblyNormalCompletion) {...}` throw untouched in-case I am missing what some of the possible completion types are that might trigger this (other than the ones I have touched).

**Tests**:
1. Simple Return
```javascript
function f(x, c) {
  switch (x) {
    default: return 42;
  }
}
```
```javascript
var _0 = function (x, c) {
  return 42;
};
```

2. Cases Return
```javascript
function f(x, c) {
  switch (x) {
    case 0: return 12;
    case 1: return 24;
    default: return 42;
  }
}
```
```javascript
var _0 = function (x, c) {
    var _9 = 0 === x;

    var _6 = 1 === x;

    var _3 = _6 ? 24 : 42;

    var _1 = _9 ? 12 : _3;

    return _1;
  };
```

3. Task Snippet
```javascript
(function () {
  function f(x, c) {
    switch (x) {
      case 0: if (c) return 42; else return 99;
      case 1: return 23;
    }
  }
  __optimize(f);
  global.inspect = function() { return f(0, 1); }
})();
```
```javascript
(function () {
  var _$0 = this;

  var _1 = function (x, c) {
    var _D = 0 === x;

    var _3 = c ? 42 : 99;

    var _A = 1 === x;

    var _7 = _A ? 23 : void 0;

    var _2 = _D ? _3 : _7;

    return _2;
  };

  var _0 = function () {
    return _1(0, 1);
  };

  _$0.inspect = _0;
}).call(this);
```

4. Throw Support
```javascript
function f(x, c) {
  switch (x) {
    case 0: throw 12;
    case 1: throw 24;
    default: throw 42;
  }
}
```
```javascript
var _0 = function (x, c) {
    var _1 = 0 === x;

    if (_1) {
      throw 12;
    } else {
      var _5 = 1 === x;

      if (_5) {
        throw 24;
      } else {
        throw 42;
      }
    }
  };
```

5. Continue Support (optimization-time known variables)
```javascript
function f() {
  let counter = 0;
  for (let i = 0; i < 5; i++) {
    switch (i) {
      case 0: counter++; break;
      case 1: counter += 2; break;
      case 2: counter += 3; break;
      case 3: continue;
      default: return counter;
    }
  }
}
```
```javascript
var _0 = function () {
    return 6;
  };
```

6. Continue Support (unknown variable):
```javascript
function f(max) {
  let counter = 0;
  for (let i = 0; i < max; i++) {
    switch (i) {
      case 0: counter++; break;
      case 1: counter += 2; break;
      case 2: counter += 3; break;
      case 3: continue;
      default: return counter;
    }
  }
}
```
```
In stdin(3:23) FatalError PP0037: failed to recover from a for/while loop bail-out due to unsupported logic in loop body (https://github.com/facebook/prepack/wiki/PP0037)

Prepack failed, reporting 1 fatal error.
```
Pull Request resolved: https://github.com/facebook/prepack/pull/2309

Reviewed By: hermanventer

Differential Revision: D9169333

Pulled By: zjijz

fbshipit-source-id: 3a9b2738679fcd6dac8fff9881b7464c85243723
2018-08-06 10:55:44 -07:00
Dominic Gannaway
1a3e13ddeb Improve defaultProps handling (#2391)
Summary:
Release notes: none

Fixes a `defaultProps` bug and also improves bloat slightly by ensuring we state that the default props helper can be omitted if the value it mutates is never used. Furthermore, added `isPure` to the binary expression temporal, so if the value created from it never gets used, then we don't emit the action.
Pull Request resolved: https://github.com/facebook/prepack/pull/2391

Differential Revision: D9179550

Pulled By: trueadm

fbshipit-source-id: b5671a725e347be3876a04b927eef99977757bd8
2018-08-06 08:04:29 -07:00
Dominic Gannaway
6beaf8e0fa Fix master CI (#2384)
Summary:
Release notes: none

Fixes master, had to run Prettier.
Pull Request resolved: https://github.com/facebook/prepack/pull/2384

Differential Revision: D9162736

Pulled By: trueadm

fbshipit-source-id: 640966786f6445402164cb5a023169f54e9a044b
2018-08-04 08:09:03 -07:00
Sapan Bhatia
db45feabfe Support InstantRender empty built-in (#2364)
Summary:
Resolves #2186. Embeds the value `__empty` in array and object literals for properties that are conditionally set, instead of handling such values via assignments and deletes. Raises an exception if there is a cycle in object or array creation (InstantRender does not support these).
Pull Request resolved: https://github.com/facebook/prepack/pull/2364

Differential Revision: D9169335

Pulled By: sb98052

fbshipit-source-id: f83d85677b30f10f3c548349e93ce792fc6c1ca0
2018-08-03 17:24:26 -07:00
Dominic Gannaway
9767030bc6 Follow up to comments in 2357 (#2381)
Summary:
Release notes: none

This is a follow up PR that addresses the post-land comments made in the PR #2357 from sebmarkbage.
Pull Request resolved: https://github.com/facebook/prepack/pull/2381

Differential Revision: D9160458

Pulled By: trueadm

fbshipit-source-id: d4b4fd98d88c0d91d4b4a4f14caf5179d68eff91
2018-08-03 14:24:16 -07:00
Chris Blappert
23a310e6ee Fix issue #2359 (#2382)
Summary:
Release Notes: None

We weren't properly adding leaked bindings to the optimized function's FunctionInstance, so state didn't get cleaned up properly between passes of the serializer.

There was also a bug in emitter invariants. Loosening the invariant causes the test cases to pass.

Also the invariant seems to have been incorrect in the first place. I suspect it is just the direct negation of [this line](393624f00f/src/serializer/Emitter.js (L103)).

Added slightly minimized repros from the issue.

I am not quite sure what we are trying to test with the last line of this invariant. I think what we're trying to test with the last line of this invariant is: If we're emitting to an additional function, it must be the case that referencedDeclaredValues has `val`. Here the test also passes when `referencedDeclaredValues.has(val)` is `false` which seems wrong to me.

I'm also dubious that top level optimized functions don't consider the MainGenerator as their parent generator, as this means that any derived ids declared in the MainGenerator shouldn't be considered declared in optimized functions (was able to repro that in conditions2 test after tightening invariants). For the sake of unblocking React Compiler, I'll look into that in a future PR.

Solves #2359.
Pull Request resolved: https://github.com/facebook/prepack/pull/2382

Differential Revision: D9158973

Pulled By: cblappert

fbshipit-source-id: a6915d0f125708739022d8576d5f007107e76b6b
2018-08-03 14:09:58 -07:00
David Cai
d04e5bbace Moved packaging of debug repro out of CLI, into own class (#2347)
Summary:
Release Notes:
- Added CLI flag to specify path to prepack. This was the solution suggested by D. Aurelio r/e getting path to zipped version of prepack on sc machines.
Pull Request resolved: https://github.com/facebook/prepack/pull/2347

Reviewed By: yinghuitan

Differential Revision: D9133562

Pulled By: caiismyname

fbshipit-source-id: d1338607efd1ed9b7002704de02b38d8b2ad407e
2018-08-03 12:38:51 -07:00
Nikolai Tillmann
393624f00f Let the visitor die when it's done (#2379)
Summary:
Release notes: Reduced memory usage of Prepack

The visitor computes some data structures that live on,
but it also maintains some internal temporary state that
should be released once the visitor is done.

This factors out the resulting data from the stateful visitor.
As a side-effect, we also need to pass less redundant stuff
through various levels of constructors.
Pull Request resolved: https://github.com/facebook/prepack/pull/2379

Differential Revision: D9150907

Pulled By: NTillmann

fbshipit-source-id: 8556a111eae67c7e6032d148e568c12b644b4e69
2018-08-03 10:39:28 -07:00
Nikolai Tillmann
4103d8e9fe Release source code and source map data after parsing and source map fixup. (#2377)
Summary:
Release notes: Reduce memory usage of running Prepack by 3% in some scenarios

This fixes #2365.

This is realized via a stateful SourceFileCollection.
On a small internal benchmark, this saved around 3% of total node memory usage
(the parsed AST that we keep around uses an order of magnitude more memory
than the original source file and source map,
11MB of source files + source map vs 88MB of parsed AST).
Pull Request resolved: https://github.com/facebook/prepack/pull/2377

Differential Revision: D9150888

Pulled By: NTillmann

fbshipit-source-id: b38a8176c4f1e4633366bd48b7d396aec023e7c3
2018-08-03 10:39:28 -07:00
Nikolai Tillmann
c89f511102 Fixing source map support with multiple source files (#2362)
Summary:
Release notes: Fixing source map support with multiple source files

This fixes issue #2353: We now track multiple --srcmapIn arguments,
and match which one applies by comparing the basenames:
The convention is that sourcemaps share the same basename with an appended .map.
Pull Request resolved: https://github.com/facebook/prepack/pull/2362

Differential Revision: D9138802

Pulled By: NTillmann

fbshipit-source-id: d359ccd372b7d87445ecc1a2bdb509ba158e0200
2018-08-02 14:39:23 -07:00
Dominic Gannaway
7cb3129cd0 Clean up of React nested optimized logic (#2348)
Summary:
Release notes: none

Follow up to https://github.com/facebook/prepack/pull/2346. This PR cleans up all the React specific nested optimized function logic, removing most of the old legacy code in favor of the simpler/cleaner approach.

This also introduces a `bubbleSideEffectReports` parameter to `evaluatePure` for specific case where we don't want side-effects to be reported to parent `evaluatePure` calls; necessary for the React reconciler to not throw a `UnsupportedSideEffect` which will cause the reconciliation process to hard-fail (instead nested optimize functions bail-out to be inline abstract function calls like before, and we do not try and optimize the nested function).

Lastly, we now throw an `SideEffect` object when evaluating a nested optimized function and detecting a side-effect. There's no point in continuing evaluation of a given side-effectful function, so this speeds things up a bit by terminating execution.
Pull Request resolved: https://github.com/facebook/prepack/pull/2348

Differential Revision: D9138290

Pulled By: trueadm

fbshipit-source-id: 6d468ea0430be0c0190ff9817b2872df368c325d
2018-08-02 14:09:28 -07:00
Dominic Gannaway
1b4f70658f Remove makeNotPartial and pass around explicit boolean instead (#2357)
Summary:
Release notes: none

This PR removes the `makeNotPartial` method and all its call-sites in favour of explicitly telling execution paths that we want own properties even if partial. This stops the mutation of objects where we set the partial boolean flag, which actually is a very common cause of side-effects in pure functions. Furthermore, the whole `makeNotPartial` before and after was potentially error-prone if the `try/catch` was omitted by mistake and it was generally a code smell.

I also made it test-runner log the failing test name upon error, which helps with debugging.
Pull Request resolved: https://github.com/facebook/prepack/pull/2357

Differential Revision: D9137606

Pulled By: trueadm

fbshipit-source-id: c0c59615f7a7d46836d26a3b2aecfb89274933d8
2018-08-02 13:28:26 -07:00
Nikolai Tillmann
28553ea4e0 A few issues I found while being a Prepack user on InstantRender. (#2352)
Summary:
Release notes: None

- Make formatting of error locations more useful.
- Factored out redundant code that initialize ECMAScriptSource functions
  (and as part of that possibly a small improvement in locality: record
   `uniqueOrderedTag` only the first time a function is encountered,
   not effectively the last time)_
- Made PP1003 recoverable (not sure what's fatal about it)
- Deleted dead `functionExpressions` property
Pull Request resolved: https://github.com/facebook/prepack/pull/2352

Differential Revision: D9136199

Pulled By: NTillmann

fbshipit-source-id: 279efcc97310ccdb11907b74f4f166851b816819
2018-08-02 12:39:02 -07:00
Chris Blappert
9b0642a7ba Fix issue 2266 (#2303)
Summary:
Release Notes: None

Fixes #2266.

Looking at the `__optimize` code, what we want is for every call to `__optimize` to optimize that function, even in conditonal contexts. In this case, it makes no sense to store the functions to optimize in a special global because we don't care about the applying/reverting of effects. It simplifies the code significantly to be able to store these values in the realm instead.

The only case where this would matter is if effects containing an __optimize call get created then discarded (I am unsure if there are any legitimate cases of this in Prepack at the moment, but I created a CompilerDiagnostic just in case).
Pull Request resolved: https://github.com/facebook/prepack/pull/2303

Differential Revision: D9134968

Pulled By: cblappert

fbshipit-source-id: 040f7ccc24f928e6b8daa068521d3848caffc4d2
2018-08-02 11:39:48 -07:00
Chris Blappert
1d04882b80 Fix visiting of leaked bindings of nested optimized functions (#2344)
Summary:
Release Notes: None

Now bindingAssignments will have their containing optimized function visit them.

Fixes #2335 and test case 3 of #2252

Test output currently fails lint, once #2339 lands and I rebase past it, that'll go away.
Pull Request resolved: https://github.com/facebook/prepack/pull/2344

Reviewed By: trueadm

Differential Revision: D9133987

Pulled By: cblappert

fbshipit-source-id: 243fc56a6ff7552c801762e2668ea5e4607599fd
2018-08-02 10:38:24 -07:00