[PATCH] D96244: [clangd] Introduce Modules

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/ClangdServer.h:33
 #include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FunctionExtras.h"
----------------
sammccall wrote:
> (include no longer used?)
well it is still used by `formatCode` :P (but dropping to keep the irrelevant changes out, same for memory and vector)


================
Comment at: clang-tools-extra/clangd/Module.h:31
+public:
+  virtual ~Module() = default;
+
----------------
sammccall wrote:
> Should either:
>  - doc that destructor should cancel as much background work as possible and block until it's done
>  - add a requestStop() and doc that destructor should block
>  - remove the concept of background work completely (i.e. blockUntilIdle())
as discussed offline going with `remove the concept of background work completely (i.e. blockUntilIdle())`. we can go with requestStop and blocking destructors, once the need arises. (i.e. we better understand the usecases)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96244