[PATCH] D66743: [clangd] Cleans up the semantic highlighting resources if clangd crashes.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 27 02:31:26 PDT 2019


hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:60
+  // Any disposables that should be cleaned up when clangd crashes.
+  private disposables: vscode.Disposable[] = [];
   fillClientCapabilities(capabilities: vscodelc.ClientCapabilities) {
----------------
nit: call it `subscriptions` to better align with vscode pattern.


================
Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/semantic-highlighting.ts:114
+  // restarts.
+  public crashDispose() {
+    this.disposables.forEach((d) => d.dispose());
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66743/new/

https://reviews.llvm.org/D66743





More information about the cfe-commits mailing list