From 0666f070581d273eddbfcf12f41c30c468213a60 Mon Sep 17 00:00:00 2001 From: Daniel Spiewak Date: Thu, 11 May 2017 23:19:04 -0600 Subject: [PATCH 1/2] Removed MonadError[Eval, Throwable] (fixes #1661, #1639) --- core/src/main/scala/cats/Eval.scala | 54 +------------------ .../src/test/scala/cats/tests/EvalTests.scala | 52 +----------------- 2 files changed, 4 insertions(+), 102 deletions(-) diff --git a/core/src/main/scala/cats/Eval.scala b/core/src/main/scala/cats/Eval.scala index 7a85842a87..5c621cda0c 100644 --- a/core/src/main/scala/cats/Eval.scala +++ b/core/src/main/scala/cats/Eval.scala @@ -1,7 +1,6 @@ package cats import scala.annotation.tailrec -import scala.util.control.NonFatal import cats.syntax.all._ @@ -109,43 +108,8 @@ sealed abstract class Eval[+A] extends Serializable { self => * Later[A] with an equivalent computation will be returned. */ def memoize: Eval[A] - - /** - * Recover from exceptions thrown within Eval (i.e. when `.value` is - * called). - * - * Note that this will not help for exceptions thrown while building - * an Eval instance (e.g. `Now(sys.error("die"))`), but will help - * with exceptions which are deferred (e.g. `Later(sys.error("die"))`). - * - * Unlike many other methods on `Eval`, this method does consume one - * stack frame during the eventual `.value` call. This means that - * it's not necessarily safe to nest recursive calls to - * `.handleErrorWith`. - * - * The `recovery` method can re-raise exceptions (if necessary) - * using the `Eval.raise` method, or via `throw` directly. - * Exceptions "at-rest" are not represented with `Eval`, your only - * options for catching and dealing with exceptions are this method, - * or wrapping your `.value` calls with something like `Try()`. - */ - final def handleErrorWith[A1 >: A](recovery: Throwable => Eval[A1]): Eval[A1] = - Eval.defer(try Now(value) catch { case NonFatal(t) => recovery(t) }) - - /** - * Recover from exceptions thrown within Eval (i.e. when `.value` is - * called). - * - * This method only differs from `handleErrorWith` in that it takes - * a partial function. See that method's documentation for a more - * complete description of the (very limited) exception-handling - * capabilities of `Eval`. - */ - final def recoverWith[A1 >: A](f: PartialFunction[Throwable, Eval[A1]]): Eval[A1] = - handleErrorWith(t => if (f.isDefinedAt(t)) f(t) else Eval.raise(t)) } - /** * Construct an eager Eval[A] instance. * @@ -241,16 +205,6 @@ object Eval extends EvalInstances { def defer[A](a: => Eval[A]): Eval[A] = new Eval.Call[A](a _) {} - /** - * Create an Eval instance which will throw the given exception when - * evaluated. - * - * This method can be paired with the `.recoverWith` method to - * encode exception handling within an `Eval` context. - */ - def raise(t: Throwable): Eval[Nothing] = - Eval.defer(throw t) - /** * Static Eval instance for common value `Unit`. * @@ -374,8 +328,8 @@ object Eval extends EvalInstances { private[cats] trait EvalInstances extends EvalInstances0 { - implicit val catsBimonadForEval: Bimonad[Eval] with MonadError[Eval, Throwable] = - new Bimonad[Eval] with MonadError[Eval, Throwable] { + implicit val catsBimonadForEval: Bimonad[Eval] = + new Bimonad[Eval] { override def map[A, B](fa: Eval[A])(f: A => B): Eval[B] = fa.map(f) def pure[A](a: A): Eval[A] = Now(a) def flatMap[A, B](fa: Eval[A])(f: A => Eval[B]): Eval[B] = fa.flatMap(f) @@ -385,10 +339,6 @@ private[cats] trait EvalInstances extends EvalInstances0 { case Left(nextA) => tailRecM(nextA)(f) case Right(b) => pure(b) } - def handleErrorWith[A](fa: Eval[A])(f: Throwable => Eval[A]): Eval[A] = - fa.handleErrorWith(f) - def raiseError[A](e: Throwable): Eval[A] = - Eval.raise(e) } implicit val catsReducibleForEval: Reducible[Eval] = diff --git a/tests/src/test/scala/cats/tests/EvalTests.scala b/tests/src/test/scala/cats/tests/EvalTests.scala index aee39c2008..8f4d23e6db 100644 --- a/tests/src/test/scala/cats/tests/EvalTests.scala +++ b/tests/src/test/scala/cats/tests/EvalTests.scala @@ -2,9 +2,8 @@ package cats package tests import scala.math.min -import scala.util.Try import cats.laws.ComonadLaws -import cats.laws.discipline.{BimonadTests, CartesianTests, MonadErrorTests, ReducibleTests, SerializableTests} +import cats.laws.discipline.{BimonadTests, CartesianTests, MonadTests, ReducibleTests, SerializableTests} import cats.laws.discipline.arbitrary._ import cats.kernel.laws.{GroupLaws, OrderLaws} @@ -83,60 +82,13 @@ class EvalTests extends CatsSuite { } } - test("handleErrorWith is semi-stacksafe") { - def loop(i: Int, e: Eval[Int]): Eval[Int] = - if (i <= 0) e - else if (i % 100 == 0) loop(i - 1, e.map(n => n).handleErrorWith(_ => Now(-999))) - else loop(i - 1, e.map(n => n)) - - // we expect to use ~1k stack frames of error handling, which - // should be safe. - loop(100000, Now(6)).value should === (6) - } - - test("Eval.raise(t).handleErrorWith(f) = f(t)") { - forAll { (t: Throwable, f: Throwable => Eval[Int]) => - Eval.raise(t).handleErrorWith(f) should === (f(t)) - } - } - - test(".recoverWith and .handleErrorWith are equivalent") { - forAll { (e0: Eval[Int], f: Throwable => Eval[Int], p: PartialFunction[Throwable, Eval[Int]]) => - - // should be an error at least 1/3 of the time. - val e = e0.map { n => if (n % 3 == 0) n / 0 else n / 2 } - - // test with a total recovery function - val x1 = Try(e.handleErrorWith(f)) - val y1 = Try(e.recoverWith { case e => f(e) }) - x1 should === (y1) - - // test with a partial recovery function - val x2 = Try(e.recoverWith(p).value) - val y2 = Try(e.handleErrorWith(t => if (p.isDefinedAt(t)) p(t) else Eval.raise(t)).value) - x2 should === (y2) - - // ensure that this works if we throw directly - val z2 = Try(e.handleErrorWith(t => if (p.isDefinedAt(t)) p(t) else throw t).value) - x2 should === (z2) - } - } - { implicit val iso = CartesianTests.Isomorphisms.invariant[Eval] checkAll("Eval[Int]", BimonadTests[Eval].bimonad[Int, Int, Int]) - { - // we need exceptions which occur during .value calls to be - // equal to each other (equivalent behavior). - implicit def eqWithTry[A: Eq]: Eq[Eval[A]] = - Eq[Try[A]].on((e: Eval[A]) => Try(e.value)) - - checkAll("Eval[Int]", MonadErrorTests[Eval, Throwable].monadError[Int, Int, Int]) - } + checkAll("Eval[Int]", MonadTests[Eval].monad[Int, Int, Int]) } checkAll("Bimonad[Eval]", SerializableTests.serializable(Bimonad[Eval])) - checkAll("MonadError[Eval, Throwable]", SerializableTests.serializable(MonadError[Eval, Throwable])) checkAll("Eval[Int]", ReducibleTests[Eval].reducible[Option, Int, Int]) checkAll("Reducible[Eval]", SerializableTests.serializable(Reducible[Eval])) From ab2f0f59b422e17982d09536892f639d5a00f34c Mon Sep 17 00:00:00 2001 From: Daniel Spiewak Date: Mon, 22 May 2017 09:59:04 -0600 Subject: [PATCH 2/2] Removedly suspiciously redundant line --- tests/src/test/scala/cats/tests/EvalTests.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/src/test/scala/cats/tests/EvalTests.scala b/tests/src/test/scala/cats/tests/EvalTests.scala index 8f4d23e6db..90bbf70f8d 100644 --- a/tests/src/test/scala/cats/tests/EvalTests.scala +++ b/tests/src/test/scala/cats/tests/EvalTests.scala @@ -3,7 +3,7 @@ package tests import scala.math.min import cats.laws.ComonadLaws -import cats.laws.discipline.{BimonadTests, CartesianTests, MonadTests, ReducibleTests, SerializableTests} +import cats.laws.discipline.{BimonadTests, CartesianTests, ReducibleTests, SerializableTests} import cats.laws.discipline.arbitrary._ import cats.kernel.laws.{GroupLaws, OrderLaws} @@ -85,9 +85,8 @@ class EvalTests extends CatsSuite { { implicit val iso = CartesianTests.Isomorphisms.invariant[Eval] checkAll("Eval[Int]", BimonadTests[Eval].bimonad[Int, Int, Int]) - - checkAll("Eval[Int]", MonadTests[Eval].monad[Int, Int, Int]) } + checkAll("Bimonad[Eval]", SerializableTests.serializable(Bimonad[Eval])) checkAll("Eval[Int]", ReducibleTests[Eval].reducible[Option, Int, Int])