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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
