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

Handle more corner cases in etaReduce #14628

Merged
merged 1 commit into from
Mar 7, 2022
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Mar 6, 2022

Generalize eta reduce so that it also handles eta expansions that are subject to
adaptation or specialization.

Fixes #14623

Generalize eta reduce so that it also handles eta expansions that are subject to
adaptation or specialization.

Fixes scala#14623
@odersky
Copy link
Contributor Author

odersky commented Mar 6, 2022

@smarter One tricky aspect is that eta reduction has to undo closure adaptation byErasure#adaptClosure. When looking at this code, it seemed that there should be a simpler way. We have a closure and create another closure that calls the original closure and adapts its parameters or result. Why not modify the existing closure instead?

@odersky
Copy link
Contributor Author

odersky commented Mar 6, 2022

Btw doing eta reduction before erasure would not be a solution here since the eta expansion in #14623 does change the result type (from normal function to context function). It's only after erasure that we can assume the two are the same.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

We have a closure and create another closure that calls the original closure and adapts its parameters or result. Why not modify the existing closure instead?

Hmm, this might be possible by special-casing the erasure of the parameter symbols of closure defs in TypeErasure#eraseInfo, though it's not clear to me if that would be a net win in terms of complexity or performance, in particular it seems like we could end up with many more boxing/unboxing steps.

@smarter smarter merged commit fb618ad into scala:main Mar 7, 2022
@smarter smarter deleted the fix-14623 branch March 7, 2022 13:15
@Kordyjan Kordyjan added this to the 3.1.3 milestone Aug 1, 2023
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.

Passing a Function0 by-name generates an (unnecessary?) wrapper
3 participants