Skip to content

Commit

Permalink
Fix the evaluation order of reduceRightTo/reduceRightToOption.
Browse files Browse the repository at this point in the history
  • Loading branch information
takayahilton committed Jun 26, 2020
1 parent 81a2efe commit 90fde63
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 16 deletions.
23 changes: 15 additions & 8 deletions core/src/main/scala/cats/Foldable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ package cats
import scala.collection.mutable
import cats.kernel.CommutativeMonoid
import simulacrum.{noop, typeclass}
import Foldable.sentinel
import Foldable.{sentinel, Source}

import scala.annotation.implicitNotFound

/**
Expand Down Expand Up @@ -98,7 +99,7 @@ import scala.annotation.implicitNotFound

def foldRightDefer[G[_]: Defer, A, B](fa: F[A], gb: G[B])(fn: (A, G[B]) => G[B]): G[B] =
Defer[G].defer(
this.foldLeft(fa, (z: G[B]) => z) { (acc, elem) => z =>
foldLeft(fa, (z: G[B]) => z) { (acc, elem) => z =>
Defer[G].defer(acc(fn(elem, z)))
}(gb)
)
Expand All @@ -109,13 +110,19 @@ import scala.annotation.implicitNotFound
case (None, a) => Some(f(a))
}

def reduceRightToOption[A, B](fa: F[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[Option[B]] =
foldRight(fa, Now(Option.empty[B])) { (a, lb) =>
lb.flatMap {
case Some(b) => g(a, Now(b)).map(Some(_))
case None => Later(Some(f(a)))
}
def reduceRightToOption[A, B](fa: F[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[Option[B]] = {
Source.fromFoldable(fa)(self).uncons match {
case Some((first, s)) =>
def loop(now: A, source: Source[A]): Eval[B] =
source.uncons match {
case Some((next, s)) => g(now, Eval.defer(loop(next, s.value)))
case None => Eval.later(f(now))
}

Eval.defer(loop(first, s.value).map(Some(_)))
case None => Eval.now(None)
}
}

/**
* Reduce the elements of this structure down to a single value by applying
Expand Down
15 changes: 10 additions & 5 deletions core/src/main/scala/cats/Reducible.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package cats

import cats.Foldable.Source
import cats.data.{Ior, NonEmptyList}
import simulacrum.{noop, typeclass}
import scala.annotation.implicitNotFound
Expand Down Expand Up @@ -386,14 +387,18 @@ abstract class NonEmptyReducible[F[_], G[_]](implicit G: Foldable[G]) extends Re
G.foldLeft(ga, f(a))((b, a) => g(b, a))
}

def reduceRightTo[A, B](fa: F[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[B] =
def reduceRightTo[A, B](fa: F[A])(f: A => B)(g: (A, Eval[B]) => Eval[B]): Eval[B] = {
def loop(now: A, source: Source[A]): Eval[B] =
source.uncons match {
case Some((next, s)) => g(now, Eval.defer(loop(next, s.value)))
case None => Eval.later(f(now))
}

Always(split(fa)).flatMap {
case (a, ga) =>
G.reduceRightToOption(ga)(f)(g).flatMap {
case Some(b) => g(a, Now(b))
case None => Later(f(a))
}
Eval.defer(loop(a, Foldable.Source.fromFoldable(ga)))
}
}

override def size[A](fa: F[A]): Long = {
val (_, tail) = split(fa)
Expand Down
1 change: 0 additions & 1 deletion core/src/main/scala/cats/data/NonEmptyChain.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cats
package data

import NonEmptyChainImpl.create
import cats.{Order, Semigroup}
import cats.kernel._
import scala.collection.immutable.SortedMap

Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/cats/data/NonEmptyList.scala
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ sealed abstract private[data] class NonEmptyListInstances extends NonEmptyListIn

override def nonEmptyPartition[A, B, C](
fa: NonEmptyList[A]
)(f: (A) => Either[B, C]): Ior[NonEmptyList[B], NonEmptyList[C]] = {
)(f: A => Either[B, C]): Ior[NonEmptyList[B], NonEmptyList[C]] = {
val reversed = fa.reverse
val lastIor = f(reversed.head) match {
case Right(c) => Ior.right(NonEmptyList.one(c))
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/cats/instances/list.scala
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ trait ListInstances extends cats.kernel.instances.ListInstances {

override def partitionEither[A, B, C](
fa: List[A]
)(f: (A) => Either[B, C])(implicit A: Alternative[List]): (List[B], List[C]) =
)(f: A => Either[B, C])(implicit A: Alternative[List]): (List[B], List[C]) =
fa.foldRight((List.empty[B], List.empty[C]))((a, acc) =>
f(a) match {
case Left(b) => (b :: acc._1, acc._2)
Expand Down

0 comments on commit 90fde63

Please sign in to comment.