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

Stack overflow on subtype of large union for IArray[String] literal #7034

Closed
ryanberckmans opened this issue Aug 13, 2019 · 13 comments · Fixed by #12972
Closed

Stack overflow on subtype of large union for IArray[String] literal #7034

ryanberckmans opened this issue Aug 13, 2019 · 13 comments · Fixed by #12972
Assignees
Milestone

Comments

@ryanberckmans
Copy link

ryanberckmans commented Aug 13, 2019

minimized code

With dotty 0.17.0-RC1

// This code produces the compiler error below
val Names = IArray(
  "Aaliyah",
  "Aaron",
  // ... 5000 more names as string literals
)

// This code triggers the same compile error
val Names: IArray[String] = IArray(/* 5000 names */)

// This code compiles successfully
val Names = IArray[String](/* 5000 names */)


[error] 2 |val Names = IArray(
[error]   |                                      ^
[error]   |Recursion limit exceeded.
[error]   |Maybe there is an illegal cyclic reference?
[error]   |If that's not the case, you could also try to increase the stacksize using the -Xss JVM option.
[error]   |A recurring operation is (inner to outer):
[error]   |
[error]   |  subtype ... | ... | ... | ...(...) | ...(...) | String("Winnie") | String("Winnifred") |
[error]   |
[error]   |String("Winona") | String("Winston") | String("Winter") | String("Wm") |
[error]   |  String("Wonda")

expectation

Expected this code to compile:

IArray("Aaliyah", "Aaron", /* 5000 more names */)```
@ashwinbhaskar
Copy link
Contributor

ashwinbhaskar commented Aug 13, 2019

@ryanberckmans What is IArray? Is it your own custom class?

@ryanberckmans
Copy link
Author

@ashwinbhaskar IArray is a new immutable array type in dotty.

@ashwinbhaskar
Copy link
Contributor

@ryanberckmans ah okay.

@ashwinbhaskar
Copy link
Contributor

I would love to give this a try if someone could just point me the way.

@ashwinbhaskar
Copy link
Contributor

@ryanberckmans Looking at the implementation of IArray it looks like it is a type alias for Array
type IArray[+T] = Array[_ <: T] .

Is it possible to verify if this problem also happens with Array?

@bishabosha
Copy link
Member

bishabosha commented Aug 13, 2019

Which version are you running on, I have tried this with the code

val arr = IArray("qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
"qwertyuiop",
//...7800 more times
)

which compiles fine, including in the repl

@ryanberckmans
Copy link
Author

@bishabosha I'm on dotty 0.17.0-RC1. The "qwertyuiop" example compiles fine I think because its subtype is IArray["qwertyuiop"], which is of fixed size because the same string is repeated 7800 times . The stack overflow comes from 5000 different strings, causing a subtype that's a large union, like IArray["Aaliyah" | "Aaron" ...].

@ryanberckmans
Copy link
Author

@ashwinbhaskar looks like the stack overflow comes from subtype generation. The error is not specific to IArray. A fix might generate this subtype non-recursively or tail-recursively, or perhaps abort subtype generation when a stack depth is hit (which would be a feasible solution in this case because the specific subtype is unused.)

@ashwinbhaskar
Copy link
Contributor

@ryanberckmans hmm..okay. Where is this subtype generation happening?

@ryanberckmans
Copy link
Author

@ashwinbhaskar I'm just a user and have no knowledge of dotty internals. Not sure I can help any further. Cheers

@nicolasstucki
Copy link
Contributor

This looks like an issue with opaque types and subtyping. It might be tricky to fix.

@odersky
Copy link
Contributor

odersky commented Aug 26, 2019

My guess is the issue is with union types, not opaque types. Handling 5000+ element unions of singleton types is super hard. That's precisely the kind of scenario that motivated our initial design where unions could not contain singletons.

I am unsure what to do here. In practice it seems that any union type larger than a couple of hundred operands is a waste of compiler resources and should be widened. But how to do that in a principled way?

@bishabosha
Copy link
Member

Progress has been made to fix this issue in #12928 however there is now a stack overflow in mapWithIndexConserve which is not stack safe
https://github.com/lampepfl/dotty/blob/dd4a23514b765b9d2e35eae798a6fd37395e4fb5/compiler/src/dotty/tools/dotc/core/Decorators.scala#L164

bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 28, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 28, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 28, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 28, 2021
bishabosha added a commit to dotty-staging/dotty that referenced this issue Jun 28, 2021
odersky added a commit that referenced this issue Jun 29, 2021
fix #7034: make mapWithIndexConserve tailrec
BarkingBad pushed a commit to BarkingBad/dotty that referenced this issue Jul 23, 2021
@Kordyjan Kordyjan added this to the 3.0.2 milestone Aug 2, 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.

6 participants