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

Improve Tuple and NamedTuple #19175

Open
odersky opened this issue Dec 3, 2023 · 2 comments
Open

Improve Tuple and NamedTuple #19175

odersky opened this issue Dec 3, 2023 · 2 comments

Comments

@odersky
Copy link
Contributor

odersky commented Dec 3, 2023

Tuple is a very important core class of the Scala 3 standard library. But its organization was a jumble of different styles, with major omissions in functionality and a lack of consistency.

I made an effort in #19172 to improve Tuple.scala and in #19174 to align NamedTuple.scala and Tuple.scala. The work is not finished yet, and it would be great if someone could continue it. In particular:

  • Make sure that every type we define has a corresponding method implementing that type, or if that does not make sense, explain in the doc comment of the type why there is no method.
  • Make sure doc comments are consistent and helpful.
  • Make sure we have tests for all operations.
  • Have we missed important operations?
  • Make sure Tuple and NamedTuple are aligned.

One area where NamedTuple and Tuple are not aligned is that there is no concept of "non-empty tuple" for named tuples. I believe adding that to Tuple was a big mistake. If you look at the definition of Tuple you'll find seemingly arbitrary and non-sensical definitions that sometimes use Tuple and sometimes use EmptyTuple. For instance

  type Head[X <: NonEmptyTuple]
  type Last[X <: Tuple]

Why is Head only defined for NonEmptyTuple whereas Last is defined for Tuple? That makes no sense!

The problem comes up because "being non empty" is a structural property of a tuple and we want to express it with a nominal type. That's usually a bad idea since nominal types are just not expressive enough. So the end result is that we shoot ourselves in the foot, and that for zero gains in safety. In fact, Last above is just as safe as Head. When applied to the empty tuple, neither of them reduces to a type.

@nicolasstucki
Copy link
Contributor

We have a similar issue with NonEmptyTuple.apply. It should be defined in Tuple.

EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 29, 2024
to only require it be defined on the elements of the tuple.

This is one of the ongoing proposed tuple improvements, addressing scala#19175.

As carefully pointed out by @sjrd, this _is_ a potential breaking change.
See tests/neg/tuple-filter-compat.scala for an example.

This is not an unprecedented change however,
the analogous improvements were made to `Tuple.{Map, FlatMap}` in 28a695e.
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 29, 2024
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 29, 2024
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 29, 2024
@EugeneFlesselle
Copy link
Contributor

EugeneFlesselle commented Jul 29, 2024

There have a few been attempts at improving the generic tuples, which have proven hard to get merged.
I am splitting the proposed changes in different PRs, the list of which I will keep updated below:

Which of those are accepted should be decided and discussed by the core team.

EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 29, 2024
Addressing scala#19175

The motivation for this has already been established, among other things:
- the corresponding type level operations already use `Tuple` as upper bound;
- the corresponding `NamedTuple` operations also do not make a distinction;
- these operations are no more unsafe than other operations already available
  on `Tuple`, such as `drop`

Note this should _not_ be a problem for binary compatibility,
as both `Tuple` and `NonEmptyTuple` are erased to `Product`s
(see `defn.specialErasure`).
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 30, 2024
If we reach the second case of `Zip[T1 <: Tuple, T2 <: Tuple]`,
then we know `(T1, T2)` is disjoint from `(NonEmptyTuple, NonEmptyTuple)`,
from which we can conclude at least one of the two is an `EmptyTuple`.

Addressing scala#19175
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 31, 2024
Addressing scala#19175

The motivation for this has already been established, among other things:
- the corresponding type level operations already use `Tuple` as upper bound;
- the corresponding `NamedTuple` operations also do not make a distinction;
- these operations are no more unsafe than other operations already available
  on `Tuple`, such as `drop`

Note this should _not_ be a problem for binary compatibility,
as both `Tuple` and `NonEmptyTuple` are erased to `Product`s
(see `defn.specialErasure`).
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 6, 2024
Addressing scala#19175

The motivation for this has already been established, among other things:
- the corresponding type level operations already use `Tuple` as upper bound;
- the corresponding `NamedTuple` operations also do not make a distinction;
- these operations are no more unsafe than other operations already available
  on `Tuple`, such as `drop`

Note this should _not_ be a problem for binary compatibility,
as both `Tuple` and `NonEmptyTuple` are erased to `Product`s
(see `defn.specialErasure`).
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 6, 2024
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 6, 2024
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 6, 2024
to only require it be defined on the elements of the tuple.

This is one of the ongoing proposed tuple improvements, addressing scala#19175.

As carefully pointed out by @sjrd, this _is_ a potential breaking change.
See tests/neg/tuple-filter-compat.scala for an example.

This is not an unprecedented change however,
the analogous improvements were made to `Tuple.{Map, FlatMap}` in 28a695e.
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 6, 2024
If we reach the second case of `Zip[T1 <: Tuple, T2 <: Tuple]`,
then we know `(T1, T2)` is disjoint from `(NonEmptyTuple, NonEmptyTuple)`,
from which we can conclude at least one of the two is an `EmptyTuple`.

Addressing scala#19175
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 7, 2024
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 7, 2024
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 7, 2024
Addressing scala#19175

The motivation for this has already been established, among other things:
- the corresponding type level operations already use `Tuple` as upper bound;
- the corresponding `NamedTuple` operations also do not make a distinction;
- these operations are no more unsafe than other operations already available
  on `Tuple`, such as `drop`

Note this should _not_ be a problem for binary compatibility,
as both `Tuple` and `NonEmptyTuple` are erased to `Product`s
(see `defn.specialErasure`).
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 7, 2024
to only require it be defined on the elements of the tuple.

This is one of the ongoing proposed tuple improvements, addressing scala#19175.

As carefully pointed out by @sjrd, this _is_ a potential breaking change.
See tests/neg/tuple-filter-compat.scala for an example.

This is not an unprecedented change however,
the analogous improvements were made to `Tuple.{Map, FlatMap}` in 28a695e.
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Nov 7, 2024
If we reach the second case of `Zip[T1 <: Tuple, T2 <: Tuple]`,
then we know `(T1, T2)` is disjoint from `(NonEmptyTuple, NonEmptyTuple)`,
from which we can conclude at least one of the two is an `EmptyTuple`.

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

No branches or pull requests

4 participants