[PATCH] D53687: [clangd] Make in-memory CDB always available as an overlay, refactor.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

FWIW, the old implementation of the CDB looked simpler (which makes sense, since it only allowed the in-memory compile commands, but the new implementation also falls back to the base CDB, i.e. it's now doing two things).
However, if we can't avoid this protocol extension, the change LG and the model behind it should hopefully make sense from the user's perspective.
Still not sure whether overriding the normal command (not the fallback command) when we also read from the CDB is a useful feature.



================
Comment at: clangd/ClangdLSPServer.cpp:670
+          /*Output=*/"");
+      if (Old != New)
+        CDB->setCompileCommand(File, std::move(New));
----------------
Is this an optimization to not trigger compile command changes?
Can we perform it on the CDB level to make sure we hit it in the future if more code calling `setCompileCommand` is added?


================
Comment at: clangd/ClangdLSPServer.h:42
   ClangdLSPServer(Transport &Transp, const clangd::CodeCompleteOptions &CCOpts,
-                  llvm::Optional<Path> CompileCommandsDir,
-                  bool ShouldUseInMemoryCDB, const ClangdServer::Options &Opts);
+                  llvm::Optional<Path> CompileCommandsDir, bool NoReadCDB,
+                  const ClangdServer::Options &Opts);
----------------
NIT: the negative variable names might cause confusion (i.e. the double negations are hard to read). Maybe use the name of the corresponding field (UseDirectoryCDB)?


================
Comment at: clangd/ClangdLSPServer.h:139
+  llvm::Optional<OverlayCDB> CDB;
+  std::unique_ptr<GlobalCompilationDatabase> BaseCDB;
   // The ClangdServer is created by the "initialize" LSP method.
----------------
Maybe add a comment on how the two compilation databases are combined?


================
Comment at: clangd/GlobalCompilationDatabase.h:79
+/// using an in-memory mapping.
+class OverlayCDB : public GlobalCompilationDatabase {
 public:
----------------
The name does not seem to fully capture what this class does, i.e. allowing to override compile commands for some files.
Maybe use `OverridenCDB` or something similar?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53687