Skip to content

Fix Tuple.toList return type #12772

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix Tuple.toList return type #12772

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a964745
Fix Tuple.toList return type
nicolasstucki Jun 9, 2021
a2391b9
Hide old Tuple.toList and implement as extension
nicolasstucki Jul 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/test/dotc/pos-test-pickling.blacklist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ i5720.scala
# Tuples
toexproftuple.scala
i7580.scala
i12721.scala

# Nullability
nullable.scala
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotc/run-test-pickling.blacklist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ tuple-zip.scala
tuples1.scala
tuples1a.scala
tuples1b.scala
toList.scala
typeCheckErrors.scala
typeclass-derivation-doc-example.scala
typeclass-derivation1.scala
Expand Down
18 changes: 15 additions & 3 deletions library/src/scala/Tuple.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@ sealed trait Tuple extends Product {
inline def toArray: Array[Object] =
runtime.Tuples.toArray(this)

// NOTE: Replaced by `toList` extension method in the `Tuple` object.
// Kept this version of the method as `private[scala]` to be
// able to unpickle older TASTy files.
// TODO: When we can break TASTy compat, replace this method with one that has the following signature
// `inline def toList[This >: this.type <: Tuple]: List[Union[This]]`
// and remove the extension method in the `Tuple` object.
/** Create a copy this tuple as a List */
inline def toList: List[Union[this.type]] =
this.productIterator.toList
.asInstanceOf[List[Union[this.type]]]
private[scala] inline def toList: List[Union[this.type]] =
this.productIterator.toList.asInstanceOf[List[Union[this.type]]]

/** Create a copy this tuple as an IArray */
inline def toIArray: IArray[Object] =
Expand Down Expand Up @@ -232,6 +237,13 @@ object Tuple {

def fromProductTyped[P <: Product](p: P)(using m: scala.deriving.Mirror.ProductOf[P]): m.MirroredElemTypes =
runtime.Tuples.fromProduct(p).asInstanceOf[m.MirroredElemTypes]

// TODO: When we can break TASTy compat, move this method to `Tuple` class and use the following signature
// `inline def toList[This >: this.type <: Tuple]: List[Union[This]]`
/** Create a copy this tuple as a List */
extension [This <: Tuple](inline tuple: This)
inline def toList: List[Union[This]] =
Copy link
Member

@smarter smarter Jul 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What's the difference between this and List[Union[tuple.type]] ?

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 would fail with

3 |  (??? : Tuple).toList.map(bar)
  |  ^^^^^^^^^^^^^^^^^^^^
  |  Tuple is not a valid singleton type, since it is not an immutable path
  | This location contains code that was inlined from Tuple.scala:246

Also, the source of the original bug was caused by using the this.type instead of the type parameter.

Copy link
Member

@smarter smarter Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Tuple is not a valid singleton type, since it is not an immutable path

I find it weird that using a singleton type in the method result type would produce this error at use-site, is this a bug in the interaction between match types and singleton types? /cc @OlivierBlanvillain

Copy link
Contributor Author

@nicolasstucki nicolasstucki Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I believe it is because the prefix is not a singleton type and hence we have no way to refer to tuple.type.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like it's widening too early? To me Tuple isn't the type that should be singleton, it's the value (??? : Tuple) that should be. Put in two lines:

val foo = ??? : Tuple
foo.toList.map(bar)

I expect tuple.type=foo.type not Tuple.

Copy link
Member

Choose a reason for hiding this comment

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

So what I'm trying to say is I think this PR is working around a problem.

Copy link
Member

Choose a reason for hiding this comment

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

the prefix is not a singleton type and hence we have no way to refer to tuple.type.

That's not a problem normally: if the prefix is not a singleton type and we need it to be, then we skolemize it, but I haven't thought about how that logic is supposed to interact with match types: https://github.com/lampepfl/dotty/blob/e59c5ea77e4a498cd3a196f6eb1fc6a6b4dab05e/compiler/src/dotty/tools/dotc/core/TypeOps.scala#L36-L43

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then we are missing some skolemization for this case

tuple.productIterator.toList.asInstanceOf[List[Union[This]]]
}

/** A tuple of 0 elements */
Expand Down
7 changes: 7 additions & 0 deletions tests/pos/i12721.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
def bar(t: Any): Int = 1
def foo(t: AnyRef): Unit =
(??? : Tuple).toList.map(bar)
(??? : EmptyTuple).toList.map(bar)
(??? : NonEmptyTuple).toList.map(bar)
(??? : *:[?, ?]).toList.map(bar)
(??? : (Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int, Int)).toList.map(bar)