-
Notifications
You must be signed in to change notification settings - Fork 21
jackson-module-scala ClassTagExtensionsTest causes compiler loop (Scala 2.13.2 and after) #12422
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
Comments
The most useful next step here would be to reduce the code in question to an standalone MCVE that reproduces the bug outside the context of the full jackson-module-scala codebase. I don't believe doing that minimization work requires compiler knowledge. |
Thanks @SethTisue - this is probably not a minimal reproducible case but it is more directed -- https://github.com/pjfanning/jackson-classtag-sample |
Yes, that that does look nicer. But the crucial big thing would be to eliminate the external dependency (the dependency on jackson-module-scala).
In such cases, a thread dump is useful to see what's at the top of the stack, so we know where it's getting stuck. At minimum we want to know what phase it's getting stuck in, but usually the stack trace gives even more specific and helpful clues than that. |
I'll see what I can do about producing a standalone test case. In the mean time, this is the most active thread when the code is looping.
|
also on same thread later:
|
Looks like implicit search is looping. Perhaps |
@SethTisue |
I slightly simplified https://github.com/pjfanning/jackson-classtag-sample and now all the scala code is in the test project - it still depends on the Java jackson-databind jar. |
Minimization: object Main extends App {
trait JavaTypeable[T]
object JavaTypeable {
implicit def anyJavaTypeable: JavaTypeable[Any] = ???
implicit def mapJavaTypeable[M[_,_] <: Map[_,_], K : JavaTypeable, V: JavaTypeable]: JavaTypeable[M[K, V]] = ???
implicit def gen5JavaTypeable[T[_, _, _, _, _], A: JavaTypeable, B: JavaTypeable, C: JavaTypeable, D: JavaTypeable, E: JavaTypeable]: JavaTypeable[T[A, B, C, D, E]] = ???
implicit def gen4JavaTypeable[T[_, _, _, _], A: JavaTypeable, B: JavaTypeable, C: JavaTypeable, D: JavaTypeable]: JavaTypeable[T[A, B, C, D]] = ???
implicit def gen3JavaTypeable[T[_, _, _], A: JavaTypeable, B: JavaTypeable, C: JavaTypeable]: JavaTypeable[T[A, B, C]] = ???
implicit def gen2JavaTypeable[T[_, _], A: JavaTypeable, B: JavaTypeable]: JavaTypeable[T[A, B]] = ???
implicit def gen1JavaTypeable[T[_], A: JavaTypeable]: JavaTypeable[T[A]] = ???
implicit def gen0JavaTypeable[T]: JavaTypeable[T] = ???
}
implicitly[JavaTypeable[Map[String, Any]]]
} Add more arities ( |
Workaround: reorder the implicits like this: object Main extends App {
trait JavaTypeable[T]
object JavaTypeable {
implicit def gen5JavaTypeable[T[_, _, _, _, _], A: JavaTypeable, B: JavaTypeable, C: JavaTypeable, D: JavaTypeable, E: JavaTypeable]: JavaTypeable[T[A, B, C, D, E]] = ???
implicit def gen4JavaTypeable[T[_, _, _, _], A: JavaTypeable, B: JavaTypeable, C: JavaTypeable, D: JavaTypeable]: JavaTypeable[T[A, B, C, D]] = ???
implicit def gen3JavaTypeable[T[_, _, _], A: JavaTypeable, B: JavaTypeable, C: JavaTypeable]: JavaTypeable[T[A, B, C]] = ???
implicit def gen2JavaTypeable[T[_, _], A: JavaTypeable, B: JavaTypeable]: JavaTypeable[T[A, B]] = ???
implicit def gen1JavaTypeable[T[_], A: JavaTypeable]: JavaTypeable[T[A]] = ???
implicit def gen0JavaTypeable[T]: JavaTypeable[T] = ???
implicit def mapJavaTypeable[M[_,_] <: Map[_,_], K : JavaTypeable, V: JavaTypeable]: JavaTypeable[M[K, V]] = ???
implicit def anyJavaTypeable: JavaTypeable[Any] = ???
}
implicitly[JavaTypeable[Map[String, Any]]]
} Note that the OP is extremely slow also on Scala 2.13.1 - if we think of a Scope as a linked list (aka stack) that we grow in definition order, then when we lookup the implicits |
Why it matter which implicit we try first is because we compute I don't know what's the purpose of |
thanks @joroKr21, I committed a code reorder to jackson-module-scala and it makes a big difference |
Scala 3 doesn't have this problem so it's either smarter or dumber depending on your PoV |
I couldn't trace it past paulp so I guess I'm not a good archaeologist. |
I forgot about the new given rules for implicit resolution where definitions with less implicit parameters have higher priority which is exactly what we want in this case: /** A relation that influences the order in which eligible implicits are tried.
*
* We prefer (in order of importance)
* 1. more deeply nested definitions
* 2. definitions with fewer implicit parameters
* 3. definitions whose owner has more parents (see `compareBaseClassesLength`)
* The reason for (2) is that we want to fail fast if the search type
* is underconstrained. So we look for "small" goals first, because that
* will give an ambiguity quickly.
*/
def compareEligibles(e1: Candidate, e2: Candidate): Int = Maybe we could apply this in Scala 2 not as a semantic change but as an optimisation? WDYT? |
Well I tried it with shapeless and it became slower, so it looks like it's not a definitive improvement ... and on the other hand having a deterministic order seems at least like a workable solution. |
Thank you @joroKr21 for diving in!
I could not find this in https://dotty.epfl.ch/docs/reference/changed-features/implicit-resolution.html? The comment on It sounds like something we could do, but as you say it might have a negative impact on code that has the implicits in a better order (by accident or maybe even deliberately). I didn't make an attempt to understand why the order matters. Playing around with your example, the Moving the Maybe @smarter hears some bells ringing when reading through this discussion? |
Ah yes you are right, my bad. I remember at some point it was discussed making this a semantic change but now it's indeed only an optimisation whereas the semantic change is that normal overloading rules apply also wrt to given parameters not only the result type. |
The order of definitions basically ends up in reverse as the
Thanks for verifying, I didn't try it. I just fired up a debugger right away based on the provided stack traces. 👍 |
Point 7 on that page says:
|
reproduction steps
using Scala 2.13.2 up to 2.13.6, jackson-module-scala ClassTagExtensionsTest causes compiler loop and test compilation does not complete
works ok with Scala 3.0.0 and Scala 2.11, 2.12 and also 2.13.0 and 2.13.1.
Build uses sbt (
sbt test
will reproduce the problem). Test compilation runs indefinitely.problem
ClassTagExtensions relies on implicit conversions and ClassTags and maybe this is what is causing the problem.
Removing ClassTagExtensionsTest allows the test compilation and test run to succeed.
I'd appreciate if someone who knows more about the scala compiler than I do could have a look.
The text was updated successfully, but these errors were encountered: