Regression: crash of JVM backend in kuceramartin/tyqu #18179
Comments
WojciechMazur
added
itype:bug
area:backend
itype:crash
stat:needs minimization
|
Did you check the 3.3.2 nightly after #18077 was merged? It handles a stale symbol error like this. |
Yes, it still fails in latest nightly: 3.3.2-RC1-bin-20230710-ed319e8-NIGHTLY |
|
@KuceraMartin, we will actually have a spree on August 1st. Would you like to look into this issue then? |
|
The crash happens when compiling this test: https://github.com/KuceraMartin/tyqu/blob/c1747edc67ae69cfecd5c5f2ee00c5dc7643e158/src/test/translators/GenericSqlTranslatorTest.scala#L324-L335 It can be simplified to: case object MyTable extends Table:
val id = Column[Int]()
val firstName = Column[String]()
val lastName = Column[String]()
val age = Column[Int | Null]()
val translator = new GenericSqlTranslator(MySqlPlatform)
val query = translator.translate(
from(MyTable).map { t =>
(t.id - t.age.getOrElse(0) * t.id)
}
)From what I've observed it fails when we reference at least 2 different columns, and use one of them at least twice |
|
@mbovel I’d be happy to work on it during the next spree but I’m away and with poor internet connection so I haven’t been able to register. Is it fine if I just join the zoom call without signing up? |
|
The problem appears starting from this commit: KuceraMartin/tyqu@2cc26d5 It also doesn't compile with Scala 3.2.0. (that is an issue we knew of but did not investigate because it was working again in 3.2.2., see https://lampepfl.slack.com/archives/C0L95M1J8/p1674819778873259) |
|
Thanks for investigating this! And sorry I didn't answer the previous message; I was on vacation. |
|
This issue was picked for the Issue Spree No. 35 of 22 August 2023 which takes place in 7 days. @mbovel, @KuceraMartin will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here. |
|
Minimized to ( abstract sealed class Expression[T]
case class Aggregation[T]() extends Expression[T | Null]
case class Plus[T <: Int | Null, E1 <: Expression[T]](lhs: E1) extends Expression[T]
class GenericSqlTranslator:
def translateExpression(expr: Expression[?]): Unit =
expr match
case Plus(lhs) => ()Stack trace |
|
The regression occurs multiple times (i.e. there are many commits on which it works and many on which it doesn't) so bisection doesn't work. I have identified 1 commit on which it breaks: 2d641ec |
|
I've been looking into this and one thing that makes it really hard to properly understand what's going on here is the use of |
|
Well, yeah, when trying to find the instantiation of a subclass such that it's a subtype, maximising is correct, to avoid considering only a subset of values. So given a type argument (as a part of protoTp1) that's in invariant position, it's important not to arbitrarily pick one of the bounds. Now maybe it's fine for it to stay as an uninstantiated type var, rather than instantiated to a fresh symbol. Even more so, I don't know if it's necessary to add the fresh symbol to the gadt constraint - but perhaps there's a nested match test case that demonstrates that need to create a fresh symbol and add it.. Arbitrarily picking one of the bounds very likely leads to false positives - probably false positive exhaustivity warnings if the SpaceEngine instantiates a bigger type for the decomposition of the scrutinee type than Typer assigned to the unapply extractor. |
Yeah that's what I had in mind, these symbols only make sense during typechecking for GADT it seems. |
|
Seems to be fixed in 3.4.2-RC1-bin-20240214-e96cb18-NIGHTLY, not sure which exact change caused that. |
|
It may have been 3b03592 |
Compiler version
Fails in 3.3.0 (stacktrace below)
Fails in 3.3.2-RC1-bin-20230707-7ede150-NIGHTLY with different error message
Works in 3.2.2
Minimized code
Not yet minimized, based on compilation of OSS project: https://github.com/kuceramartin/tyqu.git
Output (click arrow to expand)
The text was updated successfully, but these errors were encountered: