Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Free.foldMap is not stack safe #721

Closed
ceedubs opened this issue Dec 7, 2015 · 4 comments · Fixed by #1075
Closed

Free.foldMap is not stack safe #721

ceedubs opened this issue Dec 7, 2015 · 4 comments · Fixed by #1075

Comments

@ceedubs
Copy link
Contributor

ceedubs commented Dec 7, 2015

This is a followup to #713. #702 made Free.foldMap stack safe (and added a test for this), but unfortunately it had to be backed out because it introduced a bug. Currently the stack safety test for foldMap is ignored.

There was some discussion in #713 as to what should happen for this. I'm no expert in the matter. I'm hoping some others such as @mandubian or @adelbertc might be able to weigh in.

@TomasMikula
Copy link
Contributor

TomasMikula commented May 19, 2016

We can implement a stack-safe foldMap using MonadRec[M]. The question is, do we

  1. keep the current foldMap, which is stack-safe only for lazy (trampolined) monads, and add the stack-safe version as foldMapRec; or
  2. change the current method, only allowing foldMap with tail-rec monads.

I would be leaning more towards 2., to promote stack-safety by default. For cases where people don't have a stack-safe monad, but stack-safety is not a problem b/c their Free structure is shallow, they can always provide a fake MonadRec[M] instance. Or we could even keep the original method as foldMapUnsafe.

@mandubian
Copy link
Contributor

I suppose it brings also constant-stack execution, right?
By default, stack-safety is clearly preferred because when it bites, it bites hard & randomly.
But, it would be cool to check the cost on perfs with MonadRec.

@TomasMikula
Copy link
Contributor

@mandubian my definition of stack-safety is constant-stack usage, so I don't understand your first question.

Regarding the latter, are there some existing Free benchmarks?

@mandubian
Copy link
Contributor

I don't think there are benches but it would be useful ;)

TomasMikula added a commit to TomasMikula/cats that referenced this issue May 31, 2016
Fixes typelevel#721.

The fix is possible by strengthening constraints on the target monad
from Monad to MonadRec.
TomasMikula added a commit to TomasMikula/cats that referenced this issue May 31, 2016
Fixes typelevel#721.

The fix is possible by strengthening constraints on the target monad
from Monad to MonadRec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants