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

More stack safety for IndexedReaderWriterStateT & IndexedStateT #2924

Conversation

ybasket
Copy link
Contributor

@ybasket ybasket commented Jun 30, 2019

I'm currently adding Sync instances for IndexedReaderWriterStateT to cats-effect (see typelevel/cats-effect#577). Some of the tests there fail as IndexedReaderWriterStateT.flatMap is not stack-safe even when used with a stack-safe monad.

Based on previous work for Kleisli (see #2185), I made flatMap and flatMapF stack-safe by introducing IndexedReaderWriterStateT.shift which implements special behavior if F[_]'s FlatMap instance is also an Applicative.

Furthermore, I applied the usage of AndThen which made IndexedStateT.flatMap stack-safe in #2187 to map as well, allowing for repeated map calls without blowing the stack. The same method unfortunately cannot easily be applied to IndexedReaderWriterStateT as we don't have a Function1 there, so map stays unsafe there.

@codecov-io
Copy link

codecov-io commented Jun 30, 2019

Codecov Report

Merging #2924 into master will decrease coverage by 0.01%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2924      +/-   ##
==========================================
- Coverage   94.24%   94.22%   -0.02%     
==========================================
  Files         367      367              
  Lines        7002     6996       -6     
  Branches      195      197       +2     
==========================================
- Hits         6599     6592       -7     
- Misses        403      404       +1
Impacted Files Coverage Δ
core/src/main/scala/cats/data/IndexedStateT.scala 90% <100%> (ø) ⬆️
...in/scala/cats/data/IndexedReaderWriterStateT.scala 97% <75%> (-1.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 64eac53...3c671ad. Read the comment docs.

@ybasket ybasket changed the title More stack safety for IndexedReaderWriterStateT & IndexedStateT More stack safety for IndexedReaderWriterStateT & IndexedStateT Jun 30, 2019
@djspiewak djspiewak merged commit ed99e17 into typelevel:master Jul 19, 2019
@kailuowang kailuowang added this to the 2.0.0-RC1 milestone Aug 2, 2019
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.

5 participants