Skip to content

Regression: betterFors doesn't remove trailing map #23409

Open
@marcinzh

Description

@marcinzh

Compiler version

3.7.0, 3.7.1-RC1, 3.7.2-RC1

Minimized example

//> using scala "3.7.2-RC1"
//> using options "-preview", "-Vprint:typer"
@main def main =
  println:
    for
      a <- List(1)
      b <- List(a)
    yield b

For comparison, the same code with older compiler version:

//> using scala "3.6.4"
//> using options "-language:experimental.betterFors", "-Vprint:typer"
@main def main =
  println:
    for
      a <- List(1)
      b <- List(a)
    yield b

Output

In 3.7.0, 3.7.1-RC1, 3.7.2-RC1 the trailing map is not removed:

@main def main: Unit =
  println(
    List.apply[Int]([1 : Int]*).flatMap[Int]((a: Int) =>
      List.apply[Int]([a : Int]*).map[Int]((b: Int) => b))
  )

In 3.6.4 the trailing map is removed:

@main def main: Unit =
  println(
    List.apply[Int]([1 : Int]*).flatMap[Int]((a: Int) =>
      List.apply[Int]([a : Int]*))
  )

Expectation

Have the trailing map removed.

Here is more complex case. The lack of trailing map removal causes 20% lower scores in this microbenchmark suite. Applies to all effect systems.

Activity

sjrd

sjrd commented on Jun 24, 2025

@sjrd
Member

This transformation was moved to a later phase. If you print with -Vprint:dropForMap, you will see that the call to map is gone:

