-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: cyclic dependency panic #21117
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
base: master
Are you sure you want to change the base?
fix: cyclic dependency panic #21117
Conversation
|
Could you add a test for rust‑analyzer to address issue #21006? Additionally, I suggest we investigate the root cause of the cycle reported in that issue:
|
the cycle is expected as it arises from rust code patterns that rustc handles correctly. Our lowering is correct but it just needs proper cycle handling like other queries. |
|
Which pattern? Could you point it out? |
|
pattern when a trait's where clause references a type alias that depends on trait itself by its type parameter. |
|
Well, your test passes even without the cycle handler |
I am having some difficulty to write a proper test. The test still passes. Can you suggest me some ways to do it? |
|
The best way is to start with the panicking code, and remove more and more code while keeping it panicking until you get the minimal snippet which still reproduces the problem. However this query should never have a cycle (its parallels in rustc don't have either), and therefore the cycle means we're lowering things incorrectly. This fix is therefore also wrong. |
closes #21006
In commit 5038446, the
query_with_diagnosticsmethod was converted to a salsa tracked query but the cycle handler was not added during the conversion while other methods liketype_for_type_alias_with_diagnostics,generic_predicates_for_paramalready have the cycle handlers.So, I added a cycle handler for the
query_with_diagnosticsmethod.And on testing, I found the issue was fixed.