Skip to content

Regression: betterFors doesn't remove trailing map #23409

@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.)

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))
som-snytt

som-snytt commented on Jun 26, 2025

@som-snytt
Contributor

Thanks for the minimization.

Am I right that you're asking about the residual flatMap? I looked briefly this morning before I was properly awake but didn't get back to it.

<       Stateish.get.flatMap[Int, Stateish]((s: Int) => Effect.pure[Int](s))
---
>       Stateish.get.flatMap[Int, Stateish]((s: Int) =>
>         Effect.pure[Int](s).map[Int]((x: Int) => x))
marcinzh

marcinzh commented on Jun 26, 2025

@marcinzh
Author

My bad, the zero-dependency failed to be a minimization.

I suspected that the intersection type, which grows with each flatMap (a common feature of ZIO, Turbolift and Kyo), might have been responsible for triggerring the "leave trailing map intact" path:

	get:   Effect[Int, Stateish]
	pure:  Effect[Int, Any]
marcinzh

marcinzh commented on Jun 26, 2025

@marcinzh
Author

Another zero-dependency minimization:

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

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

final class Implicit()

final class Id[+A](val value: A):
  def map[B](f: A => B)(using Implicit): Id[B] = Id(f(value))
  def flatMap[B](f: A => Id[B]): Id[B] = f(value)
  def run: A = value

def program(using Implicit) =
  for
    a <- Id(42)
    x <- Id(a + 1)
  yield x

@main def main =
  given Implicit = Implicit()
  println(program.run)

The key is the implicit parameter in definition of map.

This applies to ZIO and Kyo: they use implicit parameter for stuff like type tags and tracing.

In Turbolift map doesn't have implicit parameters, but it's an inline function. Same in Kyo.

som-snytt

som-snytt commented on Jun 28, 2025

@som-snytt
Contributor

The second commit in the PR is for trailing implicit args following the function arg.

(That also drops the map in for (x <- Future(42)) yield x.)

I'll take a look at turbolift, but it involves transforming the inlined expression. Maybe that will prove easy enough.

(I made a start by re-using the existing proxy.)

som-snytt

som-snytt commented on Jun 29, 2025

@som-snytt
Contributor

Worth mentioning I used to turn off directives by changing using to abusing, but now I use the nicer technique of just adding a leading slash. It's a small inconvenience that directives must be on top lines and unmixed. (Partest would always examine 10 lines.)

///> using scala 3.6.4

Now,

    def program: turbolift.Computation[Int, MyState] =
      {
        val Computation_this:
          (MyState.get : turbolift.Computation[Int, MyState.type]) = MyState.get
        {
          final class $anon() extends
            turbolift.ComputationCases.FlatMap[Int, Int, MyState.type](
            Computation_this) {
            override def apply(a: Int): turbolift.Computation[Int, MyState.type]
               =
              {
                val Computation_this: turbolift.Computation[Int, Any] =
                  turbolift.!!.pure[Int](a)
                Computation_this
              }
          }
          new turbolift.ComputationCases.FlatMap[Int, Int, MyState.type] {...}()
            :turbolift.ComputationCases.FlatMap[Int, Int, MyState.type]
        }:turbolift.Computation[Int, MyState.type]
      }

was 3.6

    @experimental("Added by -language:experimental.experimental.betterFors") def
       program: turbolift.Computation[Int, MyState] =
      {
        val Computation_this:
          (MyState.get : turbolift.Computation[Int, MyState.type]) = MyState.get
        {
          final class $anon() extends
            turbolift.ComputationCases.FlatMap[Int, Int, MyState.type](
            Computation_this) {
            override def apply(a: Int): turbolift.Computation[Int, MyState.type]
               = turbolift.!!.pure[Int](a)
          }
          new turbolift.ComputationCases.FlatMap[Int, Int, MyState.type] {...}()
            :turbolift.ComputationCases.FlatMap[Int, Int, MyState.type]
        }:turbolift.Computation[Int, MyState.type]
      }

I'm not likely to look at kyo.

added this to the 3.8.0 milestone on Oct 28, 2025
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

Relationships

None yet

    Participants

    @som-snytt@sjrd@WojciechMazur@marcinzh

    Issue actions

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