Add a Continuation data type #1400

johnynek · 2016-10-04T04:14:58Z

addresses #700

This does not add ContinuationT[M[_], O, I] which can also be implemented for any monad M[_]. This should not be too hard, but will add a performance hit relative to the non-transformer version.

I may add a follow up that has ContinuationT if we feel that is useful. The main trick is to use tailRecM rather than the tailrec loop I have here.

adelbertc · 2016-10-04T05:04:54Z

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

+
+object Continuation {
+  def pure[O] = new PureBuilder[O]
+  class PureBuilder[O] {


final and private[Continuation] ? The former for sure, the latter mostly because I saw that's how we've been doing it: https://github.com/typelevel/cats/blob/master/core/src/main/scala/cats/data/OptionT.scala#L167

adelbertc · 2016-10-04T05:06:09Z

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

+import java.io.Serializable
+import scala.annotation.tailrec
+
+sealed abstract class Continuation[O, +I] extends Serializable {


extends Product with Serializable

Also Scaladoc saying Continuation[O, I] is (I => O) => O. Seeing the type parameters order reversed confused me for a second before I realized it was probably for the Monad instance

I was also going to suggest a signature of Continuation[-I, +O], unless there's some reason for the current order of types, and their variances

Assuming my algebra is right I think O could also be made covariant like you said. A separate but related question is if we want this to be variant at all since that comes with all the gotchas associated with variance and subtyping in Scala. I would vote no.

I'm guessing the order is because it makes partial application look a bit cleaner since the Monad instance is free in I as opposed to R. with Continuation[I, O] you would have Monad[Continuation[?, O]] as opposed to Monad[Continuation[O, ?]].

Good point, variances can turn into land-mines. Far easier to add them later if there is a reason, than try to remove them after the fact.

no, O is invariant. Note that it appears in the output, so it is either invariant or covariant. But it also appears in the input of the function, which makes it invariant or contravariant (see the apply method), so it is invariant.

A more direct way to see it: if you have a function (I => O) => O and say, O = String. Now due to covariance, I want to treat it as (I => Any) => Any can I do this? It is not clear at all how since internal to a function (I => String) => String we could make use of the String structure, but when I am given a function to Any, I can't do that.

So in fact, Continuation[O, +I] is the correct variance.

Now, about ordering of the parameters, I agree it is not the most intuitive order, but due to the the 2712 fix, right to left is the standard order searched when looking for single parameter type classes, so I think we are better off keeping the parameter there.

about adding co-variance, I'd really like to keep it. Contravariance causes the most problems, I think, but covariance is very useful and common especially in container code.

Can you point to the problem that covariance causes?

I'm unaware of any particular problems relating to covariance. Since I plays the role of input type in I => O, it seemed as though it would make sense to have it be -I, analogous to Function[-I, +O], however that doesn't take into account the ways it is different.

When you use a contravariant type in a contravariant position the variance is flipped.

So Continuation has an apply method as follows:

def apply(fn: I => O): O

In the context of Function1[I, O] the I is contravariant (as you point out). However this function itself is used in a contravariant position (an input parameter) to the apply method. This means that in the context of Continutation I is covariant (a contravariant type used in contravariant position is treated as covariant).

Does that make sense to you? It might help to think about the relationship between List[A] and its map method (e.g. def map[A, B](f: A => B): List[B]).

EDIT: Turns out @johnynek said the same thing below. Jinx!

adelbertc · 2016-10-04T05:06:39Z

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

+  }
+
+  private case class Const[O, I](i: I) extends Continuation[O, I]
+


Don't know if also marking these as final does anything but maybe for style consistency?

adelbertc · 2016-10-04T05:08:55Z

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

+    }
+
+    sealed abstract class Fn[-A]
+    case class ScalaFn[A](fn: A => O) extends Fn[A]


extends Product with Serializable, final case classes

adelbertc · 2016-10-04T05:09:13Z

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

+    }
+    case class Complete(get: O) extends Result
+    case class ContResult[I](runfn: (I => O) => O, fn: Fn[I]) extends Result {
+      def get: O = fn match {


extends Product with Serializable, final case classes?

the parent class is sealed, so case classes are already final, aren't they?

Yeah but it still bothers me from a style perspective ;)

do we really need to do needless work just for style? There are a lot of classes here all hidden inside a private class.

adelbertc · 2016-10-04T05:09:32Z

tests/src/test/scala/cats/tests/ContinuationTests.scala

+
+    Arbitrary(Gen.oneOf(I.arbitrary.map(call(_)), O.arbitrary.map { o => const(o) }))
+  }
+  implicit def arbCont[O, I](implicit I: Arbitrary[I], fn: Arbitrary[(I => O) => O]): Arbitrary[Continuation[O, I]] =