$ cs launch scala3-repl:3.7.1 -- -Vprint:dropForMap -preview
Welcome to Scala 3.7.1 (1.8.0_452, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
                                                                                                              
scala>   println:
     |     for
     |       a <- List(1)
     |       b <- List(a)
     |     yield b
     | 
List(1)
[[syntax trees at end of MegaPhase{crossVersionChecks, firstTransform, checkReentrant, elimPackagePrefixes, cookComments, checkLoopingImplicits, betaReduce, inlineVals, expandSAMs, elimRepeated, refchecks, dropForMap}]] // rs$line$1
package <empty> {
  final lazy module val rs$line$1: rs$line$1 = new rs$line$1()
  @SourceFile("rs$line$1") final module class rs$line$1() extends Object() {
    private def writeReplace(): AnyRef =
      new scala.runtime.ModuleSerializationProxy(classOf[rs$line$1.type])
    val res0: Unit =
      println(
        List.apply[Int]([1 : Int]).flatMap[Int]((a: Int) =>
          List.apply[Int]([a : Int]))
      )
  }
}
WojciechMazur

WojciechMazur commented on Jun 24, 2025

@WojciechMazur
Contributor

The mentioned change was introduce in #22619 - is it possible that cases in benchmarks where testing case where result of trailing map was different from previous transformation?

marcinzh

marcinzh commented on Jun 24, 2025

@marcinzh
Author
//> using scala "3.7.2-RC1"
//> using dep "org.typelevel::cats-mtl::1.5.0"
//> using options "-preview", "-Vprint:dropForMap"

// using scala "3.6.4"
// using dep "org.typelevel::cats-mtl::1.5.0"
// using options "-language:experimental.betterFors", "-Vprint:typer"

import cats.{Monad, Eval}
import cats.data.State
import cats.implicits._
import cats.mtl.Stateful

def program[F[_]: Monad](using S: Stateful[F, Int]): F[Int] =
  for
    s <- S.get
    x <- s.pure
  yield x

@main def main =
  println:
    program[[X] =>> State[Int, X]]
    .run(42)

The trailing map is present when compiled with 3.7.2-RC2:

def program[F[_$1]](using evidence$1: cats.Monad[F],
  S: cats.mtl.Stateful[F, Int]): F[Int] =
  cats.implicits.toFlatMapOps[F, Int](S.get)(evidence$1).flatMap[Int]((
    s: Int) =>
    cats.implicits.toFunctorOps[F, Int](
      cats.implicits.catsSyntaxApplicativeId[Int](s).pure[F](evidence$1))(
      evidence$1).map[Int]((x: Int) => x)
  )

The trailing map is removed when compiled with 3.6.4:

def program[F[_ >: Nothing <: Any] >: Nothing <: Any](using 
  evidence$1: cats.Monad[F], S: cats.mtl.Stateful[F, Int]): F[Int] =
  cats.implicits.toFlatMapOps[F, Int](S.get)(evidence$1).flatMap[Int]((
    s: Int) =>
    cats.implicits.catsSyntaxApplicativeId[Int](s).pure[F](evidence$1))
marcinzh

marcinzh commented on Jun 24, 2025

@marcinzh
Author

@WojciechMazur I'm not sure. Some effect systems could be accused of doing something weird with the types. ZIO, Turbolift and Kyo rely on variance and intersection types. Also in Kyo and Turbolift map and flatMap are macros (a microoptimization to save an allocation).

But the Cats-MTL case does none of these, yet it appears the betterFor problem applies to it as well.

som-snytt

som-snytt commented on Jun 24, 2025

@som-snytt
Contributor

Debug for the failing test says it has the attachment but avoids changing the result type.

TrailingForMap? true in cats.implicits.toFunctorOps[F, Int](
  cats.implicits.catsSyntaxApplicativeId[Int](s).pure[F](evidence$1))(evidence$1
  ).map[Int]((x: Int) => x)
No rewrite because !(cats.Functor.Ops[F, Int]{type TypeClassType = cats.Functor[F]} =:= F[Int])

Probably it has to unwrap the conversion.

I haven't looked at what 3.6 does, but I will see what changed and update here.

sjrd

sjrd commented on Jun 24, 2025

@sjrd
Member

3.6 blindly removed the map based on syntax only, without type information. That caused a lot of regressions, so we changed the spec. Now typer always inserts map and types it. dropForMap drops the ones that do not break the inferred result type.

som-snytt

som-snytt commented on Jun 24, 2025

@som-snytt
Contributor

Thanks, I just saw. I'll see if there is a coping mechanism, after I finish this coffee.

(I was concerned I might have touched something in Desugar for linting pattern vars in for comprehensions, which also depends on attachments.)

linked a pull request that will close this issue on Jun 24, 2025
self-assigned this
on Jun 24, 2025
marcinzh

marcinzh commented on Jun 25, 2025

@marcinzh
Author

I wonder if the compiler considers types Foo[T], Foo[T & Any] or Foo[T & T] as the same type, in the #22619 check?

marcinzh

marcinzh commented on Jun 25, 2025

@marcinzh
Author

As I understand, the #23416 fixes the Cats-MTL case.

ZIO, Turbolift and Kyo are quite different. I'm posting minimized versions for each of these effect systems, hoping it would help their cases get covered too:

ZIO:

//> using scala "3.7.2-RC1"
//> using dep "dev.zio::zio:2.1.19"
//> using options "-preview" "-Vprint:dropForMap"

// using scala "3.6.4"
// using dep "dev.zio::zio:2.1.19"
// using options "-language:experimental.betterFors" "-Vprint:typer"

import zio._

def program: ZIO[ZState[Int], Nothing, Int] =
  for
    s <- ZIO.getState[Int]
    x <- ZIO.succeed(s)
  yield x

@main def main =
  println:
    ZIO.stateful(42)(program).unsafeRunZio

extension [A](thiz: ZIO[Any, Nothing, A])
  def unsafeRunZio: A =
    Unsafe.unsafe(implicit _ => Runtime.default.unsafe.run(thiz).getOrThrowFiberFailure())

Turbolift:

//> using scala "3.7.2-RC1"
//> using dep "io.github.marcinzh::turbolift-core:0.114.0"
//> using options "-preview" "-Vprint:dropForMap"

// using scala "3.6.4"
// using dep "io.github.marcinzh::turbolift-core:0.114.0"
// using options "-language:experimental.betterFors" "-Vprint:typer"

import turbolift.!!
import turbolift.effects.State

case object MyState extends State[Int]
type MyState = MyState.type

def program: Int !! MyState =
  for
    s <- MyState.get
    x <- !!.pure(s)
  yield x

@main def main =
  println:
    program.handleWith(MyState.handler(0)).run

Kyo:

//> using scala "3.7.2-RC1"
//> using dep "io.getkyo::kyo-core::0.19.0"
//> using options "-preview" "-Vprint:dropForMap"

// using scala "3.6.4"
// using dep "io.getkyo::kyo-core::0.18.0"
// using options "-language:experimental.betterFors" "-Vprint:typer"

// Note: kyo is compild against the latest scala version, so we need to use v18 and v19 respectively

import kyo.*

def pure[A](a: A): A < Any = a

def program: Int < Var[Int] =
  for
    s <- Var.get[Int]
    x <- pure(s)
  yield x

@main def main =
  println:
    Var.run(42)(program).eval

Additionally, here is a zero-dependency version that mimics the common typing technique used in each of those 3 effect systems:

IGNORE! this minimization is incorrect

//> using scala "3.7.2-RC1"
//> using options "-preview" "-Vprint:dropForMap"

// using scala "3.6.4"
// using options "-language:experimental.betterFors" "-Vprint:typer"


sealed trait Effect[+A, -U]:
  final def map[B](f: A => B): Effect[B, U] = flatMap(a => Pure(f(a)))
  final def flatMap[B, V <: U](f: A => Effect[B, V]): Effect[B, V] = FlatMap(this, f)

object Effect:
  def pure[A](a: A): Effect[A, Any] = Pure(a)

  def run[A](ma: Effect[A, Any]): A =
    ma match
      case Pure(a) => a
      case FlatMap(mb, k) => run(k(run(mb)))

final case class Pure[A](a: A) extends Effect[A, Any]
final case class FlatMap[A, B, U](that: Effect[A, U], f: A => Effect[B, U]) extends Effect[B, U]

type Stateish = Stateish.type
object Stateish:
  case object Get extends Effect[Int, Stateish]
  def get: Effect[Int, Stateish] = Get


  def handle[A, U](initial: Int)(ma: Effect[A, U & Stateish]): Effect[A, U] =
    def loop[A](ma: Effect[A, U & Stateish]): Effect[A, U] =
      ma match
        case Pure(a) => Pure(a)
        case Stateish.Get => Pure(initial)
        case FlatMap(mb: Effect[tB, tU & Stateish], k) => mb match
          case Pure(b) => loop(k(b))
          case Stateish.Get => loop(k(initial.asInstanceOf[tB]))
          case FlatMap(mc, j) => loop(mc.flatMap(c => j(c).flatMap(k)))
    loop(ma)


def program: Effect[Int, Stateish] =
  for
    s <- Stateish.get
    x <- Effect.pure(s)
  yield x

@main def main =
  println:
    Effect.run(Stateish.handle(42)(program))

5 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @som-snytt@sjrd@WojciechMazur@marcinzh

    Issue actions

      Regression: `betterFors` doesn't remove trailing `map` · Issue #23409 · scala/scala3