Skip to content

Recursive type throws RecursionOverflow: Recursion limit exceeded #17380

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

Closed
steinybot opened this issue May 1, 2023 · 4 comments · Fixed by #17386
Closed

Recursive type throws RecursionOverflow: Recursion limit exceeded #17380

steinybot opened this issue May 1, 2023 · 4 comments · Fixed by #17386

Comments

@steinybot
Copy link

Compiler version

3.3.0-RC3

Minimized code

class C { type T; type U }

type X = (C { type U = T; def u: U } { type T = String })

Output

Recursion limit exceeded.
Maybe there is an illegal cyclic reference?
If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
For the unprocessed stack trace, compile with -Yno-decode-stacktraces.
A recurring operation is (inner to outer):

  find-member C#T
  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U
  find-member C#T
  ...

  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U
  find-member C#T
  find-member C#U

Expectation

It should compile (I think?).

I was trying to figure out what these RecTypes were so I took the example from lookuprefined.scala and added the u member to it.

@steinybot steinybot added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels May 1, 2023
@Kordyjan Kordyjan added area:typer stat:needs info and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels May 1, 2023
@Kordyjan
Copy link
Contributor

Kordyjan commented May 1, 2023

@odersky Should this compile? It looks like it should, but I don't know the exact limitations around types like that.

@sjrd
Copy link
Member

sjrd commented May 1, 2023

It seems like a reasonable type to me. AFAICT it should compile, indeed. At the very least, there's nothing cyclic in there.

odersky added a commit to dotty-staging/dotty that referenced this issue May 1, 2023
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 self-assigned this May 1, 2023
@odersky
Copy link
Contributor

odersky commented May 1, 2023

Yes, this should compile. Trying a fix...

@steinybot
Copy link
Author

Fixed by the time I woke up the next morning 😄

smarter added a commit that referenced this issue May 5, 2023
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.
@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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants