[PATCH] D50955: [clangd] Implement TRUE Iterator

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:230
+/// order to prevent additional memory consumption, it only stores the size of
+/// this virtual posting list because posting lists of such density are likely
+/// to consume a lot of memory. All operations are performed in O(1) as a
----------------
nit: The documentation here is a bit verbose as. How about `... It stores size of the virtual posting list, and all operations are performed in O(1).`? Other statements can be easily derived from this IMO.


================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:248
+    assert(!reachedEnd() && "Can't advance iterator after it reached the end.");
+    Index = ID;
+  }
----------------
Should we check `ID < Size` here?


================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:258
+  llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+    OS << "(TrueIterator {" << Index << "} out of " << Size << ")";
+    return OS;
----------------
nit: maybe `s/TrueIterator/TRUE/`?


https://reviews.llvm.org/D50955