Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

New invariant finds objects without properly applied effects #2279

Closed
NTillmann opened this issue Jul 17, 2018 · 0 comments
Closed

New invariant finds objects without properly applied effects #2279

NTillmann opened this issue Jul 17, 2018 · 0 comments
Assignees
@hermanventer
Labels
abstract bug interpreter

Comments

@NTillmann
Copy link
Contributor

#2278 uncovers an issue with test/serializer/additional-functions/NestedThrowEffects.js:

Invariant Violation:
This is likely a bug in Prepack, not your code. Feel free to open an issue on GitHub.
    at invariant (../src/invariant.js:18:15)
    at Function._generatorOfEffects (../../src/utils/generator.js:512:7)
    at evaluateForEffectsInGlobalEnv (../src/realm.js:808:18)
    at Realm.evaluateForEffects (../src/realm.js:858:11)
    at wrapInGlobalEnv (../src/realm.js:798:39)
    at Realm.wrapInGlobalEnv (../src/realm.js:658:7)
    at Realm.evaluateForEffectsInGlobalEnv (../src/realm.js:798:12)
    at Realm.withEffectsAppliedInGlobalEnv (../src/realm.js:805:5)
    at Function.fromEffects (../../src/utils/generator.js:552:12)
    at createAdditionalEffects (../../src/serializer/utils.js:194:19)
    at getEffectsFromAdditionalFunctionAndNestedFunctions (../../src/serializer/functions.js:232:39)
    at Functions.checkThatFunctionsAreIndependent (../../src/serializer/functions.js:271:7)
    at statistics.checkThatFunctionsAreIndependent.measure (../../src/serializer/serializer.js:142:9)
    at PerformanceTracker.measure (../src/statistics.js:100:7)
    at statistics.total.measure (../../src/serializer/serializer.js:141:7)
    at PerformanceTracker.measure (../src/statistics.js:100:7)
    at Serializer.init (../../src/serializer/serializer.js:126:18)
    at prepackSources (../src/prepack-standalone.js:66:22)
    at /Users/nikolait/git/prepack/scripts/test-runner.js:519:30
    at Array.map (<anonymous>)
    at /Users/nikolait/git/prepack/scripts/test-runner.js:499:60
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
    at Function.Module.runMain (module.js:695:11)
    at Object.<anonymous> (/Users/nikolait/git/prepack/node_modules/@babel/node/lib/_babel-node.js:221:23)
    at Module._compile (module.js:652:30)
    at Object.Module._extensions..js (module.js:663:10)
    at Module.load (module.js:565:32)
    at tryModuleLoad (module.js:505:12)
    at Function.Module._load (module.js:497:3)
    at Function.Module.runMain (module.js:693:10)
    at startup (bootstrap_node.js:188:16)
    at bootstrap_node.js:609:3
@NTillmann NTillmann mentioned this issue Jul 17, 2018
Closed
@hermanventer hermanventer self-assigned this Jul 17, 2018
@hermanventer hermanventer mentioned this issue Aug 10, 2018
Closed
facebook-github-bot pushed a commit that referenced this issue Aug 12, 2018
@facebook-github-bot
Do full joins (#2402)
7157849 
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: #2402

Differential Revision: D9236263

Pulled By: hermanventer

fbshipit-source-id: 92a25b591591297afeba536429226c5a0291f451
facebook-github-bot pushed a commit that referenced this issue Aug 15, 2018
@facebook-github-bot
Weekly release v0.2.48: Fuzz tester and join logic refactor
90c3aa2 
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
facebook-github-bot pushed a commit that referenced this issue Aug 31, 2018
@NTillmann @facebook-github-bot
Add invariant that an object with modified properties must be valid. (#…
b3bcd73 
…2278)

Summary:
Release notes: None

The uncovered bug is tracked in #2279.
Pull Request resolved: #2278

Differential Revision: D9472482

Pulled By: NTillmann

fbshipit-source-id: 5d6e24abd76fd83e9078c3908fa0be0e7f0b2c51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@hermanventer hermanventer

Labels
abstract bug interpreter
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hermanventer @NTillmann