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

Drop reference to implicit conversion from named to regular tuples #22015

Closed
wants to merge 1 commit into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Nov 23, 2024

The NamedTuple spec mentioned that .toTuple was inserted implicitly to convert from a named to a regular tuple. But this was not implemented. The sentiment is that this presents an opportunity to get more experience with the feature before deciding whether the conversion is actually beneficial. For now, one can implement the conversion by hand if desired, like this:

given [N <: Tuple, V <: Tuple] => Conversion[NamedTuple.NamedTuple[N, V], V] = _.toTuple

Question: Should we put that conversion somewhere in the stdlib so that one can import it?

The NamedTuple spec mentioned that `.toTuple` was inserted implicitly to convert
from a named to a regular tuple. But this was not implemented. The sentiment is
that this presents an opportunity to get more experience with the feature before
deciding whether the conversion is actually beneficial. For now, one can implement
the conversion by hand if desired, like this:
```scala
given [N <: Tuple, V <: Tuple] => Conversion[NamedTuple.NamedTuple[N, V], V] = _.toTuple
```
@ritschwumm
Copy link

since i haven't used named tuples yet, i can only give a gut feeling:

if someone calls toTuple manually it's much easier for me to figure out what's happening when i read her code, so i'd advise against using such a given.

i guess if at some time in the future there is enough evidence the convenience trumps "explicit is better than implicit", it can still be added without breaking existing code.

Copy link
Contributor

@EugeneFlesselle EugeneFlesselle left a comment

Choose a reason for hiding this comment

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

My understanding was that we do insert toTuple as described in the spec, specifically in adaptToSubType:

if tree.tpe.widen.isNamedTupleType && pt.derivesFrom(defn.TupleClass) then
readapt(typed(untpd.Select(untpd.TypedSplice(tree), nme.toTuple)))

Are there cases where we fail to insert toTuple as expected?

@odersky
Copy link
Contributor Author

odersky commented Nov 26, 2024

Are there cases where we fail to insert toTuple as expected?

There are. See #22028. But since we already have a partial implementation which is postulated by the SIP, it's better to fix the issue as is done in #22028 than backpedalling and removing the functionality. So I am closing this.

@odersky odersky closed this Nov 26, 2024
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