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

Allow Tuple.Head and Tuple.Tail to work with EmptyTuple #17189

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

Decel
Copy link
Contributor

@Decel Decel commented Mar 30, 2023

Fixes #17186.

I think this is correct behaviour since we already allow things like

val a = List(1).tail
val b = Tuple1(1).tail

@Decel Decel marked this pull request as draft March 30, 2023 23:36
@dwijnand
Copy link
Member

head and tail are partial functions, so I think it makes more sense as they stand, to have them as "partial match types". I'd say this change is akin to making Nil.tail == Nil, which I think is surprising behaviour.

@Decel
Copy link
Contributor Author

Decel commented Mar 31, 2023

head and tail are partial functions, so I think it makes more sense as they stand, to have them as "partial match types". I'd say this change is akin to making Nil.tail == Nil, which I think is surprising behaviour.

I definitely agree, but that seems like the only way to make it work (and I think it's important to make it work since we want type-level programming with tuples), without changing the logic of match types. And I think trying to change it to fit the logic here is a very bad idea, -- There are, basically, no cases, where we should allow type parameters to violate type bounds, except here.

But I'd offer the following compromise, -- we shouldn't return EmptyTuple in case of EmptyTuple.tail, but we should allow Tuple.Tail to take EmptyTuple. It would solve the issue and preserve errors.

@Decel Decel marked this pull request as ready for review March 31, 2023 11:40
@dwijnand
Copy link
Member

OK, makes sense. So we're making it more partial, rather than less - which is fine. And it allows for irreducible Tuple.Tail/Head types to nest, without running into type parameter bound checking issues.

Now the problem is when can we change this, in terms of tasty compatibility? I assume next minor version.

@dwijnand dwijnand added the needs-minor-release This PR cannot be merged until the next minor release label Mar 31, 2023
@sjrd
Copy link
Member

sjrd commented Mar 31, 2023

Now the problem is when can we change this, in terms of tasty compatibility? I assume next minor version.

Yes that's a minor version change. It is backward compatible, but not forward compatible.

@dwijnand dwijnand added this to the 3.4.0 milestone Mar 31, 2023
@dwijnand dwijnand self-assigned this Apr 3, 2023
Copy link
Contributor

@nicolasstucki nicolasstucki left a comment

Choose a reason for hiding this comment

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

I also rebased to solve some conflicts with the blacklist.

@dwijnand
Copy link
Member

dwijnand commented Dec 5, 2023

Your rebase, somehow, threw away my comments in 4983ebb

@nicolasstucki nicolasstucki merged commit d09bfda into scala:main Dec 7, 2023
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tuple.Head being unable to work with NonEmptyTuple in match types.
4 participants