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

Add tighter bounds on AnyNamedTuple #22037

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

soronpo
Copy link
Contributor

@soronpo soronpo commented Nov 27, 2024

AnyNamedTuple should have a Tuple lower bound and a Product upper-bound

@soronpo soronpo requested a review from odersky November 27, 2024 05:15
@@ -11,7 +11,7 @@ object NamedTuple:
opaque type NamedTuple[N <: Tuple, +V <: Tuple] >: V <: AnyNamedTuple = V

/** A type which is a supertype of all named tuples */
opaque type AnyNamedTuple = Any
opaque type AnyNamedTuple >: Tuple <: Product = Product
Copy link
Member

Choose a reason for hiding this comment

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

We cannot change the right hand side anymore. It breaks binary and TASTy compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

If we assume 3.6.0 and 3.6.1 do no exist, then we're still in the RC period where modifications are allowed. We're currently at 3.6.2-RC2, and it's probably the last moment before signatures would be written in stone forever.

Copy link
Member

Choose a reason for hiding this comment

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

Regardless, if we are making such a change at this point, it means named tuples are not ready for prime time.

So either we back out and make them experimental again, or we don't touch them except for bug fixes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Which makes me realize that we need another strategy to roll out new features. Hardly anybody uses them when they are experimental. And when we stabilize small nits like this one inevitably pop up. Which means we have only three choices:

  • we accept that it's imperfect and freeze anyway
  • we roll back, which means that we cannot get anything substantial into the language anymore, ever
  • we accept some breakage to fix nits such as this one, for a limited period.

I am not sure what to do here. Java has previews, would that help?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should discuss it later today, but some thoughts on the topic:

  • people are testing it in an experimental RC Scala version (though named as 3.6.1 due to known reasons), so I wouldn't say that hardly anyone tests experimental stuff
  • the period for experimenting with it was rather short though (less than 3 months) and making it stable felt rushed just before 3.6.0 cutoff. I think a lot of people including us were surprised by this.
  • there is a lot of discussion about it and uncertainty, so it would make sense to make it experimental again as a nod to the community. It's always better to be careful than not and better to not to risk antagonizing the community if not needed.

I think we should:

  • mark it back as experimental
  • make as much publicity about the feature as possible to encourage people to try it out. We usually promote released features, we should also promote heavily anything that experimental that we merge.
  • make sure that the details are discussed and we announce making it stable in the next release (another section in release notes like 'becoming stable'). If no one has anything serious to add we go ahead.

I fully understand your feelings here, things might be slow moving, but is there really a need to rush it? I think the best result we can get is having people actively discuss any new features, which indeed happened with named tuples and we should make it happen every time new experimental is dropped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be mentioned here that the SIP committee can approve a feature to be accepted, and it's the compiler's team decision when to actually merge it as non-experimental.

@odersky
Copy link
Contributor

odersky commented Nov 27, 2024

How important is the change? AnyNamedTuple was intended as a marker "only named tuples can go here". It was not supposed to have any functionality beyond that.

@Gedochao Gedochao added the stat:needs decision Some aspects of this issue need a decision from the maintainance team. label Nov 27, 2024
@Gedochao
Copy link
Contributor

Let's discuss this on the next Scala Core

@soronpo
Copy link
Contributor Author

soronpo commented Nov 27, 2024

How important is the change? AnyNamedTuple was intended as a marker "only named tuples can go here". It was not supposed to have any functionality beyond that.

I don't know how important it is, but it's a surprising element that NamedTuple is not a Product. Why shouldn't a method that accepts a Product argument not be able to use NamedTuple? Why shouldn't we be able to use Product internal methods like productIterator on NamedTuple? Regarding the Tuple lower bound, it could go with/without I guess.

@Gedochao
Copy link
Contributor

It has been decided that named tuples will remain experimental in 3.6.x, so it should still be valid to make significant changes to the implementation (like the changes suggested in this PR).

Before we announce it as stable, we will likely make it a preview feature, as tracked in:

@Gedochao Gedochao removed the stat:needs decision Some aspects of this issue need a decision from the maintainance team. label Nov 27, 2024
@soronpo
Copy link
Contributor Author

soronpo commented Nov 27, 2024

@Gedochao why not do the same (preview) for the new given syntax?

@hamzaremmal
Copy link
Member

@Gedochao why not do the same (preview) for the new given syntax?

What does preview even mean ?

@Gedochao
Copy link
Contributor

Gedochao commented Nov 28, 2024

What does preview even mean ?

@hamzaremmal Think of it as a stage 2, promoted experimental feature.
A middle step before announcing something as stable, promoting it to be tried out more, but still subject to changes.
The process will likely be announced along with Scala 3.7.x series.

@Gedochao why not do the same (preview) for the new given syntax?

@soronpo Feature previews were freshly decided on just yesterday, with named tuples being the only ones we know already will be shipped with it.
We haven't considered other features for it yet (and we don't have an implementation in place yet, either).

EDIT: also, linking these for context:

@Gedochao
Copy link
Contributor

@Gedochao why not do the same (preview) for the new given syntax?

FYI after some discussions in the background, it seems we wouldn't do that. The new given syntax will likely ship as stable in 3.6.2.

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.

7 participants