[PATCH] D51155: [clangd] Allow to merge symbols on-the-fly in global-symbol-builder

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp:61
+    /// rely on MR use-case to work properly.
+    llvm::cl::init(false));
+
----------------
ilya-biryukov wrote:
> ioeric wrote:
> > AFAIK, this flag would basically depend on the executor being used. If executor can provide information like whether all tasks share memory, we can make the decision totally based on the selected executor (i.e. one fewer option).
> That SG. So we could add a virtual method to executor that would provide us with this information and then we can get rid of that option, does that LG?
> However, do we have non-single-binary executor implementations upstream? We need to leave a way to run the code that actually uses YAML serialization to make sure we can experiment when things break.
Sounds good.

> However, do we have non-single-binary executor implementations upstream? We need to leave a way to run the code that actually uses YAML serialization to make sure we can experiment when things break.
The framework allows this, so I'm not sure if there is other downstream implementations that do this (probably not?). But I don't think we can get rid of the YAML serialization at this point because we still dump everything to yaml in the end? 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51155