Should this go in cats.laws.discipline.arbitrary

johnynek · 2016-10-04T18:54:59Z

So, it is -I twice! so that makes it positive again: F => O has -F
but F = I => O so you have -(-I) = +I

On Tue, Oct 4, 2016 at 8:40 AM Erik Erlandson notifications@github.com
wrote:

@erikerlandson commented on this pull request.

In core/src/main/scala/cats/data/Continuation.scala
#1400:

@@ -0,0 +1,108 @@
+package cats
+package data
+
+import java.io.Serializable
+import scala.annotation.tailrec
+
+sealed abstract class Continuation[O, +I] extends Serializable {

I'm unaware of any particular problems relating to covariance. Since I
plays the role of input type in I => O, it seemed as though it would make
sense to have it be -I, analogous to Function[-I, +O], however that
doesn't take into account the ways it is different.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1400, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEJdvweEYzlYJ1r_l0-47_EiP7K2bukks5qwp2WgaJpZM4KNSUt
.

johnynek · 2016-10-05T00:05:25Z

@adelbertc can you explain the Product with Serializable comments here? It seems you are requesting them only on ephemeral and private classes, so I'd rather not do it unless there is really a gain. Have we written down what this gain normally is and why we want to add the extra ceremony (final + product + serializable).

codecov-io · 2016-10-05T02:06:18Z

Codecov Report

Merging #1400 into master will increase coverage by 0.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1400      +/-   ##
==========================================
+ Coverage   91.68%   91.77%   +0.08%     
==========================================
  Files         240      241       +1     
  Lines        3610     3648      +38     
  Branches       63       60       -3     
==========================================
+ Hits         3310     3348      +38     
  Misses        300      300

Impacted Files	Coverage Δ
...rc/main/scala/cats/laws/discipline/Arbitrary.scala	`90.9% <100%> (+1.25%)`	⬆️
core/src/main/scala/cats/data/Continuation.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 a392654...21f60ef. Read the comment docs.

tixxit · 2016-10-07T00:36:50Z

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

+        case _ =>
+          // This is not stack safe, we could keep finding continuations
+          // the stack depth will scale as the number of inner continuations,
+          // not flatMaps (which you can see


This comment didn't quite get finished.

DavidGregory084 · 2016-11-16T13:46:59Z

@johnynek @adelbertc
Ensuring that traits extend Product with Serializable just prevents awkwardness like this:

scala> sealed trait Foo
defined trait Foo

scala> case object Bar extends Foo
defined object Bar

scala> case object Baz extends Foo
defined object Baz

scala> List(Bar, Baz)
res0: List[Product with Serializable with Foo] = List(Bar, Baz) // Product with Serializable with Foo

Marking case classes as final only ensures that they cannot be extended, which is good since this usually breaks compiler-generated method implementations like .unapply, .copy (among other things).

I'm sure you both knew these things already, but there is no other benefit as far as I'm aware.

I'd be inclined to suggest that final case class Foo(...) and sealed trait Bar extends Product with Serializable should always be applied to classes that form part of cats' public API but perhaps aren't necessary otherwise.

johnynek · 2016-11-17T03:32:54Z

@DavidGregory084 Ahh, I had not seen the explanation for adding Product with Serializable to ADTs, but that is nice.

I agree it is nice for public classes.

13h3r · 2016-12-13T10:50:55Z

@johnynek do you need any help to complete this?

johnynek · 2016-12-13T16:02:11Z

I'm not sure there is any interest in merging. The requested edits are mechanical, but I don't want to spend time on them if actually we won't merge.

…

On Tue, Dec 13, 2016 at 00:50 Alexey Romanchuk ***@***.***> wrote: @johnynek <https://github.com/johnynek> do you need any help to complete this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1400 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdi1lClB0EkNjiuLRe8xNw7XI-sGOks5rHniPgaJpZM4KNSUt> .

erikerlandson · 2016-12-13T17:45:27Z

I'm still interested in this, however my interest has become mostly abstract. I now think that free-monads may be sufficient for my purposes. In any case, my project is completely experimental (and currently inactive) and I could just as easily include this prototype in my own code if I want to play with it some more. I don't think my side projects are sufficient justification to merge something that there's ambivalence about supporting :)

djspiewak · 2017-05-26T16:47:50Z

@johnynek I'm tempted to just close this. There doesn't seem to be that much interest in it, overall, which is unfortunate but understandable. Opinion?

reverofevil · 2017-06-03T04:24:27Z

@djspiewak Don't.

johnynek · 2017-06-03T04:28:01Z

I think we can merge it, maybe with a bit of polish. If we want to make it fully stack safe, I think we have to introduce a stack safe function type, and use that as the continuation.

…

On Fri, Jun 2, 2017 at 18:24 Philip Polkovnikov ***@***.***> wrote: @djspiewak <https://github.com/djspiewak> Don't. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1400 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEJdnQcydxQ_rTBnASXHRIzwwc7OzBBks5sAN_8gaJpZM4KNSUt> .

djspiewak · 2017-06-03T04:49:25Z

I think a non stack safe variant is probably ok. I mean, the variant you've implemented (which is basically Free of Cont) is stack safe on binds, just not on suspends. Which is fine, because callcc is not something I would expect in a continuous loop.

Add a Continuation data type

c78cdb2

stew added the in progress label Oct 4, 2016

johnynek mentioned this pull request Oct 4, 2016

Continuation monads #700

Open

add from2

0a97fae

adelbertc reviewed Oct 4, 2016

View reviewed changes

Address review comments

21f60ef

tixxit reviewed Oct 7, 2016

View reviewed changes

djspiewak mentioned this pull request Mar 26, 2017

Proposal: move tailRecM to FlatMap and remove FlatMapRec #1278

Closed

kailuowang mentioned this pull request Sep 29, 2017

[Proposal] new cats.more module #1940

Closed

johnynek closed this Sep 16, 2018

johnynek mentioned this pull request Sep 16, 2018

Add ContT monad #2506

Merged

larsrh deleted the oscar/cont branch April 11, 2020 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Continuation data type #1400

Add a Continuation data type #1400

johnynek commented Oct 4, 2016

adelbertc Oct 4, 2016 •

edited

Loading

adelbertc Oct 4, 2016 •

edited

Loading

erikerlandson Oct 4, 2016 •

edited

Loading

adelbertc Oct 4, 2016

erikerlandson Oct 4, 2016

johnynek Oct 4, 2016

johnynek Oct 4, 2016

erikerlandson Oct 4, 2016

non Oct 7, 2016 •

edited

Loading

adelbertc Oct 4, 2016 •

edited

Loading

adelbertc Oct 4, 2016

adelbertc Oct 4, 2016

erikerlandson Oct 4, 2016

adelbertc Oct 4, 2016

johnynek Oct 4, 2016

adelbertc Oct 4, 2016

johnynek commented Oct 4, 2016

@erikerlandson commented on this pull request.

johnynek commented Oct 5, 2016

codecov-io commented Oct 5, 2016 •

edited

Loading

tixxit Oct 7, 2016

DavidGregory084 commented Nov 16, 2016

johnynek commented Nov 17, 2016

13h3r commented Dec 13, 2016

johnynek commented Dec 13, 2016 via email

erikerlandson commented Dec 13, 2016

djspiewak commented May 26, 2017

reverofevil commented Jun 3, 2017

johnynek commented Jun 3, 2017 via email

djspiewak commented Jun 3, 2017

		}

		private case class Const[O, I](i: I) extends Continuation[O, I]

