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 iterateRight sound #1980

Merged
merged 2 commits into from
Oct 20, 2017
Merged

Conversation

LukaJCB
Copy link
Member

@LukaJCB LukaJCB commented Oct 19, 2017

Small fix for #1973. I'm not sure if the implementation is the most efficient, so feedback would be greatly appreciated! :)

Here's the code I used to test the unsoundness:

scala> Foldable.iterateRight(List(1,2,3).iterator, Eval.now(0)) { (a, eb) => 
     |   eb.flatMap(b => Eval.always(a + b))
     | }
res0: cats.Eval[Int] = cats.Eval$$anon$9@432234e6

scala> res0.value
res1: Int = 6

scala> res0.value
res2: Int = 0

johnynek
johnynek previously approved these changes Oct 19, 2017
Copy link
Contributor

@johnynek johnynek left a comment

Choose a reason for hiding this comment

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

You’re a machine!

Thanks for fixing so quickly.

@codecov-io
Copy link

codecov-io commented Oct 19, 2017

Codecov Report

Merging #1980 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1980      +/-   ##
==========================================
- Coverage   96.21%   96.21%   -0.01%     
==========================================
  Files         272      272              
  Lines        4628     4627       -1     
  Branches      124      115       -9     
==========================================
- Hits         4453     4452       -1     
  Misses        175      175
Impacted Files Coverage Δ
core/src/main/scala/cats/Foldable.scala 100% <100%> (ø) ⬆️
core/src/main/scala/cats/instances/map.scala 94.28% <100%> (-0.16%) ⬇️
core/src/main/scala/cats/instances/set.scala 100% <100%> (ø) ⬆️

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 b66b121...39f1027. Read the comment docs.

@kailuowang kailuowang added the bug label Oct 19, 2017
@kailuowang kailuowang added this to the 1.0.0-RC1 milestone Oct 19, 2017
kailuowang
kailuowang previously approved these changes Oct 19, 2017
@kailuowang
Copy link
Contributor

@LukaJCB sorry, merge conflicts.

@kailuowang kailuowang merged commit 882a673 into typelevel:master Oct 20, 2017
@LukaJCB LukaJCB deleted the fix-iterate-right branch October 20, 2017 10:25
kailuowang pushed a commit to kailuowang/cats that referenced this pull request Oct 23, 2017
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