-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Try to refactor convertTo method for improved type handling with union types #24585
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
base: main
Are you sure you want to change the base?
Conversation
noti0na1
commented
Nov 28, 2025
| case pt: OrType => | ||
| val leftResult = convertTo(pt.tp1) | ||
| if leftResult != null then leftResult | ||
| else convertTo(pt.tp2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the part where I would say I don't agree with and why I say it is a can of worms.
What if we have val x: Byte | Short = 0, maybe this should be a Short instead of a Byte (take the largest type). This is something that needs to be decided and what approach we will take.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have a concrete example why this matters? I think if there is any ambiguity inside the expected union type, it's user's responsibility to specify a type first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could say why does it matter what is the type of the constant of the rhs.
It matters because of macros first. Second, we are thinking about how to encode Byte, Short,... literals in the language. We need to then know that literal, what would be its exact type.
Third, transparent inline def foo: Byte | Short = 0, since this is a transparent inline, we cannot just ignore the type information to see that it was a Byte | Short, we need to know what kind of 0 and we can clearly not say it was an Int by stripping the type information completely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were writing at the same time 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's user's responsibility to specify a type first.
Yes, maybe the user should not write this and in my opinion they shouldn't but Scala 3 has a specification and even if this is a very niche edge case that no one should write, it is still important to specify it and understand all the possible designs we could have chosen and chose one wisely (because once we do, there is no changing it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just had a chat with @odersky about this and he thinks that this should not be fixed at all.
Even val x: Byte | String = 0 should fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would need at least a SIP. Feature interaction is a problem here. What if there was an implicit conversion from Int to Byte | String? Then that would have been picked before but now it would be bypassed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about: it should fail if there are different ways to convert according to the union type; success if there is only one result.
val a: Byte | String | Byte = 0 // ok
val b: Byte | Int = 0 // failsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would need at least a SIP.
I completely agree on this.
Feature interaction is a problem here. What if there was an implicit conversion from Int to Byte | String? Then that would have been picked before but now it would be bypassed.
That's also something to consider if we were to pursue these improvements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about: it should fail if there are different ways to convert according to the union type; success if there is only one result
That's also another way of handling the situation. My point is (and was this morning too), this is not as simple as opening a PR with one of these solutions. There is many factors to take into account and this will always result into a change of spec (so a SIP as Martin mentioned too). The T | Null was an obvious one that needed to be fixed and we did, so pressure is off.