Page MenuHomePhabricator

[clangd] Cleans up the semantic highlighting resources if clangd crashes.
ClosedPublic

Authored by jvikstrom on Aug 26 2019, 7:25 AM.

Details

Summary

Disposes of the vscode listeners when clangd crashes and reuses the old highlighter when it restarts. The reason for reusing the highlighter is because this way the highlightings will not disappear as we won't have to dispose of them.

Diff Detail

Repository
rL LLVM

Event Timeline

jvikstrom created this revision.Aug 26 2019, 7:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2019, 7:25 AM
hokein added inline comments.Aug 27 2019, 2:25 AM
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
60 ↗(On Diff #217152)

nit: call it subscriptions to better align with vscode pattern.

114 ↗(On Diff #217152)

we just dispose some class members (leading the class to an intermediate state), I'd suggest that we dispose the whole class.

nit: please remove the "clangd crashes" bit in the API.

jvikstrom updated this revision to Diff 217367.Aug 27 2019, 4:42 AM

Renamed disposables to subscriptions and removed clangd crashes in api.

jvikstrom marked 2 inline comments as done.Aug 27 2019, 4:43 AM
jvikstrom added inline comments.
clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts
114 ↗(On Diff #217152)

The only thing remaining to dispose is the highlighter. Should we remove all highlightings if clangd restarts?

jvikstrom updated this revision to Diff 217396.Aug 27 2019, 7:24 AM

Dispose of all resources when disposing.

hokein accepted this revision.Aug 28 2019, 1:43 AM
This revision is now accepted and ready to land.Aug 28 2019, 1:43 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2019, 6:45 AM