Skip to content
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

Fix isEffectivelySingleton #20486

Merged
merged 1 commit into from
May 28, 2024
Merged

Fix isEffectivelySingleton #20486

merged 1 commit into from
May 28, 2024

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 28, 2024

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

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
Copy link
Member

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

Copy link
Contributor Author

@odersky odersky May 28, 2024

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

@odersky
Copy link
Contributor Author

odersky commented May 28, 2024

I propose we merge without further refinements, since it plugs a recent soundness hole.

@noti0na1 noti0na1 self-requested a review May 28, 2024 18:51
@noti0na1 noti0na1 merged commit a21d2af into scala:main May 28, 2024
19 checks passed
@noti0na1 noti0na1 deleted the fix-20474-b branch May 28, 2024 18:53
@Kordyjan Kordyjan added this to the 3.5.1 milestone Jul 3, 2024
WojciechMazur added a commit that referenced this pull request Jul 9, 2024
Backports #20486 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants