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

Mention general conversions for untupling #14315

Merged
merged 1 commit into from
Jan 27, 2022

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Jan 20, 2022

Addresses the question at #897 (comment)

This is to address "why isn't my function value adapted?"

It shows literal xs.map(_ + _), etamatic xs.map(f), explicit xs.map(g.tupled) and also user-defined conversion.

The conversion is equivalent to the untupling adaptation, and improves on unspecialized tupled. (The tuples themselves are also currently treated as unspecialized, but that is targeted for improvement.) However, no benchmarks were run.

import scala.language.implicitConversions
inline transparent implicit def `enable untupling`(f: (Int, Int) => Int): ((Int, Int)) => Int =
p => f(p._1, p._2)
xs.map(combine)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about xs.map(Function.tupled(combine)) instead? That seems more usual than a transparent inline implicit conversion...

Copy link
Contributor Author

@som-snytt som-snytt Jan 21, 2022

Choose a reason for hiding this comment

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

This was to address "why isn't my function value adapted?" and show that the expected conversion works (and is equivalent or as efficient as expected with transparent inline). I accidentally switched the order of modifiers after seeing them printed that way in printed tree output. Ha.

Also considering the question of existing conversions as less efficient. BTW, I don't insist that this is the best clarification.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense and I agree that the inline definition should be more efficient, but I think I would still be more comfortable merging a more "straightforward" solution, it seems like it would be a better fit for parameter-untupling.md!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the description, keeping the conversion but on the down low.

@OlivierBlanvillain OlivierBlanvillain merged commit afbb5a5 into scala:master Jan 27, 2022
@OlivierBlanvillain
Copy link
Contributor

LGTM, thanks!

@som-snytt som-snytt deleted the tweak/untupling branch January 27, 2022 15:45
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.

None yet

2 participants