Skip to content

Fix crashing on semantic tokens going past last line#833

Merged
dzhou121 merged 1 commit intolapce:masterfrom
MinusGix:semantic-tokens-line
Jul 20, 2022
Merged

Fix crashing on semantic tokens going past last line#833
dzhou121 merged 1 commit intolapce:masterfrom
MinusGix:semantic-tokens-line

Conversation

@MinusGix
Copy link
Copy Markdown
Member

There is a crash where on a large project (like lapce, I typically did testing in the middle of lsp.rs) it would crash if you typed quickly (easy method: hold down enter). I believe this is a timing related crash.
This sort of fixes the issue by just stopping the processing of the semantic tokens request if there is a newer revision of the buffer. This works because we just do a full semantic tokens request each time, and so if there's newer buffer (different rev) then we can just stop processing. This is probably a minor perf benefit in of itself.

My theory as to what was happening:
I noticed that it crashed when it processed (request_semantic_tokens finishing) a newer revision before an older revisions request. You'd be going through a bunch of revisions as you're typing quickly.
I believe RA is getting told of the new file contents before the old request has finished, and so it provides offsets for the new file contents rather than the old ones. Then those new offsets cause a crash on the old text because they go out of bounds in it.

@MinusGix
Copy link
Copy Markdown
Member Author

I realized this is an imperfect fix, because it is visible that highlighting isn't occurring until you stop typing.
So, perhaps I should just do a fix inside of the format semantic tokens that makes sure it doesn't panic. This is somewhat unpleasant but it would work and keep styling appearing in most cases.
I'll open a separate PR for that, since it seems like the better choice.

@MinusGix MinusGix closed this Jul 20, 2022
@MinusGix MinusGix reopened this Jul 20, 2022
@MinusGix
Copy link
Copy Markdown
Member Author

Actually I think this is fine? We don't use the results of the semantic highlighting if the revision doesn't match so there isn't actually a change. I was just noticing an existing issue of semantic highlighting not being immediate.. I think.

@dzhou121 dzhou121 merged commit 3ff22b3 into lapce:master Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants