[PATCH] D50970: [clangd] Implement BOOST iterator

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:92
+  /// function, the parameter is needed to propagate through the tree.
+  virtual unsigned boost(DocID ID) const = 0;
 
----------------
Maybe add a note to the comment on why an `ID` parameter actually is here?
IIUC, we need to because various iterators in the tree may point to different elements and we need to know which one we've actually matched.


================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:129
+std::vector<std::pair<DocID, unsigned>>
+consumeAndBoost(Iterator &It,
+                size_t Limit = std::numeric_limits<size_t>::max());
----------------
Could we describe the rationale for keeping both `consume` and `consumeAndBoost` somewhere in the comments?

>From the offline conversation, it seems `consumeAndBoost` is more expensive, but our clients will all use it at some point in the future.
The idea of paying for boosting without actually using it seems bad, so keeping this function separate makes sense.


================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:164
+std::unique_ptr<Iterator> createBoost(std::unique_ptr<Iterator> Child,
+                                      unsigned Factor);
+
----------------
Maybe use `float` scores to align with the scoring code we have for completion?


https://reviews.llvm.org/D50970