Add Defer typeclass, laws and implementations #2279

johnynek · 2018-06-04T13:56:33Z

This seems like a little thing, but I was able to use it to implement ContT[M, A, B], a wrapper for (B => M[A]) => M[A] and pass all the laws using the naive map/flatMap (with cats.data.AndThen) and the following tailRecM:

 def tailRecM[M[_], A, B, C](a: A)(fn: A => ContT[M, C, Either[A, B]])(implicit M: Defer[M]): ContT[M, C, B] =
  ContT[M, C, B] { cb: (B => M[C]) => 
     def go(a: A): M[C] =
        fn(a).run {
          case Left(a) => M.defer(go(a))
          case Right(b) => cb(b)
        }
       go(a)
    }

I think this is pretty clear that this is an interesting typeclass along with the fact that we have so many implementations.

closes #2273

johnynek · 2018-06-04T14:09:09Z

I think this strategy can allow us to extend ContT to any cats.Monad: in the ContT.tailRecM loop, lift M to Free, use defer on Free, then finally run the Free to get back to M at the end.

kailuowang · 2018-06-04T14:19:46Z

core/src/main/scala/cats/data/IndexedReaderWriterStateT.scala

@@ -462,6 +462,12 @@ private[data] sealed abstract class IRWSTInstances extends IRWSTInstances1 {
      implicit def L: Monoid[L] = L0
    }

+  implicit def catsDataDefer[F[_], E, L, SA, SB](implicit F: Defer[F]): Defer[IndexedReaderWriterStateT[F, E, L, SA, SB, ?]] =


totally trivial nit: the naming convention has a ForIRWST suffix. Although I don't think it matters much since there is no risk of naming conflict here.

yep, that was a goof up.

kailuowang · 2018-06-04T15:25:14Z

tests/src/test/scala/cats/tests/FunctionSuite.scala

@@ -30,13 +30,35 @@ class FunctionSuite extends CatsSuite {
  import Helpers._

  checkAll("Function0[Int]", SemigroupalTests[Function0].semigroupal[Int, Int, Int])
+  // TODO: make an binary compatible way to do this
+  implicit val deferFunction0: Defer[Function0] =


One way to do this that I can think of is to just write this in a new cats.instance.FunctionInstancesBinaryCompatible1 trait and then add a new cats.instances.AllInstancesBinaryCompatible1 trait to extend it. And let cats.instances.all and cats.implicits extend it. (basically the same way as how we extend syntax while reserving binary compatibility)

kailuowang · 2018-06-04T15:45:08Z

laws/src/main/scala/cats/laws/DeferLaws.scala

+      if (c <= 0) F.defer(fa(()))
+      else F.defer(loop(c - 1))
+
+    loop(1000000) <-> (fa(()))


this number might be so too big that all jobs in build ran OOM.

codecov-io · 2018-06-05T15:03:29Z

Codecov Report

Merging #2279 into master will increase coverage by <.01%.
The diff coverage is 96.15%.

@@            Coverage Diff             @@
##           master    #2279      +/-   ##
==========================================
+ Coverage   95.08%   95.08%   +<.01%     
==========================================
  Files         343      345       +2     
  Lines        5935     5987      +52     
  Branches      218      217       -1     
==========================================
+ Hits         5643     5693      +50     
- Misses        292      294       +2

Impacted Files	Coverage Δ
core/src/main/scala/cats/data/WriterT.scala	`91.52% <100%> (+0.14%)`	⬆️
...c/main/scala/cats/laws/discipline/DeferTests.scala	`100% <100%> (ø)`
core/src/main/scala/cats/data/Nested.scala	`94.44% <100%> (+0.15%)`	⬆️
core/src/main/scala/cats/data/Tuple2K.scala	`91.11% <100%> (+0.41%)`	⬆️
core/src/main/scala/cats/Eval.scala	`98.78% <100%> (+0.03%)`	⬆️
core/src/main/scala/cats/data/IndexedStateT.scala	`89.47% <100%> (+0.22%)`	⬆️
free/src/main/scala/cats/free/Free.scala	`91.78% <100%> (+0.23%)`	⬆️
core/src/main/scala/cats/data/OptionT.scala	`97.67% <100%> (+0.05%)`	⬆️
core/src/main/scala/cats/data/Kleisli.scala	`97.11% <100%> (+0.05%)`	⬆️
core/src/main/scala/cats/instances/function.scala	`100% <100%> (ø)`	⬆️
... and 6 more

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 e15188b...c86ece0. Read the comment docs.

kubukoz · 2018-06-05T20:58:33Z

core/src/main/scala/cats/data/IorT.scala

@@ -435,6 +435,12 @@ private[data] abstract class IorTInstances extends IorTInstances1 {
      Monad[IorT[M, E, ?]]
    }
  }
+
+  implicit def catsDataDeferForIor[F[_], E](implicit F: Defer[F]): Defer[IorT[F, E, ?]] =


missing T after catsDataDeferForIor?

looks like we are still missing the T.

I got you. https://github.com/typelevel/cats/pull/2688/files

kubukoz · 2018-06-05T21:03:13Z

Why not call it Suspend to keep consistent with Sync's suspend method? You mentioned yourself that it could be an alias to it, but is there a reason why we can't just make Sync extend Defer/Suspend and provide delay?

EDIT: now I see that #2273's name originally mentioned a Suspend typeclass, so I guess the decision to rename was influenced by Suspend being too specific (side effects, not deferred computations)...

Can we rename suspend then? :D

kailuowang · 2018-06-11T14:36:23Z

@johnynek do you want this in the coming 1.2 release?

johnynek · 2018-06-12T06:27:06Z

@kailuowang I would.

kailuowang · 2018-06-19T20:01:09Z

#2284 adds a FunctionInstancesBinCompat0, shall we merge that one first so that you can add your Function Defer instance there?

johnynek · 2018-06-19T20:11:56Z

@kailuowang sounds great.

LukaJCB

LGTM, thanks! 👍

alexandru

[deleted comment]

alexandru · 2018-06-25T12:45:32Z

laws/src/main/scala/cats/laws/DeferLaws.scala

+    F.defer(fa(())) <-> fa(())
+
+  def deferDoesNotEvaluate[A](fa: Unit => F[A]): IsEq[Boolean] = {
+    var evaluated = false


Unsuspended vars in laws make me feel uneasy. First of all I fear that such laws will not have a long life.

To fix it, you can defer the whole thing:

def deferDoesNotEvaluate[A](f: Boolean => F[A]): IsEq[Boolean] = { val lh = F.defer { var evaluated = false F.defer { evaluated = true f(evaluated) } f(evaluated) } lh <-> f(false) }

I’m nervous that if you pass a function that ignores the Boolean this is true trivially.

alexandru · 2018-06-25T12:47:40Z

laws/src/main/scala/cats/laws/DeferLaws.scala

+  def deferIsStackSafe[A](fa: Unit => F[A]): IsEq[F[A]] = {
+    def loop(c: Int): F[A] =
+      if (c <= 0) F.defer(fa(()))
+      else F.defer(loop(c - 1))


This law is interesting, but in case an F[_] provides such a powerful defer, what about the behavior of flatMap? Shouldn't flatMap also be stack safe if defer is?

Also the question on my mind would be: is defer any relevant without a Monad restriction? Or in other words, do we have any data type that can implement Defer, but not Monad?

Note that we also have a StackSafeMonad.

That's a good question about making the law stronger: if there is a monad, flatMap must be stack safe. The function cases wouldn't pass this law.

As to what might not be a monad: command line parsing is often done with an applicative, not a monad (see optparse-applicative library in haskell or https://github.com/bkirwi/decline for a cats version). Those types can still have a lawful Defer.

If we have Applicative examples without a Monad definition, I guess that's fine 👍

alexandru · 2018-06-25T12:52:53Z

laws/src/main/scala/cats/laws/DeferLaws.scala

+      if (c <= 0) F.defer(fa(()))
+      else F.defer(loop(c - 1))
+
+    val cnt = if (Platform.isJvm) 20000 else 2000


In my experience these arbitrary values hurt for F[_] data types that have an expensive flatMap, for example streaming data types, because you're not talking of a single event being emitted, but multiple ones, per flatMap.

In Cats-Effect what I did was to introduce an iterations: Int in SyncLaws (see sample) and an implicit Parameters in SyncTests (see definition and usage). Otherwise those tests would choke for Monix's Iterant for example, especially when executed on Travis.

And in absence of that, I'd use smaller values:

val cnt = if (Platform.isJvm) 10000 else 1000

I don’t see a compelling reason why this is only 2x too large. Can you give an idea?

kailuowang · 2018-07-16T17:24:33Z

@johnynek do you happen to have time to finish this one up? or can this wait until the next release?

johnynek · 2018-07-16T18:37:42Z

@kailuowang what needs to happen to merge?

I didn't want to take the time to cut the counts in half suggested above.

kailuowang · 2018-07-16T19:00:57Z

Do you want to respond to this feedback?

Also the instance of Function1 has a place now, thanks to the FunctionInstancesBinCompat0 added by #2284

kailuowang · 2018-07-20T15:30:21Z

@johnynek I added a commit to move Function0 instance and added Function1 instance. would you review?

johnynek · 2018-07-20T18:25:49Z

This is failing Mima:

[info] Cats core: found 2 potential binary incompatibilities while checking against org.typelevel:cats-core_2.12:1.0.0 
[error]  * abstract synthetic method cats$instances$Function0Instances$_setter_$catsSddDeferForFunction0_=(cats.Defer)Unit in interface cats.instances.Function0Instances is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("cats.instances.Function0Instances.cats$instances$Function0Instances$_setter_$catsSddDeferForFunction0_=")
[error]  * abstract method catsSddDeferForFunction0()cats.Defer in interface cats.instances.Function0Instances is present only in current version
[error]    filter with: ProblemFilters.exclude[ReversedMissingMethodProblem]("cats.instances.Function0Instances.catsSddDeferForFunction0")

I'm not sure how badly we need the function instances anyway. :/ Sorry this is so much work. Unfortunately I am buried between work and travel. I'm sorry.

kailuowang · 2018-07-20T18:36:27Z

@johnynek mima should've been fixed by the latest commit. It was me being dumb and totally forgot about it.

kailuowang · 2018-07-20T18:57:28Z

@LukaJCB do you want to take another quick look at the commits I made?

LukaJCB · 2018-07-21T13:08:03Z

LGTM 👍

johnynek added 3 commits June 1, 2018 19:08

Add Defer typeclass, laws and instances

a9aed70

require stack safety on Defer

36f4252

rewrite typeclass to type class

6f9339a

kailuowang reviewed Jun 4, 2018

View reviewed changes

johnynek added 2 commits June 4, 2018 19:43

address review, fix the build

2b07112

Use Free[Option, A] to avoid a resolution bug

fd43163

johnynek mentioned this pull request Jun 5, 2018

Monad[Free[Id, ?]] cannot be found #2281

Closed

reduce size for non-jvm platforms

c1ebba7

kubukoz reviewed Jun 5, 2018

View reviewed changes

kailuowang added this to the 1.2 milestone Jun 12, 2018

kailuowang added the enhancement label Jun 12, 2018

LukaJCB previously approved these changes Jun 26, 2018

View reviewed changes

alexandru reviewed Jun 26, 2018

View reviewed changes

djspiewak mentioned this pull request Jul 5, 2018

Add higher kinded versions of Eq, Order, Show #2308

Closed

kailuowang added 2 commits July 20, 2018 11:28

Merge remote-tracking branch 'upstream/master' into oscar/defer

f3df8ef

moved Function0 to BC trait /added Function1 instance

3e25d2c

kailuowang dismissed LukaJCB’s stale review via 3e25d2c July 20, 2018 15:29

fix build

c86ece0

kailuowang approved these changes Jul 20, 2018

View reviewed changes

kailuowang merged commit 6f049ad into master Jul 22, 2018

LukaJCB deleted the oscar/defer branch July 22, 2018 22:34

johnynek mentioned this pull request Jul 3, 2019

Bring back Applicative.pureEval or provide equivalent #1316

Closed

johnynek mentioned this pull request Aug 9, 2021

Optimize traverseWithChain for StackSafeMonad(s) #3790

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Defer typeclass, laws and implementations #2279

Add Defer typeclass, laws and implementations #2279

johnynek commented Jun 4, 2018

johnynek commented Jun 4, 2018

kailuowang Jun 4, 2018

johnynek Jun 4, 2018

kailuowang Jun 4, 2018 •

edited

Loading

kailuowang Jun 4, 2018

codecov-io commented Jun 5, 2018 •

edited

Loading

kubukoz Jun 5, 2018

johnynek Jul 20, 2018

andyscott Jan 7, 2019

kubukoz commented Jun 5, 2018 •

edited

Loading

kailuowang commented Jun 11, 2018

johnynek commented Jun 12, 2018

kailuowang commented Jun 19, 2018

johnynek commented Jun 19, 2018

LukaJCB left a comment

alexandru left a comment •

edited

Loading

alexandru Jun 25, 2018

johnynek Jul 16, 2018

alexandru Jun 25, 2018

johnynek Jun 26, 2018

alexandru Jun 27, 2018

alexandru Jun 25, 2018

johnynek Jul 16, 2018

kailuowang commented Jul 16, 2018

johnynek commented Jul 16, 2018

kailuowang commented Jul 16, 2018

kailuowang commented Jul 20, 2018

johnynek commented Jul 20, 2018

kailuowang commented Jul 20, 2018

kailuowang commented Jul 20, 2018

LukaJCB commented Jul 21, 2018

Add Defer typeclass, laws and implementations #2279

Add Defer typeclass, laws and implementations #2279

Conversation

johnynek commented Jun 4, 2018

johnynek commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kailuowang Jun 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jun 5, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kubukoz commented Jun 5, 2018 • edited Loading

kailuowang commented Jun 11, 2018

johnynek commented Jun 12, 2018

kailuowang commented Jun 19, 2018

johnynek commented Jun 19, 2018

LukaJCB left a comment

Choose a reason for hiding this comment

alexandru left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kailuowang commented Jul 16, 2018

johnynek commented Jul 16, 2018

kailuowang commented Jul 16, 2018

kailuowang commented Jul 20, 2018

johnynek commented Jul 20, 2018

kailuowang commented Jul 20, 2018

kailuowang commented Jul 20, 2018

LukaJCB commented Jul 21, 2018

kailuowang Jun 4, 2018 •

edited

Loading

codecov-io commented Jun 5, 2018 •

edited

Loading

kubukoz commented Jun 5, 2018 •

edited

Loading

alexandru left a comment •

edited

Loading