[PATCH] D51982: [clangd] Introduce PostingList interface

Eric Liu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 13 04:39:53 PDT 2018


ioeric added inline comments.


================
Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:125
+  for (const auto &TokenToPostingList : TempInvertedIndex)
+    InvertedIndex.insert(
+        {TokenToPostingList.first,
----------------
nit: `InvertedIndex[TokenToPostingList.first] = buildSparsePostingList(...)` ?


================
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:23
+/// compress underlying data.
+class SparsePostingList : public PostingList {
+public:
----------------
I'd probably call this something other than `SparsePostingList`. It doesn't really do anything fancier than just storing the vector. What if sparse posting lists have their own optimizations one day? So how about something more straightforward? E.g. `VectorPostingList` or `PlainPostingList`?


================
Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:40
+  /// std::vector<DocID>::const_iterator.
+  class SparseIterator : public Iterator {
+  public:
----------------
As this is just a helper class that is not exposed, I'd suggest making this a standalone class to make it easier to read.


https://reviews.llvm.org/D51982





More information about the cfe-commits mailing list