[PATCH] D53220: Remove possibility to change compile database path at runtime

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Oct 14 23:40:53 PDT 2018


sammccall added a comment.

Thanks for cleaning this up!



================
Comment at: clangd/ClangdLSPServer.cpp:433
 
     reparseOpenedFiles();
   }
----------------
This isn't needed, the compilation database can only be set during initialization.


================
Comment at: clangd/ClangdLSPServer.h:90
   void reparseOpenedFiles();
+  void applyConfiguration(const ClangdInitializationOptions &Settings);
   void applyConfiguration(const ClangdConfigurationParamsChange &Settings);
----------------
Prefer a different name for this function - an overload set should have similar semantics and these are quite different (pseudo-constructor vs mutation allowed at any time).


================
Comment at: clangd/Protocol.h:422
+struct ClangdInitializationOptions : public ClangdConfigurationParamsChange {
+  llvm::Optional<std::string> compilationDatabasePath;
+};
----------------
Can we just move this to InitializeParams as a clangd extension?
Doing tricks with inheritance here makes the protocol harder to understand.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53220





More information about the cfe-commits mailing list