OOM with endless recursion

[scf37]

Looking at #89 it seems this issue have been fixed at least once.

import cats.effect.{IO, Sync}
import cats.implicits._
object Boo {
  def main(args: Array[String]): Unit = {

    def app[F[_]: Sync](c: Int): F[Unit] = for {
      _ <- Sync[F].delay(if (c % 1000000 == 0) {println(c)})
      _ <- app(c + 1)
    } yield ()

    def app2[F[_]: Sync](c: Int): F[Unit] =
      Sync[F].delay(if (c % 1000000 == 0) {println(c)})
      .flatMap(_ => app2(c + 1))

    app2[IO](0).unsafeRunSync()
  }
}

app2 works fine. app leaks memory.

[djspiewak]

Known issue. I'm tempted to put this into the readme. The solution is to use this: https://github.com/oleg-py/better-monadic-for Basically, scalac's default desugaring of for-comprehensions is terrible and suffers from problems like this one.

[djspiewak]

Actually, this makes me think about Dotty, a bit. casts summon @smarter I'm assuming dotc's desugaring of for is identical to nsc's. Would it be possible to get something analogous to --kind-projector to address this kind of use-case? The default desugaring is just really really bad, and a ton of pure effectful code is going to break (pretty much anything using for-comprehensions with fs2, for example) in the Scala 3 upgrade if we don't have some sort of answer to this.

Like, one possible answer is a scalafix that just removes for-comprehensions from codebases, but that's not particularly pleasant.

[smarter]

Would it be possible to get something analogous to --kind-projector to address this kind of use-case?

Unlikely. We're trying very hard to avoid having language flags that change language semantics since those can lead to so many issues (e.g., copy-pasting some code in a different project and having it do something different) and it quickly becomes infeasible to test and debug the 2^N combinations of N language flags.

However, we would love to get improved for comprehensions in Scala 3, but don't have the resources to work on it ourselves at EPFL. The current blocker is that we really ought to figure out what to do with withFilter before trying to change other things: https://contributors.scala-lang.org/t/pre-sip-improve-for-comprehensions-functionality/3509/21?u=smarter scala/scala3#2573

[neko-kai]

Format it like this:

def app[F[_]: Sync](c: Int): F[Unit] = (for {
      _ <- Sync[F].delay(if (c % 1000000 == 0) {println(c)})
    } yield app(c + 1)).flatMap(next => next)

For scalafix removing for-comprehension entirely is unnecessary, it may substitute trailing map by trailing flatMap as above, but leave the rest intact.

[ritschwumm]

just a wild thought:

for {
  // no filter involved, this just results in a MatchError if the generator does not generate pairs
  val (a,b) <- Vector(1->2, 2->3)
}
yield ...

[djspiewak]

@neko-kai That does fix the issue, but it's still a little annoying. Like, obviously there are answers here, it's just frustrating that the intuitive use of a language construct can give rise to these scenarios. BMF fixes that, and if the chips fall well on that Dotty issue, perhaps Scala 3 could out of the box.

[djspiewak]

Going to close this because there's not much that can be done within Cats Effect itself. It's on the radar of the Scala 3 team though, and BMF is a viable solution in Scala 2. We just need to bring more visibility to each.