-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix isEffectivelySingleton #20486
Fix isEffectivelySingleton #20486
Conversation
As usual, OrTypes need to be excluded. a.type | b.type is not effectively a singleton. It seems to be an easy trap to fall into. Follow-up to scala#20474
val b1: a.type = a | ||
val b2: a.type & B = a.asInstanceOf[a.type & B] | ||
val b3: c.type & A = c | ||
val b4: a.type | c.type = c |
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.
Can you add the test below for OrType as well?
val b5: a.type | a.type = a
val d5: b5.type = a // ok
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 don't think that will work. Have you tried it? I see no strong reason to make it work.
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 works in 3.4.2: https://scastie.scala-lang.org/A6PehvQvRhW2sUPVVtHiuA
I think the atoms
is doing all these checks.
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.
Ah, yes, atoms would do it. Bit it's an extreme corner case. Not sure we want to make sure it works in the general case. Because in this case we'd also have to accept
val b6: (a.type & A) | a.type
val d6: b6.type = a // ok
and I am not sure atoms would help us here.
I propose we merge without further refinements, since it plugs a recent soundness hole. |
Backports #20486 to the LTS branch. PR submitted by the release tooling. [skip ci]
As usual, OrTypes need to be excluded. a.type | b.type is not effectively a singleton. It seems to be an easy trap to fall into.
Follow-up to #20474