Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change for SIP-62: betterFors #99

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KacperFKorban
Copy link
Member

A proposal of a new approach for implementing change 3 from SIP-62: betterFors.

TLDR:

Don't drop the trailing map when desugaring. Instead, mark the last Apply node with a TrailingForMap attachment and move the optimization to a later phase (after typing). This way, we can make sure that the resulting type of the for doesn't change.

@KacperFKorban KacperFKorban changed the title Addendum for SIP-62: betterFors Change for SIP-62: betterFors Nov 26, 2024
@KacperFKorban
Copy link
Member Author

I suppose that the manager stays the same. ping @lihaoyi

@lihaoyi
Copy link
Contributor

lihaoyi commented Nov 27, 2024

I think this looks reasonable, bit i dont know enough about dotty internals to know what rhe consequences of this would
be (if any). @sjrd do you know?

@lihaoyi
Copy link
Contributor

lihaoyi commented Dec 20, 2024

Ping @sjrd


This kind of effect loop is pretty commonly used in Scala FP programs and often ends in `yield ()`.

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.
Copy link

@He-Pin He-Pin Dec 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it then be desugared to two foreachs? as the _ will never be used anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, it should end up with just one flatMap. It will eliminate the last map call. This is also what bm4 would do in this situation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants