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

Avoid generating given definitions that loop #19282

Merged
merged 6 commits into from
Dec 23, 2023

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Dec 17, 2023

Fixes a long-standing footgun of implicit resolution: givens that loop back to themselves.

@odersky odersky force-pushed the change-looping-givens branch from e1cc2bb to df79601 Compare December 18, 2023 13:19
@som-snytt
Copy link
Contributor

I appreciate the attention to quality-of-life issues when you might prefer to pursue an interesting area of research today. Hopefully these efforts do not go unnoticed at large.

@odersky odersky force-pushed the change-looping-givens branch from 473b9af to 9c8108d Compare December 18, 2023 20:51
`given ... with` or `given ... = new { ... }` kinds of definitions now follow
the old rules. This allows recursive `given...with` definitions as they are
found in protoQuill.

We still have the old check in a later phase against directly recursive methods.
Of the three loops in the original i15474 we now detect #2 and #3 with new new
restrictions. #1 slips through since it is a loop involving a `given...with` instance
of `Conversion`, but is caught later with the recursive method check.

Previously tests #1 and #3 were detected with the recursive methods check and #2 slipped
through altogether.

The new rules are enough for defining simple givens with `=` without fear of looping.
@odersky odersky force-pushed the change-looping-givens branch from dde86c5 to 07c821a Compare December 18, 2023 22:49
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Very nice, and well received!

@odersky odersky merged commit 938d405 into scala:main Dec 23, 2023
19 checks passed
@odersky odersky deleted the change-looping-givens branch December 23, 2023 17:41
@Kordyjan Kordyjan added the release-notes Should be mentioned in the release notes label Jan 18, 2024
Kordyjan added a commit that referenced this pull request Jan 18, 2024
@Kordyjan Kordyjan added this to the 3.4.1 milestone Feb 14, 2024
@Kordyjan Kordyjan modified the milestones: 3.4.1, 3.4.0 Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes Should be mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants