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 #18246: correctly compute capture sets in TypeComparer.glb #18254

Merged
merged 3 commits into from
Jul 23, 2023

Conversation

Linyxus
Copy link
Contributor

@Linyxus Linyxus commented Jul 20, 2023

Intuitively, glb(S1^C1, S2^C2) should capture C1 ⊓ C2, where denotes the glb of capture sets. When computing glb(tp1, tp2), if tp1 matches CapturingType(parent1, refs1), it captures C0 ++ refs1 where C0 is the nested capture sets inside parent1. Denote the capture set of tp2 with C2, the result should capture (C0 ++ refs1) ⊓ C2. Presumably, distributes over the union (++); therefore (C0 ++ refs1) ⊓ C2 equals C0 ⊓ C2 ++ refs1 ⊓ C2.

Previously, if C2 is a subcapture of refs1, it is simply dropped. However, based on the above reasoning, we have refs1 ⊓ C2 equals C2; therefore C0 ⊓ C2 ++ refs1 ⊓ C2 equals C0 ⊓ C2 ++ C2. C2 should not be dropped, but rather kept. This PR fixes the logic to behave correctly.

Fixes #18246.

@Linyxus Linyxus requested a review from odersky July 20, 2023 06:09
@Linyxus Linyxus changed the title Fix #18246 Fix #18246: correct the capturing type case in TypeComparer.glb Jul 20, 2023
@Linyxus Linyxus changed the title Fix #18246: correct the capturing type case in TypeComparer.glb Fix #18246: correctly compute capture sets in TypeComparer.glb Jul 20, 2023
@odersky
Copy link
Contributor

odersky commented Jul 23, 2023

Denote the capture set of tp2 with C2, the result should capture (C0 ++ refs1) ⊔ C2.

I guess that should be \squarecap not \squarecup.

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

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

Nice catch!

&& tp1.isBoxedCapturing == tp2.isBoxedCapturing
then
parent1 & tp2
if refs2.isAlwaysEmpty then parent1 & tp2
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should roll the isAlwaysEmpty optimization into capturing?

@odersky odersky assigned Linyxus and unassigned odersky Jul 23, 2023
@Linyxus Linyxus enabled auto-merge July 23, 2023 16:57
@Linyxus Linyxus merged commit 19dbbf5 into scala:main Jul 23, 2023
15 checks passed
@Linyxus Linyxus deleted the cc-glb-fix-1 branch July 23, 2023 18:13
@Kordyjan Kordyjan added this to the 3.4.0 milestone Aug 1, 2023
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.

CC unsoundness: TypeComparer.glb strips capture set
3 participants