Fix Tuple.toList return type #12772
Fix Tuple.toList return type #12772
Conversation
|
This change is not forward nor backward TASTy compatible. It can break existing |
nicolasstucki
force-pushed
the
fix-#12721
branch
2 times, most recently
from
5d158f8 to
cf2b94e
Compare
|
@sjrd in which version would we be able to fix this without breaking compatibility? We could also try to special-case it in the compiler so that if we have a reference to the old one we adapt it before expanding it. |
As currently proposed, not before Scala 4. (or 3.T)
That is a possibility, but it would have to enter the TASTy spec, because if any other tool unpickles that, it will have to perform the same fix. This is going to be super tricky. |
|
What about making the old |
|
I guess we can try that. |
|
If that doesn't work, give |
Exactly why I was worried 3.0 was released so soon. |
nicolasstucki
force-pushed
the
fix-#12721
branch
from
cf2b94e to
0cdc242
Compare
That did not work as we would be trying to override a final method. If I change the signature, at call site, it complains due to an ambiguity between the two |
nicolasstucki
force-pushed
the
fix-#12721
branch
2 times, most recently
from
ea1f66a to
670b360
Compare
| // `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]] = |
There was a problem hiding this comment.
What's the difference between this and List[Union[tuple.type]] ?
There was a problem hiding this comment.
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:246Also, the source of the original bug was caused by using the this.type instead of the type parameter.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I believe it is because the prefix is not a singleton type and hence we have no way to refer to tuple.type.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
So what I'm trying to say is I think this PR is working around a problem.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Then we are missing some skolemization for this case
This method failed for any subtype of `Tuple`. It only worked for `Tuple` and `TupleN` as these define their own `toList`. Fixes scala#12721
This should in theory be backwards TASTy compatible. Fixes scala#12721
nicolasstucki
force-pushed
the
fix-#12721
branch
from
670b360 to
a2391b9
Compare
This method failed for any subtype of
Tuple.It only worked for
TupleandTupleNas these define their owntoList.Fixes #12721