[PATCH] D48071: [clangd] Add an option controlling caching of compile commands.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/tool/ClangdMain.cpp:141
+                   "come from the compilation databases."),
+    llvm::cl::init(false), llvm::cl::Hidden);
+
----------------
sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > init(true) to avoid changing behavior?
> > This is intentional, it seems `false` is a better default, given that all implementation in LLVM cache it all themselves.
> > We should set to true for our CDB that actually needs it. WDYT?
> I agree the default should be false, at least eventually.
> 
> I'm not sure what you're suggesting though - we change the default value of the flag in our own copy when we link against our DB that doesn't cache?
> 
> That could work. I wonder if this will break others who might have similar DBs.
I suggested telling our users to set this flag to true.
Otherwise I think we should just punt on this change and not introduce the flag. If we'll have dependency tracking, caching could probably be nicely incorporated into the build system integration and than we'll just remove the caching from ClangdLSPServer.
Happy to go this way, BTW, I don't think this cache creates any problems apart from some inefficiency.

The flag is of no use if we're the only ones who need to set it to 'true', but 'true' is also the default.

> I wonder if this will break others who might have similar DBs.
Do we know of anyone else who builds clangd with custom DB implementations?
If there are any and this change breaks them, we can revert it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48071