[PATCH] D42429: [clangd] Moved caching of compile commands to ClangdServer

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks for this cleanup!

The way we deal with ResourceDir still doesn't feel ideal, but that's no worse here and everything else is better!



================
Comment at: clangd/ClangdServer.cpp:211
 
+  // Clear the cached CompileCommand for File, this ensures the new one will be
+  // requested for the next reparse.
----------------
nit: the comment and whitespace seems unneccesary, this is exactly what invalidate() is documented to do.
(A comment saying *why* might be useful, but we don't really know why here)


================
Comment at: clangd/ClangdServer.cpp:285
   std::shared_ptr<PCHContainerOperations> PCHs = this->PCHs;
 
+  tooling::CompileCommand CompileCommand = CompileArgs.getCompileCommand(File);
----------------
nit: remove blank line?


================
Comment at: clangd/ClangdServer.h:339
 
-  GlobalCompilationDatabase &CDB;
+  // Stores compile commands produced by GlobalCompilationDatabase.
+  class CompileArgsCache {
----------------
nit: can we hide this in the cpp file or move to GCD.h to reduce clutter?


================
Comment at: clangd/ClangdServer.h:339
 
-  GlobalCompilationDatabase &CDB;
+  // Stores compile commands produced by GlobalCompilationDatabase.
+  class CompileArgsCache {
----------------
sammccall wrote:
> nit: can we hide this in the cpp file or move to GCD.h to reduce clutter?
Can the comment give some motivation here? It's clear enough from the name what it does, but why?


================
Comment at: clangd/ClangdServer.h:340
+  // Stores compile commands produced by GlobalCompilationDatabase.
+  class CompileArgsCache {
+  public:
----------------
sorry for naive question - this doesn't need to be threadsafe, because clangdserver itself isn't, right?
I never really looked at the threading model of clangdserver - it looks like most stuff is called externally on one thread, and any async logic avoids accessing most of the members?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42429