Skip to content

Commit 4cdb87a

Browse files
Update SIPs state
1 parent d7bad93 commit 4cdb87a

File tree

2 files changed

+106
-6
lines changed

2 files changed

+106
-6
lines changed

_sips/sips/better-fors.md

+104-4
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,11 @@ title: SIP-62 - For comprehension improvements
1010

1111
## History
1212

13-
| Date | Version |
14-
|---------------|--------------------|
15-
| June 6th 2023 | Initial Draft |
16-
| Feb 15th 2024 | Reviewed Version |
13+
| Date | Version |
14+
|---------------|------------------------|
15+
| June 6th 2023 | Initial Draft |
16+
| Feb 15th 2024 | Reviewed Version |
17+
| Nov 21th 2024 | Addendum for change 3. |
1718

1819
## Summary
1920

@@ -333,6 +334,60 @@ doSth(arg).map { a =>
333334

334335
This change is strictly an optimization. This allows for the compiler to get rid of the final `map` call, if the yielded value is the same as the last bound pattern. The pattern can be either a single variable binding or a tuple.
335336

337+
This optimization should be done after type checking (e.g. around first transform). See the reasons to why it cannot be done in desugaring in [here](#previous-design-in-desugaring).
338+
339+
We propose an approach where an attachment (`TrailingForMap`) is attached to the last `map` `Apply` node. After that, a later phase will look for `Apply` nodes with this attachment and possibly remove the `map` call.
340+
341+
The condition for allowing to remove the last map call (for a binding `pat <- gen yield pat1`) are as follows:
342+
- `pat` is (syntactically) equivalent to `pat1` ($pat =_{s} pat1$)
343+
344+
where
345+
346+
$x =_{s} x, \text{if x is a variable reference}$
347+
348+
$x =_{s} (), \text{if x is a variable reference of type Unit}$
349+
350+
$(x_1, ..., x_n) =_{s} (y_1, ..., y_n) \iff \forall i \in n.\; x_i =_{s} y_i$
351+
352+
This means that the two patterns are equivalent if they are the same variable, if they are tuples of the same variables, or if one is a variable reference of type `Unit` and the other is a `Unit` literal.
353+
- `pat` and `pat1` have the same types (`pat.tpe` =:= `pat1.tpe`)
354+
355+
##### Changes discussion
356+
357+
This adresses the problem of changing the resulting type after removing trailing `map` calls.
358+
359+
There are two main changes compared to the previous design:
360+
1. Moving the implementation to the later phase, to be able to use the type information and explicitly checking that the types are the same.
361+
2. Allowing to remove the last `map` call if the yielded value is a `Unit` literal (and obviously the type doesn't change).
362+
363+
The motivation for the second change is to avoid potential memory leaks in effecting loops. e.g.
364+
365+
```scala
366+
//> using scala 3.3.3
367+
//> using lib "dev.zio::zio:2.1.5"
368+
369+
import zio.*
370+
371+
def loop: Task[Unit] =
372+
for
373+
_ <- Console.print("loop")
374+
_ <- loop
375+
yield ()
376+
377+
@main
378+
def run =
379+
val runtime = Runtime.default
380+
Unsafe.unsafe { implicit unsafe =>
381+
runtime.unsafe.run(loop).getOrThrowFiberFailure()
382+
}
383+
```
384+
385+
This kind of effect loop is pretty commonly used in Scala FP programs and often ends in `yield ()`.
386+
387+
The problem with the desugaring of this for-comprehensions is that it leaks memory because the result of `loop` has to be mapped over with `_ => ()`, which often does nothing.
388+
389+
##### Previous design (in desugaring)
390+
336391
One desugaring rule has to be modified for this purpose.
337392

338393
```scala
@@ -356,6 +411,46 @@ will just be desugared to
356411
List(1, 2, 3)
357412
```
358413

414+
**Cause of change**
415+
416+
This design ended up breaking quite a few existing projects in the open community build run.
417+
418+
For example, consider the following code:
419+
420+
```scala
421+
//> using scala 3.nightly
422+
423+
import scala.language.experimental.betterFors
424+
425+
case class Container[A](val value: A) {
426+
def map[B](f: A => B): Container[B] = Container(f(value))
427+
}
428+
429+
sealed trait Animal
430+
case class Dog() extends Animal
431+
432+
def opOnDog(dog: Container[Dog]): Container[Animal] =
433+
for
434+
v <- dog
435+
yield v
436+
```
437+
438+
With the new desugaring, the code gave an error about type mismatch.
439+
440+
```scala
441+
-- [E007] Type Mismatch Error: /home/kpi/bugs/better-fors-bug.scala:13:2 -------
442+
13 | for
443+
| ^
444+
| Found: (dog : Container[Dog])
445+
| Required: Container[Animal]
446+
14 | v <- dog
447+
15 | yield v
448+
|
449+
| longer explanation available when compiling with `-explain`
450+
```
451+
452+
This is because the container is invariant. And even though the last `map` was an identity function, it was used to upcast `Dog` to `Animal`.
453+
359454
### Compatibility
360455

361456
This change may change the semantics of some programs. It may remove some `map` calls in the desugared code, which may change the program semantics (if the `map` implementation was side-effecting).
@@ -372,10 +467,15 @@ yield a + b
372467

373468
As far as I know, there are no widely used Scala 3 libraries that depend on the desugaring specification of `for`-comprehensions.
374469

470+
The only Open community build library that failed because of the change to the desugaring specification is [`avocADO`](https://github.com/VirtusLab/avocado).
471+
375472
## Links
376473

377474
1. Scala contributors discussion thread (pre-SIP): https://contributors.scala-lang.org/t/pre-sip-improve-for-comprehensions-functionality/3509/51
378475
2. Github issue discussion about for desugaring: https://github.com/lampepfl/dotty/issues/2573
379476
3. Scala 2 implementation of some of the improvements: https://github.com/oleg-py/better-monadic-for
380477
4. Implementation of one of the simplifications: https://github.com/lampepfl/dotty/pull/16703
381478
5. Draft implementation branch: https://github.com/dotty-staging/dotty/tree/improved-fors
479+
6. Minimized issue reproducing the problem with the current desugaring: https://github.com/scala/scala3/issues/21804
480+
7. (empty :sad:) Contributors thread about better effect loops with for-comprehensions: https://contributors.scala-lang.org/t/pre-sip-sip-62-addition-proposal-better-effect-loops-with-for-comprehensions/6759
481+
8. Draft implementation of dropping the last map call after type checking (only for `Unit` literals): https://github.com/KacperFKorban/dotty/commit/31cbd4744b9375443a0770a8b8a9d16de694c6bb#diff-ed248bb93940ea4f38e6da698051f882e81df6f33fea91a046d1d4f6af506296R2066
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
---
22
title: SIP-67 - Improve strictEquality feature for better compatibility with existing
33
code bases
4-
status: under-review
4+
status: waiting-for-implementation
55
pull-request-number: 97
6-
stage: design
6+
stage: implementation
77

88
---

0 commit comments

Comments
 (0)