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

Named tuples 3 #20346

Closed
wants to merge 68 commits into from
Closed

Named tuples 3 #20346

wants to merge 68 commits into from

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented May 6, 2024

No description provided.

odersky added 30 commits May 6, 2024 17:09
New methods: filter, indicesWhere, reverseOnto
Also, add deep matches to tests
This is a general improvement, independent of named tuples.
Use the sugared representation, not the raw NamedTuple type tree.
Subclasses of Selectable can instantiate Fields to a named tuple type
that provides possible selection names and their types on instances of the type.

See: https://contributors.scala-lang.org/t/expanding-changing-selectable-based-on-upcoming-named-tuples-feature/6395/5
I usually try to avoid explicit returns, but here they do make the
code easier to read.
The selectDynamic call could already have influenced type variables
in the expected type before we wrap it in a cast.

Need to pass in the right expected type to the typedDynamicSelect.
NamedTyple.From should be the identity for named tuple arguments
odersky and others added 22 commits May 6, 2024 17:11
as is the case for `Concat`
to only require being defined on the element types.
Similar to what we have for `type FlatMap`
This is for the same reason as we changed
`type Head[X <: NonEmptyTuple] = ...` to
`type Head[X <: Tuple] = ...`

Also, this is no more unsafe than the other operations already defined for all tuples.
`drop(1)` for example was always defined, even though `tail` wasn't.
The tuple term level definitions were not being tested before
For the two operations that we really need for NamedTuple, we
define them as "internal" helpers inside NamedTuple.

We remove or amend tests that relied on the changes to Tuple. No
tests about NamedTuple's are affected.
If we add it as is now, we will *not* be able to add the bound in
a future release. It is best to leave it completely undefined. The
typer is happy to ignore the case where it does not exist at all.
@bishabosha
Copy link
Member

Relevant failure:
Found @experimental definition in library not listed:
scala.NamedTuple$.FutureTupleOps
scala.NamedTuple$.FutureTupleOps$
scala.Tuple$.Reverse
scala.Tuple$.ReverseOnto
scala.Tuple.reverse
scala.runtime.Tuples$.reverse

If added new experimental definitions to the library, add them to the list in tests/run-tasty-inspector/stdlibExperimentalDefinitions.scala

Test only: sbt "scala3-bootstrapped/testCompilation tests/run-tasty-inspector/stdlibExperimentalDefinitions.scala"

@odersky
Copy link
Contributor

odersky commented May 7, 2024

I don't think we should backtrack on FutureTupleOps again. This is just causes friction and statis. Any improvement to Tuple is welcome, and should not be held up. If it has been thoroughly reviewed it should not be experimental. I am going agead with the minimum changes to Tuple that make NamedTuple work and these will not be experimental.

It's all very good to hedge our bets, but we have seen in the past that this means progress will be delayed indefinitely. Otherwise, why would Tuple.scala still be in the bad state it is?

@sjrd
Copy link
Member Author

sjrd commented May 7, 2024

I am going agead with the minimum changes to Tuple that make NamedTuple work and these will not be experimental.

Yes, OK. Fine by me as long as it's only minimal additions.

@odersky
Copy link
Contributor

odersky commented May 7, 2024

OK, I incorporated all changes into named-tuples-2. We should do a separate effort to clean up Tuple in the next cycle. @EugeneFlesselle did a good job in dotty-staging#41 to split into reviewable commits. Maybe we can build on that?

@sjrd
Copy link
Member Author

sjrd commented May 7, 2024

OK, I incorporated all changes into named-tuples-2.

OK, closing this one as superseded by #19174.

We should do a separate effort to clean up Tuple in the next cycle. @EugeneFlesselle did a good job in dotty-staging#41 to split into reviewable commits. Maybe we can build on that?

Yes, absolutely.

@sjrd sjrd closed this May 7, 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.

4 participants