[PATCH] D50500: [clangd] Allow consuming limited number of items
This revision was automatically updated to reflect the committed changes.
Closed by commit rL339426: [clangd] Allow consuming limited number of items
(authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D50500?vs=160088=160089#toc
Repository:
rL LLVM
https://reviews.llvm.org/D50500
Files:
clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
clang-tools-extra/trunk/clangd/index/dex/Iterator.h
clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -240,6 +240,27 @@
"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
}
+TEST(DexIndexIterators, Limit) {
+ const PostingList L0 = {4, 7, 8, 20, 42, 100};
+ const PostingList L1 = {1, 3, 5, 8, 9};
+ const PostingList L2 = {1, 5, 7, 9};
+ const PostingList L3 = {0, 5};
+ const PostingList L4 = {0, 1, 5};
+ const PostingList L5;
+
+ auto DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+}
+
testing::Matcher>
trigramsAre(std::initializer_list Trigrams) {
std::vector Tokens;
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator ) {
+std::vector consume(Iterator , size_t Limit) {
std::vector Result;
- for (; !It.reachedEnd(); It.advance())
+ for (size_t Retrieved = 0; !It.reachedEnd() && Retrieved < Limit;
+ It.advance(), ++Retrieved)
Result.push_back(It.peek());
return Result;
}
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h
@@ -101,9 +101,10 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
@@ -240,6 +240,27 @@
"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
}
+TEST(DexIndexIterators, Limit) {
+ const PostingList L0 = {4, 7, 8, 20, 42, 100};
+ const PostingList L1 = {1, 3, 5, 8, 9};
+ const PostingList L2 = {1, 5, 7, 9};
+ const PostingList L3 = {0, 5};
+ const PostingList L4 = {0, 1, 5};
+ const PostingList L5;
+
+ auto DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+}
+
testing::Matcher>
trigramsAre(std::initializer_list Trigrams) {
std::vector Tokens;
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator ) {
+std::vector consume(Iterator , size_t Limit) {
std::vector Result;
- for (; !It.reachedEnd(); It.advance())
+ for (size_t Retrieved = 0; !It.reachedEnd() && Retrieved < Limit;
+ It.advance(), ++Retrieved)
Result.push_back(It.peek());
return Result;
}
Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h
[PATCH] D50500: [clangd] Allow consuming limited number of items
kbobyrev updated this revision to Diff 160088.
kbobyrev marked 2 inline comments as done.
https://reviews.llvm.org/D50500
Files:
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tools-extra/clangd/index/dex/Iterator.h
clang-tools-extra/unittests/clangd/DexIndexTests.cpp
Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -240,6 +240,27 @@
"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
}
+TEST(DexIndexIterators, Limit) {
+ const PostingList L0 = {4, 7, 8, 20, 42, 100};
+ const PostingList L1 = {1, 3, 5, 8, 9};
+ const PostingList L2 = {1, 5, 7, 9};
+ const PostingList L3 = {0, 5};
+ const PostingList L4 = {0, 1, 5};
+ const PostingList L5;
+
+ auto DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+}
+
testing::Matcher>
trigramsAre(std::initializer_list Trigrams) {
std::vector Tokens;
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,9 +101,10 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator ) {
+std::vector consume(Iterator , size_t Limit) {
std::vector Result;
- for (; !It.reachedEnd(); It.advance())
+ for (size_t Retrieved = 0; !It.reachedEnd() && Retrieved < Limit;
+ It.advance(), ++Retrieved)
Result.push_back(It.peek());
return Result;
}
Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -240,6 +240,27 @@
"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
}
+TEST(DexIndexIterators, Limit) {
+ const PostingList L0 = {4, 7, 8, 20, 42, 100};
+ const PostingList L1 = {1, 3, 5, 8, 9};
+ const PostingList L2 = {1, 5, 7, 9};
+ const PostingList L3 = {0, 5};
+ const PostingList L4 = {0, 1, 5};
+ const PostingList L5;
+
+ auto DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+}
+
testing::Matcher>
trigramsAre(std::initializer_list Trigrams) {
std::vector Tokens;
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,9 +101,10 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator
[PATCH] D50500: [clangd] Allow consuming limited number of items
ioeric accepted this revision. ioeric added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:252 + auto DocIterator = create(L0); + EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100)); + nit: maybe use the default parameter here? Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:260 + + auto Root = createAnd(createAnd(create(L1), create(L2)), +createOr(create(L3), create(L4), create(L5))); The AND iterator case doesn't seem to add more coverage. Maybe drop? https://reviews.llvm.org/D50500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50500: [clangd] Allow consuming limited number of items
kbobyrev updated this revision to Diff 159959.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.
Fix the implementation and add test coverage.
https://reviews.llvm.org/D50500
Files:
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tools-extra/clangd/index/dex/Iterator.h
clang-tools-extra/unittests/clangd/DexIndexTests.cpp
Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -240,6 +240,29 @@
"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
}
+TEST(DexIndexIterators, Limit) {
+ const PostingList L0 = {4, 7, 8, 20, 42, 100};
+ const PostingList L1 = {1, 3, 5, 8, 9};
+ const PostingList L2 = {1, 5, 7, 9};
+ const PostingList L3 = {0, 5};
+ const PostingList L4 = {0, 1, 5};
+ const PostingList L5;
+
+ auto DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+
+ auto Root = createAnd(createAnd(create(L1), create(L2)),
+createOr(create(L3), create(L4), create(L5)));
+
+ EXPECT_THAT(consume(*Root, 42), ElementsAre(1, 5));
+}
+
testing::Matcher>
trigramsAre(std::initializer_list Trigrams) {
std::vector Tokens;
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,9 +101,10 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator ) {
+std::vector consume(Iterator , size_t Limit) {
std::vector Result;
- for (; !It.reachedEnd(); It.advance())
+ for (size_t Retrieved = 0; !It.reachedEnd() && Retrieved < Limit;
+ It.advance(), ++Retrieved)
Result.push_back(It.peek());
return Result;
}
Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -240,6 +240,29 @@
"(& (& [1, 3, 5, 8, 9] [1, 5, 7, 9]) (| [0, 5] [0, 1, 5] []))");
}
+TEST(DexIndexIterators, Limit) {
+ const PostingList L0 = {4, 7, 8, 20, 42, 100};
+ const PostingList L1 = {1, 3, 5, 8, 9};
+ const PostingList L2 = {1, 5, 7, 9};
+ const PostingList L3 = {0, 5};
+ const PostingList L4 = {0, 1, 5};
+ const PostingList L5;
+
+ auto DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 42), ElementsAre(4, 7, 8, 20, 42, 100));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 3), ElementsAre(4, 7, 8));
+
+ DocIterator = create(L0);
+ EXPECT_THAT(consume(*DocIterator, 0), ElementsAre());
+
+ auto Root = createAnd(createAnd(create(L1), create(L2)),
+createOr(create(L3), create(L4), create(L5)));
+
+ EXPECT_THAT(consume(*Root, 42), ElementsAre(1, 5));
+}
+
testing::Matcher>
trigramsAre(std::initializer_list Trigrams) {
std::vector Tokens;
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,9 +101,10 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
[PATCH] D50500: [clangd] Allow consuming limited number of items
kbobyrev planned changes to this revision. kbobyrev added a comment. Oops, I thought I pushed "Plan Changes" for this one. https://reviews.llvm.org/D50500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50500: [clangd] Allow consuming limited number of items
ioeric added a comment. Could you add test? ;) Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:223 std::vector Result; - for (; !It.reachedEnd(); It.advance()) + for (size_t Retreived = 0; !It.reachedEnd() && Retreived < Limit; + It.advance()) Where do we increment `Retrieved`? Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:223 std::vector Result; - for (; !It.reachedEnd(); It.advance()) + for (size_t Retreived = 0; !It.reachedEnd() && Retreived < Limit; + It.advance()) ioeric wrote: > Where do we increment `Retrieved`? nit: `s/Retreived/Retrieved/` Comment at: clang-tools-extra/clangd/index/dex/Iterator.h:108 +/// for the number of elements obtained before the iterator is exhausted. +std::vector consume(Iterator , + size_t Limit = std::numeric_limits::max()); nit: I think `Size of the returned ...` is just repeating the first two sentences. Maybe drop? https://reviews.llvm.org/D50500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D50500: [clangd] Allow consuming limited number of items
kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: arphaman, jkorous, MaskRay.
This patch modifies `consume` function to allow retrieval of limited number of
symbols. This is the "cheap" implementation of top-level limiting iterator. In
the future we would like to have a complete limit iterator implementation to
insert it into the query subtrees, but in the meantime this version would be
enough for a fully-functional proof-of-concept Dex implementation.
https://reviews.llvm.org/D50500
Files:
clang-tools-extra/clangd/index/dex/Iterator.cpp
clang-tools-extra/clangd/index/dex/Iterator.h
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,9 +101,12 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs. Size of
+/// the returned vector is min(Limit, IteratorSize) where IteratorSize stands
+/// for the number of elements obtained before the iterator is exhausted.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator ) {
+std::vector consume(Iterator , size_t Limit) {
std::vector Result;
- for (; !It.reachedEnd(); It.advance())
+ for (size_t Retreived = 0; !It.reachedEnd() && Retreived < Limit;
+ It.advance())
Result.push_back(It.peek());
return Result;
}
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -101,9 +101,12 @@
virtual llvm::raw_ostream (llvm::raw_ostream ) const = 0;
};
-/// Exhausts given iterator and returns all processed DocIDs. The result
-/// contains sorted DocumentIDs.
-std::vector consume(Iterator );
+/// Advances the iterator until it is either exhausted or the number of
+/// requested items is reached. The result contains sorted DocumentIDs. Size of
+/// the returned vector is min(Limit, IteratorSize) where IteratorSize stands
+/// for the number of elements obtained before the iterator is exhausted.
+std::vector consume(Iterator ,
+ size_t Limit = std::numeric_limits::max());
/// Returns a document iterator over given PostingList.
std::unique_ptr create(PostingListRef Documents);
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -218,9 +218,10 @@
} // end namespace
-std::vector consume(Iterator ) {
+std::vector consume(Iterator , size_t Limit) {
std::vector Result;
- for (; !It.reachedEnd(); It.advance())
+ for (size_t Retreived = 0; !It.reachedEnd() && Retreived < Limit;
+ It.advance())
Result.push_back(It.peek());
return Result;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
