[PATCH] D39843: [clangd] Use in-memory preambles in clangd.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clangd/ClangdUnit.cpp:238
   if (Preamble) {
-    Preamble->AddImplicitPreamble(*CI, Buffer.get());
+    Preamble->AddImplicitPreamble(*CI, /*ref*/ VFS, Buffer.get());
   } else {
----------------
sammccall wrote:
> ref is still here :-)
Thx. Missed that. Removed.


================
Comment at: clangd/tool/ClangdMain.cpp:48
 
+static llvm::cl::opt<bool> StorePreamblesInMemory(
+    "in-memory-pchs",
----------------
sammccall wrote:
> Couple of questions about the flag syntax:
>  - I wonder about the granularity - if we had another temp storage (indexes?), would we want another flag or something shared like `-in-memory-storage`
>  - Having this as a bool seems confusing compared to `-pch-storage={memory/disk}`. (We could also overload such a flag to allow setting the location, but nobody's asked for that yet)
  - Indexes are fundamentally different because we actually want to persist them across multiple sessions. But I see your point. I'd go with another flag for greater flexibility. We do want clangd work great by-default, so those flags are mostly for people who know what they're doing and can figure out which flags they want to change and why. Maybe I'm not right, though, and people would want those kinds of switches.
  - Totally like the `-pch-storage={memory/disk}`  more, updated the change accordingly.




https://reviews.llvm.org/D39843