[PATCH] D51585: [clangd] Define a compact binary serialization fomat for symbol slab/index.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Super cool! Just a few nits.



================
Comment at: clangd/RIFF.cpp:58
+  if (RIFF->ID != fourCC("RIFF"))
+    return makeError("Extra content after RIFF chunk");
+  if (RIFF->Data.size() < 4)
----------------
The error message seems wrong?


================
Comment at: clangd/index/Serialization.cpp:50
+
+void writeVar(uint32_t I, raw_ostream &OS) {
+  constexpr static uint8_t More = 1 << 7;
----------------
This function could use a comment. What's the difference between this and `write32`?


================
Comment at: clangd/index/Serialization.cpp:96
+  std::vector<StringRef> Sorted;
+  DenseMap<std::pair<const char *, size_t>, unsigned> Index;
+
----------------
Any reason to use `std::pair<const char *, size_t>` instead of `StringRef`?


================
Comment at: clangd/index/Serialization.cpp:335
+  std::vector<Symbol> Symbols;
+  for (const auto &Sym : *Data.Symbols) {
+    Symbols.emplace_back(Sym);
----------------
`assert(Data.Symbols)`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51585