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

Only set AppliedType#validSuper after AppliedType#cachedSuper #20553

Merged
merged 1 commit into from
Jun 11, 2024

Conversation

EugeneFlesselle
Copy link
Contributor

since cycles are possible when computing AppliedType#superType, see tests/neg/i20546.scala for an example leading to an NPE.

We could use ctx.period == validSuper && cachedSuper == null as condition to detect cycles, but they are already handled in TypeApplications#appliedTo, with a better error message.

We can update AppliedType#validSuper only after the computation is done to fix #20546

since cycles are possible when computing `AppliedType#superType`,
see tests/neg/i20546.scala for an example leading to an NPE.

We could use `ctx.period == validSuper && cachedSuper == null` as condition to detect cycles,
but they are already handled in `TypeApplications#appliedTo`, with a better error message.

We can update `AppliedType#validSuper` only after the computation is done to fix scala#20546
@EugeneFlesselle EugeneFlesselle requested a review from noti0na1 June 11, 2024 12:46
@EugeneFlesselle EugeneFlesselle enabled auto-merge (rebase) June 11, 2024 13:10
Copy link
Member

@noti0na1 noti0na1 left a comment

Choose a reason for hiding this comment

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

LGTM

@EugeneFlesselle EugeneFlesselle merged commit e2dfea3 into scala:main Jun 11, 2024
23 checks passed
@EugeneFlesselle EugeneFlesselle deleted the issues/i20546 branch June 11, 2024 14:09
@EugeneFlesselle
Copy link
Contributor Author

EugeneFlesselle commented Jun 11, 2024

I should probably have added this as a comment in the test, I'm not particularly convinced this has to by a cyclic error, but at least we don't have a NPE and it's no longer a regression.

@Kordyjan Kordyjan added this to the 3.5.1 milestone Jul 3, 2024
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.

NullPointerException with type lambda + match type
4 participants