[PATCH] D52300: [clangd] Implement VByte PostingList compression

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:29
+      DecompressedChunk = ChunkIndex->decompress();
+      InnerIndex = begin(DecompressedChunk);
+    }
----------------
sammccall wrote:
> nit: we generally use members (DecompressedChunk.begin()) unless actually dealing with arrays or templates, since lookup rules are simpler
>  
I thought using `std::begin(Container)`, `std::end(Container)` is way more robust because the API is essentially the same if the code changes, so I used it everywhere in Dex. Do you think I should change this patch or keep it to keep the codebase more consistent?


================
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:121
 
+/// Single-byte masks used for VByte compression bit manipulations.
+constexpr uint8_t SevenBytesMask = 0x7f;  // 0b01111111
----------------
sammccall wrote:
> move to the function where they're used
But they're used both in `encodeStream()` and `decompress()`. I tried to move as much static constants to functions where they're used, but these masks are useful for both encoding and decoding. Is there something I should do instead (e.g. make them members of `PostingList`)?


https://reviews.llvm.org/D52300