-
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
Better error diagnostics for cyclic references #19408
Conversation
We now suggest to compile with -explain-cyclic, in which case we give a trace of the forcings that led to the cycle. The reason for the separate option is that maintaining a trace is not free so we should not be doing it by default.
I used this for #19398, which gave this:
After I turned the stacktrace on with -Ydebug-cyclic and correlated the two pieces of info, I found the root cause: It's because we now use more self types, which are annotated types, and which create a link back from Iterable to Seq when loading Tasty. I'll try to tweak things on the cc side so that the self types are not needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise, LGTM
val traceCycles = CyclicReference.isTraced | ||
try | ||
if traceCycles then | ||
CyclicReference.pushTrace("complete the info of ", symbol, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"complete the info of"
will be shown to users with -explain-cyclic
. This terminology might not be clear to them. Maybe it should be something along the lines of "computing the signature of"
) Backports #19408 to the LTS branch. PR submitted by the release tooling. [skip ci]
We now suggest to compile with -explain-cyclic, in which case we give a trace of the forcings that led to the cycle.
The reason for the separate option is that maintaining a trace is not free so we should not be doing it by default.