Revert regression in Free.foldMap

[ceedubs]

This PR does a couple of things.

Firstly, it includes Gosub in the instances generated by the Free Arbitrary instance. This reveals a bug that caused the "mapSuspension id" test to fail.

It also reverts most of the changes made in #702. This fixes the failing "mapSuspension id" test and #712. Unfortunately, it comes at the cost of reintroducing the stack safety issue with Free.foldMap, and therefore I have its stack-safety unit test ignored for now. I'm hoping that we can address this issue with a separate PR in the future.

[codecov-io]

Current coverage is 85.43%

Merging #713 into master will increase coverage by +0.01% as of d831efe

@@            master    #713   diff @@
======================================
  Files          162     162       
  Stmts         2230    2231     +1
  Branches        74      74       
  Methods          0       0       
======================================
+ Hit           1905    1906     +1
  Partial          0       0       
  Missed         325     325       

Review entire Coverage Diff as of d831efe

Powered by Codecov. Updated on successful CI builds.

[ceedubs]

@svalaskevicius to be honest I don't understand why your change broke this. If you can find an alternative that fixes the "mapSuspension id" test and #712 but is still stack safe, that would be ideal.

[svalaskevicius]

Hmm it looks I've made an error earlier in gosub suspend case and missed flatmap.. Will check later to confirm

[non]

I think we should revert now (to remove bug) and then submit a new PR to fix the actual issue.

👍

[svalaskevicius]

had a quick look.. looks like I made it tail recursive but removing the flatMap... which is required if we want to keep the structure.

so when I add it back:

case Gosub(c, g) => c match {
    case Suspend(s) => M.flatMap(f(s))(cc => g(cc).foldMap(f))

the recursion is not in the tail position any more and I'm not sure how to make it so.. any ideas?

[svalaskevicius]

@non, @ceedubs:

actually, does foldMap have to preseve the structure?

e.g. on hackage it is Map each element of the structure to a monoid, and combine the results.

maybe it would make sense to leave structure preservation to mapSuspension only?

the reason being is that I don't see a way to make it both stack safe and structure preserving (using flatmap there) - that would be the ultimate solution :)

[ceedubs]

I'm going to go ahead and merge this since Free isn't really usable at the moment. I'll create a new issue for its stack safety, as @non has suggested.