arrow-check icon indicating copy to clipboard operation
arrow-check copied to clipboard

Rewrite to support suspend over IO and mtl.

Open 1Jajen1 opened this issue 4 years ago • 5 comments

Things that need to be done before this can be merged:

  • [ ] ~Function generation. The old version relied on Eval for stacksafety but since that does not work with suspend generators I need to come up with something else. That code is really ugly tho and thus will take a bit.~ I am not doing that this pr. It will come back, but it takes more work and thought. This currently fails because of stackoverflows and missing laziness and its kind of hard to fix.
  • [ ] Stacksafe shrinking. Currently Gen.map calls into Flow.map which afaik is not stacksafe. This is rarely a concern for generators but needs to be addressed anyway. I am fine with skipping that tho.
  • [x] Testing, this version isn't well tested just yet and that needs to change.
    • [x] Test the suspend interop and the implications multishot brings
    • [x] Test the environment parameter and if local works as expected
  • [x] Gen.subterms up to arity 3
  • [x] Gen debug functions like sample, printTree etc
  • [x] Api docs

The new encoding is strictly less powerful but much easier to use.

1Jajen1 avatar Dec 13 '20 15:12 1Jajen1

Function generation. The old version relied on Eval for stacksafety but since that does not work with suspend generators I need to come up with something else. That code is really ugly tho and thus will take a bit.

How hard would it be to write an Eval that supports suspend 🤔 Even if it's just for the purpose of having it as internal here?

nomisRev avatar Dec 18 '20 17:12 nomisRev

How hard would it be to write an Eval that supports suspend thinking Even if it's just for the purpose of having it as internal here?

I actually don't think that is hard at all, just like with the AndThenS class. It would be even easier if I do that without memoization.

But I also discovered other problems with suspend there that reach a little further and require substantial changes to the api, so for now I just use runBlocking inside the function gen...

Also I am running into so many problems even with this implementation that I am considering skipping this and redoing it later because most if not all other things are done and working. The jvm really isn't nice to this style of programming, no gadts, no stacksafety, no ghc-generics equivalent... I need to find a better encoding...

1Jajen1 avatar Dec 18 '20 17:12 1Jajen1

So quick status update: Feature wise this is good to go. I'll skip function generation as that requires a pr of its own + new ideas. What this branch still needs is also what the current version lacks: More tests, tho mostly for internal components and the existing generator tests make use of those and thus kind of imply that those components work which is why I am somewhat confident in slowly adding tests over time rather than trying to cover all of it at once.

I'll also leave this pr in a ready to be merged state so I won't block arrow as a whole when we get to the next release^^

Documentation is the next task, for now I'll add a minimum viable bit of docs in the readme, but eventually this needs a few sections explaining property tests and some of the more hidden parts.

Lastly: Most of the deprecation messages currently come from the current parser combinators and the diffing algorithm which is based on recursion schemes. If those are a problem for the next release I can specialize both of those to unkinded versions.

1Jajen1 avatar Jan 17 '21 10:01 1Jajen1

Super awesome work @1Jajen1 👏 👏 👏 All your suggestions sounds perfect for me! Will try to make some time tomorrow to review but it's not going to be easy 😅 +3,258 −4,950

nomisRev avatar Jan 17 '21 12:01 nomisRev

Will try to make some time tomorrow to review but it's not going to be easy sweat_smile +3,258 −4,950

Ah ye, it's quite large :see_no_evil: . I'd suggest taking a look at the encodings of Gen/Rose and Test/PropertyTest first and then view the rest of the it because without understanding what those do its going to be hard to parse some of the more in-depth methods.

1Jajen1 avatar Jan 17 '21 13:01 1Jajen1