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

make collection traversals stack safe #3521

Merged
merged 1 commit into from
Jul 15, 2020
Merged

make collection traversals stack safe #3521

merged 1 commit into from
Jul 15, 2020

Conversation

johnynek
Copy link
Contributor

close #3517

This makes all the traversals of all collections stack safe.

It follows the approach of: typelevel/fs2#1957 but it is lazier (and maybe a bit slower because of that).

Cats has some tests that traversals on Applicatives that have lazy map2Eval instances that it won't trigger the function in those cases, so these laws require the traverse not only to have a certain result, but also invoke a function a known number of times.

I added tests before making each change and showed that the collection would fail a certain traversal (using Function0 as an example monad with poor stack safety).

@johnynek johnynek mentioned this pull request Jul 15, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #3521 into master will decrease coverage by 0.19%.
The diff coverage is 83.58%.

@@            Coverage Diff             @@
##           master    #3521      +/-   ##
==========================================
- Coverage   91.49%   91.30%   -0.20%     
==========================================
  Files         386      380       -6     
  Lines        8486     8288     -198     
  Branches      258      222      -36     
==========================================
- Hits         7764     7567     -197     
+ Misses        722      721       -1     

Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really great, thanks @johnynek!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stack safe traverse
4 participants