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.

Sign up for GitHub

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

Jump to bottom

Change logic to find members of recursive types #17386

Merged
merged 2 commits into from
May 5, 2023
Merged

Change logic to find members of recursive types #17386

merged 2 commits into from
May 5, 2023

Conversation

odersky
Copy link
Contributor

@odersky odersky commented May 1, 2023 •
edited
Loading

The previous logic used two boolean variables to selectively create defensive copies. It was quite complicated. The new logic is simpler and copies less.

  • It copies only if the same recursive type is accessed with two different prefixes. As long as the prefix stays the same,no confusion in the substRecThis is possible.
  • It avoids the openedTwice logic that causes defensive copies at all points in the future. It seems that trick is no longer necessary.

Fixes #17380 by avoiding infinite recursion due to defensive copies.
Fixes #17381 as well.

@odersky
Change logic to find members of recursive types
ded5d25 
The previous logic used two boolean variables to selectively create defensive copies.
It was quite complicated. The new logic is simpler and copies less.

 - It copies only if the same recursive type is accessed with two different
   prefixes. As long as the prefix stays the same,no confusion in the
   `substRecThis` is possible.
 - It avoids the openedTwice logic that causes defensive copies at all points
   in the future. It seems that trick is no longer necessary.

Fixes scala#17380 by avoiding infinite recursion due to defensive copies.
@odersky odersky requested a review from smarter May 2, 2023 08:10
@Kordyjan
Copy link
Contributor

Kordyjan commented May 4, 2023

It changes the error message of #17381 to:

[error] value applyDynamic is not a member of Y, but could be made available as an extension method.
[error]
[error] The following import might fix the problem:
[error]
[error]   import reflect.Selectable.reflectiveSelectable
[error]
[error]   val y2 = y1.u()
[error]            ^^^^

The new error message seems to be expected, so I think we can consider #17381 as also fixed.

@Kordyjan Kordyjan linked an issue May 4, 2023 that may be closed by this pull request
Refinement with concrete type and method throw RecursionOverflow: #17381
Closed
@smarter smarter merged commit f7ba81c into scala:main May 5, 2023
@smarter smarter deleted the fix-17380 branch May 5, 2023 13:07
@Kordyjan Kordyjan added this to the 3.3.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smarter smarter smarter approved these changes

Assignees

@smarter smarter

@Kordyjan Kordyjan

Labels
None yet
Projects
None yet
Milestone
3.3.1
3 participants
@odersky @Kordyjan @smarter