Add a Continuation data type #1400

Add a Continuation data type #1400

Conversation

johnynek commented Oct 4, 2016

adelbertc Oct 4, 2016 • edited Loading

Choose a reason for hiding this comment

adelbertc Oct 4, 2016 • edited Loading

Choose a reason for hiding this comment

erikerlandson Oct 4, 2016 • 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

non Oct 7, 2016 • edited Loading

Choose a reason for hiding this comment

adelbertc Oct 4, 2016 • 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

johnynek commented Oct 4, 2016

@erikerlandson commented on this pull request.

johnynek commented Oct 5, 2016

codecov-io commented Oct 5, 2016 • edited Loading

Codecov Report

Choose a reason for hiding this comment

DavidGregory084 commented Nov 16, 2016

johnynek commented Nov 17, 2016

13h3r commented Dec 13, 2016

johnynek commented Dec 13, 2016 via email

erikerlandson commented Dec 13, 2016

djspiewak commented May 26, 2017

reverofevil commented Jun 3, 2017

johnynek commented Jun 3, 2017 via email

djspiewak commented Jun 3, 2017

adelbertc Oct 4, 2016 •

edited

Loading

adelbertc Oct 4, 2016 •

edited

Loading

erikerlandson Oct 4, 2016 •

edited

Loading

non Oct 7, 2016 •

edited

Loading

adelbertc Oct 4, 2016 •

edited

Loading

codecov-io commented Oct 5, 2016 •

edited

Loading