Add `reverse` method to `NonEmptyTuple` #13752

danicheg · 2021-10-15T13:16:46Z

In Scala 2, there is a method swap on Tuple2. In Scala 3, we could have it generalized version for any non-empty tuple as a whole.

bishabosha · 2021-10-15T13:39:57Z

library/src/scala/Tuple.scala

@@ -185,6 +185,14 @@ object Tuple {
   */
  type IsMappedBy[F[_]] = [X <: Tuple] =>> X =:= Map[InverseMap[X, F], F]

+  /** Type of the reversed tuple */
+  @experimental
+  type Reverse[X <: Tuple] <: Tuple = X match {


This should be more efficient

Suggested change

type Reverse[X <: Tuple] <: Tuple = X match {

type Reverse[X <: Tuple] = ReverseImpl[EmptyTuple, X]

@experimental

private type ReverseImpl[Acc <: Tuple, X <: Tuple] <: Tuple = X match {

case x *: xs => ReverseImpl[x *: Acc, xs]

case EmptyTuple => Acc

}

If I'm not wrong this will lead to the compilation error non-private type Reverse in object Tuple refers to private type ReverseImpl.

yes you are right, thank you, in this case maybe the implementation can be moved to a public object to avoid namespace pollution :/ or rename it ReverseOnto

Speaking for myself, I'm ok with adding that extra type, but I don't think that type could be reusable anywhere else. So probably it's good to hear the opinions of other Tuple contributors. What do you think, @nicolasstucki @anatoliykmetyuk?

An alternative that seems to work is

type Reverse[X <: Tuple] = Fold[X, EmptyTuple, [A <: Tuple, Y] =>> Y *: A]

But it would be better if it would be

type Reverse[X <: Tuple] <: Tuple = Fold[X, EmptyTuple, [A <: Tuple, Y] =>> Y *: A]

Opened #13813 to track this issue

Hm, I've tried a variant with Fold and got (tried to compile after clean):

[error] 209 | type Reverse[X <: Tuple] = Fold[X, EmptyTuple, [A <: Tuple, Y] =>> Y *: A] [error] | ^ [error] |Type argument [A <: Tuple, Y] =>> Y *: A does not conform to upper bound [_, _] =>> Any

nicolasstucki · 2021-10-25T11:12:28Z

library/src/scala/Tuple.scala

+   * consisting all its elements.
+   */
+  @experimental
+  inline def reverse[This >: this.type <: NonEmptyTuple]: Reverse[This] =


Should work for any Tuple

nicolasstucki · 2021-10-25T11:23:36Z

library/src/scala/Tuple.scala

@@ -185,6 +185,14 @@ object Tuple {
   */
  type IsMappedBy[F[_]] = [X <: Tuple] =>> X =:= Map[InverseMap[X, F], F]

+  /** Type of the reversed tuple */
+  @experimental
+  type Reverse[X <: Tuple] <: Tuple = X match {


An alternative that seems to work is

type Reverse[X <: Tuple] = Fold[X, EmptyTuple, [A <: Tuple, Y] =>> Y *: A]

But it would be better if it would be

type Reverse[X <: Tuple] <: Tuple = Fold[X, EmptyTuple, [A <: Tuple, Y] =>> Y *: A]

Opened #13813 to track this issue

Decel

I think if we apply the proposed changes, it's a good solution. I explained in #13813 why we can't do another approach with Fold.

Decel · 2023-04-06T14:18:52Z

library/src/scala/Tuple.scala

+   * consisting all its elements.
+   */
+  @experimental
+  inline def reverse[This >: this.type <: NonEmptyTuple]: Reverse[This] =


Suggested change

inline def reverse[This >: this.type <: NonEmptyTuple]: Reverse[This] =

inline def reverse[This >: this.type <: Tuple]: Reverse[This] =

Decel · 2023-04-06T14:22:23Z

project/MiMaFilters.scala

@@ -9,5 +9,6 @@ object MiMaFilters {
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.runtime.Tuples.append"),
    ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.quoted.Quotes#reflectModule#TypeReprMethods.substituteTypes"),
    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.quoted.Quotes#reflectModule#TypeReprMethods.substituteTypes"),
+    ProblemFilters.exclude[DirectMissingMethodProblem]("scala.runtime.Tuples.reverse"),


Suggested change

ProblemFilters.exclude[DirectMissingMethodProblem]("scala.runtime.Tuples.reverse"),

danicheg · 2023-04-06T14:47:56Z

If dotty's team is interested in taking this, I'm happy to move on with any changes 👍

ckipp01 · 2023-05-10T08:06:03Z

If dotty's team is interested in taking this, I'm happy to move on with any changes 👍

@bishabosha since you were commenting on this I'll ping you. Is this something worth pushing forward? it looks like @Decel is willing if it is.

bishabosha · 2023-05-11T10:47:44Z

I think its worth having, @Decel would you like to try a second attempt?

Decel · 2023-05-11T11:40:28Z

I think its worth having, @Decel would you like to try a second attempt?

I think we must finish up here and reverse should be implemented, but we haven't actually decided on the solution:

Keep the match type slow, but clean.
Add a helper match type with an upper bound. Works, but we can't keep the helper private.
Try to allow Wildcards in upper bounds. I don't think we can have that, although I'd be glad to be convinced otherwise.
@dwijnand suggestion: Change Fold definition to

type Fold[Tup <: Tuple, R, Z <: R, F[_, _ <: Tuple] <: R] <: R = Tup match
  case EmptyTuple => Z
  case h *: t => F[h, Fold[t, R, Z, F]]

I think 2 is the best option available. But we need to come up with a design for the helper.

bishabosha · 2023-05-11T11:45:35Z

I think 2 is the best option available. But we need to come up with a design for the helper.

object Tuple:

  @experimental
  object Helpers:

    @experimental
    type ReverseImpl[Acc <: Tuple, X <: Tuple] <: Tuple = X match
      case x *: xs    => ReverseImpl[x *: Acc, xs]
      case EmptyTuple => Acc

Decel · 2023-05-11T12:59:02Z

object Tuple:

  @experimental
  object Helpers:

    @experimental
    type ReverseImpl[Acc <: Tuple, X <: Tuple] <: Tuple = X match
      case x *: xs    => ReverseImpl[x *: Acc, xs]
      case EmptyTuple => Acc

Looks good. Would you like to commit it to the PR? The only other change I think should be is Tuple as an upper bound of the scrutinee, instead of NonEmptyTuple

danicheg · 2023-05-14T14:02:04Z

fwiw: this definition of Reverse type doesn't work

type Reverse[X <: Tuple] <: Tuple = Helpers.ReverseImpl[EmptyTuple, X]
// ^ cannot combine bound and alias

Am I missing something?

Decel · 2023-05-14T14:38:21Z

fwiw: this definition of Reverse type doesn't work
type Reverse[X <: Tuple] <: Tuple = Helpers.ReverseImpl[EmptyTuple, X]
// ^ cannot combine bound and alias
Am I missing something?

We can't write type bounds on aliases:

type Reverse[X <: Tuple] = Helpers.ReverseImpl[EmptyTuple, X]

Since ReverseImpl upper bound is Tuple we will be able to infer the same for Reverse

danicheg · 2023-05-14T17:13:53Z

I've addressed the review comments, thanks for your tips and suggestions. CI is green, so this could be reviewed once again.

bishabosha · 2023-05-15T13:35:27Z

library/src/scala/runtime/Tuples.scala

@@ -447,6 +449,79 @@ object Tuples {
    }
  }

+  // Reverse for TupleXXL
+  private def xxlReverse(xxl: TupleXXL): Tuple = {
+    if (xxl.productArity == 22) {


this branch should be removed - TupleXXL should not be constructed unless its arity is 23+

Yeah, indeed. Good call, thanks

bishabosha · 2023-05-17T12:23:52Z

Experimental definitions shouldn't make warnings in mima so you can remove those changes to MiMaFilters

danicheg · 2023-05-17T12:56:35Z

Experimental definitions shouldn't make warnings in mima so you can remove those changes to MiMaFilters

Actually, I tried that locally before pushing it to the remote branch, and MiMa didn't like it.

bishabosha · 2023-05-17T14:03:42Z

Actually, I tried that locally before pushing it to the remote branch, and MiMa didn't like it.

I need someone else to advise here

danicheg · 2023-05-19T09:11:39Z

If we don't mention anyone from the Scala3 team, this PR will get stuck as it was earlier. We all spent quite a lot of resources to get to this point, so can you @bishabosha @ckipp01 please step into finding the right person to perform a final push? But yeah, it's not an urgent thing at all.

bishabosha · 2023-05-19T10:20:50Z

@Kordyjan what is the MiMa policy for experimental definitions?

Kordyjan · 2023-05-22T11:47:20Z

They need to be listed in the MimaFilters. Everything looks good here!

danicheg · 2023-05-30T16:14:08Z

So, 3.3.0 is published 🎉 . What does it mean for this PR? Can it be released only in 3.4.0?

bishabosha · 2023-05-30T18:02:41Z

it can be released as experimental in a patch release

danicheg · 2023-06-03T11:49:29Z

Good to know! So, I will do the final ping and stop bothering people with this.

bishabosha · 2023-06-20T10:20:00Z

library/src/scala/Tuple.scala

+   * consisting all its elements.
+   */
+  @experimental
+  inline def reverse[This >: this.type <: Tuple]: Reverse[This] =


This method should also be moved to the Tuple trait, currently it is in NonEmptyTuple

bishabosha · 2023-06-20T10:21:18Z

tests/run-custom-args/tasty-inspector/stdlibExperimentalDefinitions.scala

+  "scala.quoted.Quotes.reflectModule.defnModule.ErasedFunctionClass",
+
+  // New feature: reverse method on Tuple
+  "scala.NonEmptyTuple.reverse",


this will be affected when you move reverse to Tuple

tests/run-deep-subtype/Tuple-reverse.scala

bishabosha

reverse should be a member of the Tuple trait to match its implementation

danicheg · 2023-07-02T13:29:25Z

@bishabosha here we go (again)...

bishabosha

Ok looks good, sorry could we also have a test that checks e.g. that (1, true, "hello"). reverse is typed as (String, Boolean, Int) and try to do a git rebase instead of pull - otherwise looks good

danicheg · 2023-07-02T16:20:07Z

@bishabosha

could we also have a test that checks e.g. that (1, true, "hello"). reverse is typed as (String, Boolean, Int)

done

try to do a git rebase instead of pull

could you accomplish a squash of commits when merging?

bishabosha reviewed Oct 15, 2021

View reviewed changes

danicheg added 3 commits October 22, 2021 20:47

Add reverse method to NonEmptyTuple

d8dfef6

Merge branch 'master' into tuple-reverse

ebd5f2b

Update MiMa filters

ab1a19f

danicheg force-pushed the tuple-reverse branch from fedba9c to ab1a19f Compare October 22, 2021 17:51

nicolasstucki self-assigned this Oct 23, 2021

nicolasstucki self-requested a review October 23, 2021 16:22

nicolasstucki reviewed Oct 25, 2021

View reviewed changes

Decel suggested changes Apr 6, 2023

View reviewed changes

ckipp01 requested a review from bishabosha May 10, 2023 08:06

ckipp01 assigned bishabosha and unassigned nicolasstucki May 10, 2023

Merge branch main

ebf8d8c

Address review comments

a1d8b66

bishabosha reviewed May 15, 2023

View reviewed changes

Remove wrong branch in Tuples#xxlReverse

5006b2f

bishabosha reviewed Jun 20, 2023

View reviewed changes

tests/run-deep-subtype/Tuple-reverse.scala Show resolved Hide resolved

bishabosha requested changes Jun 20, 2023

View reviewed changes

danicheg added 2 commits July 2, 2023 15:43

Merge branch 'main' into tuple-reverse

cb2f91f

Move reverse method to Tuple

6aef25a

bishabosha requested changes Jul 2, 2023

View reviewed changes

Add one more test

ae14dab

bishabosha approved these changes Jul 3, 2023

View reviewed changes

bishabosha merged commit b72dfb5 into scala:main Jul 3, 2023

danicheg deleted the tuple-reverse branch July 3, 2023 14:05

Kordyjan added this to the 3.4.0 milestone Aug 2, 2023

	inline def reverse[This >: this.type <: NonEmptyTuple]: Reverse[This] =
	inline def reverse[This >: this.type <: Tuple]: Reverse[This] =

Add reverse method to NonEmptyTuple #13752

Add reverse method to NonEmptyTuple #13752

Conversation

danicheg commented Oct 15, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha Oct 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Decel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danicheg commented Apr 6, 2023

ckipp01 commented May 10, 2023

bishabosha commented May 11, 2023

Decel commented May 11, 2023

bishabosha commented May 11, 2023

Decel commented May 11, 2023 • edited Loading

danicheg commented May 14, 2023

Decel commented May 14, 2023

danicheg commented May 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha commented May 17, 2023 • edited Loading

danicheg commented May 17, 2023

bishabosha commented May 17, 2023 • edited Loading

danicheg commented May 19, 2023

bishabosha commented May 19, 2023

Kordyjan commented May 22, 2023

danicheg commented May 30, 2023

bishabosha commented May 30, 2023

danicheg commented Jun 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bishabosha left a comment

Choose a reason for hiding this comment

danicheg commented Jul 2, 2023

bishabosha left a comment

Choose a reason for hiding this comment

danicheg commented Jul 2, 2023

Add `reverse` method to `NonEmptyTuple` #13752

Add `reverse` method to `NonEmptyTuple` #13752

bishabosha Oct 15, 2021 •

edited

Loading

Decel commented May 11, 2023 •

edited

Loading

bishabosha commented May 17, 2023 •

edited

Loading

bishabosha commented May 17, 2023 •

edited

Loading