-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Incremental compilation leads to Cyclic Reference Error #20245
Comments
Thanks for the minimization! Here's a standalone reproducer as a pos test:
Run as pos test. |
It works without cyclic errors if unification has an explicit type: private[typer] val unification: Unification = new Unification(using this) |
Hey thanks for looking into it. Maybe I reduced it too much :( In the original compiler adding the type annotation to
The spurious |
Also report type-checked right hand sides and export expansions. Streamline trace-handling code using inline functions. Fixes scala#20245
…ava. We don't usually need it. I played with it to figure out scala#20245, and found out it was set up incorrectly.
I added some more diagnostics in #20251. You need to compile with -explain-cyclic then that gives hopefully more info. It's unfortunately not feasible to ensure that cyclic errors appear independent of compilation order. Also, messages after the first "Cyclic Reference" should usually be ignored. Once a cyclic reference happens all bets are off and follow-on errors are expected. |
@b-studios Did you compile the project with -explain-cyclic? Any new insights what might be the cause? If not I could take a look at the error trace. Ideally, with -explain-cyclic and -Ydebug-cyclic both set. |
@odersky Here is the trace generated by the latest nightly
Thanks for looking into it |
Also report type-checked right hand sides and export expansions. Streamline trace-handling code using inline functions. Fixes #20245
#20251 would hopefully give some more information than nightly. I justy merged it, so next nightly should work better. But anyway, let's look at the trace: Cyclic reference involving class Context Yes, since TyperOps is a parent of Context, we need to compute its baseclasses. [error] | which required to compute the signature of trait TyperOps Which means we have to elaborate its classinfo, (i.e. run Namer on it). [error] | which required to compute the signature of value unification That's an interesting bit. Why does it need to know the type of [error] | which required to compute the base classes of class Context That's the mysterious bit. Why does it need the base classes of Context. What does the definition of |
private[typer] val unification = new Unification(using this)
export unification.{ requireSubtype, requireSubregion, join, instantiate, instantiateFresh, freshTypeVar, freshCaptVar, without, requireSubregionWithout } |
Did you try adding an explicit type for |
The way it is written it's clearly a cycle. The RHS needs to be type-checked, which requires typing I believe #20251 would have clarified that much better. |
Yes, I tried annotating. Here is the trace with the annotation on
(update: I had to update the trace just now) |
I guess the question is why does it need the base classes of Context when running the namer on TyperOps? Maybe the next nightly will give more info. Could it be an implicit search that needs to analyze the import? |
Closing this for now, let's re-open if there's new evidence that the diagnostics are insufficient. |
Incrementally recompiling leads to "Cyclic reference" while fresh compilation succeeds.
We consistently hit this issue when touching our own
Typer.scala
forcing us to alwaysclean; compile
which of course takes longer than incrementally compiling.Compiler version
3.3.1, 3.4.1
Minimized code
I minimized our compiler to only a few lines of code in:
effekt-lang/effekt#446
the full minimized example (not the diff or PR) can be found here
https://github.com/effekt-lang/effekt/tree/34b9e79126db91604df4785aaabace15d154f8cb
Output
Expectation
It should consistently compile or always report the error, if this is indeed a problem.
The text was updated successfully, but these errors were encountered: