kbobyrev planned changes to this revision.
kbobyrev added a comment.
Upcoming changes:
- Improve debugging experience by providing `llvm::raw_ostream
&operator<<(llvm::raw_ostream &OS, std::unique_ptr<QueryIterator> Iterator` to
recursively pretty print queries in human-readable format: e.g. `(& [0, 1, 2,
3] ([3, 4, 6, 8] || [0, 1]))`. This can be later used for debugging and making
sure optimizations were correct.
- Properly document the interface and implementation. The current documentation
is just a draft, it should be refined.
- Think about introducing iterator `cost` and applying cheap optimizations like
pre-sorting children of `AndIterator` before performing `advance()`: this way
the iterator doesn't spend too much time iterating through long posting lists
when it has are short/empty children
================
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.h:39
+public:
+ virtual bool reachedEnd() const = 0;
+ // FIXME(kbobyrev): Should advance() and advanceTo() return size_t peek()
----------------
sammccall wrote:
> or have peek return an InvalidID == size_t{-1}?
Tried to do that, but that creates additional complexity for the `AndIterator`
which stores `ReachedEnd` anyway. Otherwise I would either have to spend
`O(Children.size())` to understand that `AndIterator` reached the end or have
inconsistent APIs across the classes, I think it's better to leave it as it is.
================
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.h:44
+ virtual bool advanceTo(size_t Rank) = 0;
+ virtual size_t peek() const = 0;
+
----------------
sammccall wrote:
> could reasonably call this operator*
As discussed offline, `*(*Child)` would be probably not very clean.
https://reviews.llvm.org/D49546
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
