kbobyrev added inline comments.
================
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.cpp:175
+// FIXME(kbobyrev): This compares IDs of two iterators...
+auto CompareIterators = [](std::unique_ptr<QueryIterator> &Left,
+ std::unique_ptr<QueryIterator> &Right) -> bool {
----------------
sammccall wrote:
> can this just be a static function?
Wiped this one. Also, in the future I guess it's better to implement
`operator<()` for iterators, because that's what this essentially is.
================
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.cpp:196
+ });
+ std::make_heap(begin(Children), end(Children), CompareIterators);
+ }
----------------
sammccall wrote:
> can we start with the simple/obvious implementation, and add heap later if it
> actually improves performance?
Done. It doesn't look simpler to me, but I tried to keep it as simple as
possible. I also put a `FIXME` on top of `OrIterator` with the trade-offs
description.
================
Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:324
+TEST(DexIndexIterators, QueryTree) {
+ // An example of more complicated query
+ //
----------------
sammccall wrote:
> This is too complicated. Try to use each iterator once or twice.
> (We'll need to extend this when we add new iterator types)
Done. Dropped this one and two other complicated cases.
https://reviews.llvm.org/D49546
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
