Skip to content
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

The Semantics API is prone to panics #17367

Open
Veykril opened this issue Jun 8, 2024 · 1 comment
Open

The Semantics API is prone to panics #17367

Veykril opened this issue Jun 8, 2024 · 1 comment
Labels
A-ide general IDE features Broken Window Bugs / technical debt to be addressed immediately I-panic

Comments

@Veykril
Copy link
Member

Veykril commented Jun 8, 2024

We tend to have a lot of IDE features randomly panic simply because a node that is being used in a lookup is not cached in the parse tree cache. This is really annoying and keeps getting worse and better every few weeks. We ought to remodel the APIs involved to either make this practically impossible (any node that may be produced immediately being cached) or have the cache be redundant for lookups (passing the corresponding file ids, making the find_file internal function obsolete).

The former would be nicer in terms of usability, but the latter will ensure the problem from not occuring anymore (unless the file id is mismatched but that's is a lot more difficult to run into than it is to not cache a node).

@Veykril Veykril added Broken Window Bugs / technical debt to be addressed immediately A-ide general IDE features I-panic labels Jun 8, 2024
@Veykril
Copy link
Member Author

Veykril commented Jun 9, 2024

Another thing I just realized (as r-a immediately panicked on start up for me in inlay hints), the LRU cache can cause issues here. The root_to_file_cache maps root nodes to the file ids, equality for nodes is identity based. Now if we cache one copy from the LRU in there, then the LRU evicts this one result in the Database and we requery it somehow we will now fail the lookup because the identitiy of the root node differs.

bors added a commit that referenced this issue Jun 10, 2024
fix: Remove extra parse cache from Semantics again

Should fix #17376, specifically 30c04d5

The recent changes here were heavily triggering what I realized in #17367 (comment)
lnicola pushed a commit to lnicola/rust that referenced this issue Jun 23, 2024
fix: Remove extra parse cache from Semantics again

Should fix rust-lang/rust-analyzer#17376, specifically 30c04d5aa9523140f0f2daa07bc461534fb09b95

The recent changes here were heavily triggering what I realized in rust-lang/rust-analyzer#17367 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ide general IDE features Broken Window Bugs / technical debt to be addressed immediately I-panic
Projects
None yet
Development

No branches or pull requests

1 participant