kbobyrev updated this revision to Diff 157206.
kbobyrev added a comment.
Slightly refactored the code, improved documentation. This patch is ready for
review.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
cl
kbobyrev added inline comments.
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:181
+const auto HighestID = peek();
+DocID ChildrenToAdvance = 0;
+while ((ChildrenToAdvance++ < Children.size()) && !reachedEnd() &&
sammccall wrote:
> I can'
kbobyrev updated this revision to Diff 157203.
kbobyrev marked 14 inline comments as done.
kbobyrev added a comment.
Address the last round of comments.
Incoming: documentation overhaul.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd
sammccall added a comment.
This looks really nice.
Iterator implementations can be simplified a bit I think.
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:50
+ OS << Documents[Index];
+ if (Index + 1 != Documents.size())
+OS << ", ";
---
kbobyrev updated this revision to Diff 157004.
kbobyrev added a comment.
Refactored unit tests in few places.
https://reviews.llvm.org/D49546
Files:
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tools-extra/clangd/index/dex/Iterator.h
clan
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 &Left,
+ std::unique_ptr &Right) -> bool {
---
kbobyrev updated this revision to Diff 156997.
kbobyrev marked 19 inline comments as done.
kbobyrev added a comment.
Herald added a subscriber: jfb.
As discussed offline: I simplified the implementation and cleaned up unit
tests. Should look better now.
The next step is to document the implement
sammccall added a comment.
A few more comments about the bits I understand, but waiting mostly on the
documentation.
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.cpp:155
+ llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
+OS << "(&& ";
+
kbobyrev updated this revision to Diff 156725.
kbobyrev added a comment.
Herald added a subscriber: mgrang.
- Implemented convenient dumping via `llvm::raw_ostream
&operator<<(llvm::raw_ostream &OS, QueryIterator &Iterator)`, which dumps
iterator tree in human-readable format, e.g. `(&& [1, 2, 3
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 Iterator` to
recursively pretty print queries in human-readable format: e.g. `(& [0, 1, 2,
3]
kbobyrev updated this revision to Diff 156483.
kbobyrev marked 9 inline comments as done.
kbobyrev added a comment.
- Switched from `std::shared_ptr` to `std::unique_ptr` for iterator's children:
iterators own their subtrees, the lifetime should depend on the root
- Store `PostingListRef`in `Docu
sammccall added a comment.
Thanks for sending this early!
Rough interface comments - mostly looks good though!
Comment at: clang-tools-extra/clangd/index/dex/QueryIterator.h:26
+
+using PostingList = std::vector;
+
we should at least use a type alias for a DocI
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay, ilya-biryukov, mgorny.
This is a proof-of-concept implementation of query iterators. At the moment, it
is pretty messy an
13 matches
Mail list logo
