https://github.com/HighCommander4 requested changes to this pull request.
Okay, I had a look through the patch. Thanks for putting it together. I have two high-level pieces of feedback. 1. What the patch is doing is not quite right. It's writing one shard per translation unit (i.e. one shard per source file that appears in the input `compile_commands.json`) -- but clangd's background index contains one shard per _file_, including header files. There's an additional step which takes the information gathered from indexing a translation unit, and splits (shards) it into several pieces for storage. This is implemented in [`BackgroundIndex::update`](https://searchfox.org/llvm/rev/de8126d62a47a571b931e0f21aba0542e1d58e61/clang-tools-extra/clangd/index/Background.cpp#186), and we'll need to do something similar in this patch. * Note, we don't need to do [this part](https://searchfox.org/llvm/rev/de8126d62a47a571b931e0f21aba0542e1d58e61/clang-tools-extra/clangd/index/Background.cpp#243-250) of `BackgroundIndex::update()`. That part has to do with updating the in-memory data structures that clangd uses to answer queries from the index, which we don't need here. 2. While we don't need to implement the combination YAML + sharded, I _would_ like to see sharded vs. monolithic as an `--index-type` option separate from `--format`. With "sharded" lumped into `--format` we get confusing code like the "SHARDED format not supported for serialization" case in `operator<<` that I'd rather avoid. https://github.com/llvm/llvm-project/pull/175209 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
