[clang-tools-extra] r340157 - [clangd] NFC: Cleanup Dex Iterator comments and simplify tests
Kirill Bobyrev via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 20 02:16:14 PDT 2018
Author: omtcyfz
Date: Mon Aug 20 02:16:14 2018
New Revision: 340157
URL: http://llvm.org/viewvc/llvm-project?rev=340157&view=rev
Log:
[clangd] NFC: Cleanup Dex Iterator comments and simplify tests
Proposed changes:
* Cleanup comments in `clangd/index/dex/Iterator.h`: Vim's `gq`
formatting added redundant spaces instead of newlines in few
places
* Few comments in `OrIterator` are wrong
* Use `EXPECT_TRUE(Condition)` instead of
`EXPECT_THAT(Condition, true)` (same with `EXPECT_FALSE`)
* Don't expose `dump()` method to the public by misplacing
`private:`
This patch does not affect functionality.
Reviewed by: ioeric
Differential Revision: https://reviews.llvm.org/D50956
Modified:
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
Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp?rev=340157&r1=340156&r2=340157&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Mon Aug 20 02:16:14 2018
@@ -46,6 +46,7 @@ public:
return *Index;
}
+private:
llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
OS << '[';
auto Separator = "";
@@ -66,7 +67,6 @@ public:
return OS;
}
-private:
PostingListRef Documents;
PostingListRef::const_iterator Index;
};
@@ -103,6 +103,7 @@ public:
DocID peek() const override { return Children.front()->peek(); }
+private:
llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
OS << "(& ";
auto Separator = "";
@@ -114,7 +115,6 @@ public:
return OS;
}
-private:
/// Restores class invariants: each child will point to the same element after
/// sync.
void sync() {
@@ -180,7 +180,7 @@ public:
/// Moves each child pointing to the smallest DocID to the next item.
void advance() override {
assert(!reachedEnd() &&
- "OrIterator must have at least one child to advance().");
+ "OrIterator can't call advance() after it reached the end.");
const auto SmallestID = peek();
for (const auto &Child : Children)
if (!Child->reachedEnd() && Child->peek() == SmallestID)
@@ -199,7 +199,7 @@ public:
/// value.
DocID peek() const override {
assert(!reachedEnd() &&
- "OrIterator must have at least one child to peek().");
+ "OrIterator can't peek() after it reached the end.");
DocID Result = std::numeric_limits<DocID>::max();
for (const auto &Child : Children)
@@ -209,6 +209,7 @@ public:
return Result;
}
+private:
llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
OS << "(| ";
auto Separator = "";
@@ -220,7 +221,6 @@ public:
return OS;
}
-private:
// FIXME(kbobyrev): Would storing Children in min-heap be faster?
std::vector<std::unique_ptr<Iterator>> Children;
};
Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.h?rev=340157&r1=340156&r2=340157&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.h (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.h Mon Aug 20 02:16:14 2018
@@ -11,14 +11,14 @@
// symbol, such as high fuzzy matching score, scope, type etc. The lists of all
// symbols matching some criteria (e.g. belonging to "clang::clangd::" scope)
// are expressed in a form of Search Tokens which are stored in the inverted
-// index. Inverted index maps these tokens to the posting lists - sorted ( by
-// symbol quality) sequences of symbol IDs matching the token, e.g. scope token
+// index. Inverted index maps these tokens to the posting lists - sorted (by
+// symbol quality) sequences of symbol IDs matching the token, e.g. scope token
// "clangd::clangd::" is mapped to the list of IDs of all symbols which are
// declared in this namespace. Search queries are build from a set of
// requirements which can be combined with each other forming the query trees.
// The leafs of such trees are posting lists, and the nodes are operations on
-// these posting lists, e.g. intersection or union. Efficient processing of
-// these multi-level queries is handled by Iterators. Iterators advance through
+// these posting lists, e.g. intersection or union. Efficient processing of
+// these multi-level queries is handled by Iterators. Iterators advance through
// all leaf posting lists producing the result of search query, which preserves
// the sorted order of IDs. Having the resulting IDs sorted is important,
// because it allows receiving a certain number of the most valuable items (e.g.
Modified: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp?rev=340157&r1=340156&r2=340157&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp Mon Aug 20 02:16:14 2018
@@ -28,22 +28,22 @@ TEST(DexIndexIterators, DocumentIterator
auto DocIterator = create(L);
EXPECT_EQ(DocIterator->peek(), 4U);
- EXPECT_EQ(DocIterator->reachedEnd(), false);
+ EXPECT_FALSE(DocIterator->reachedEnd());
DocIterator->advance();
EXPECT_EQ(DocIterator->peek(), 7U);
- EXPECT_EQ(DocIterator->reachedEnd(), false);
+ EXPECT_FALSE(DocIterator->reachedEnd());
DocIterator->advanceTo(20);
EXPECT_EQ(DocIterator->peek(), 20U);
- EXPECT_EQ(DocIterator->reachedEnd(), false);
+ EXPECT_FALSE(DocIterator->reachedEnd());
DocIterator->advanceTo(65);
EXPECT_EQ(DocIterator->peek(), 100U);
- EXPECT_EQ(DocIterator->reachedEnd(), false);
+ EXPECT_FALSE(DocIterator->reachedEnd());
DocIterator->advanceTo(420);
- EXPECT_EQ(DocIterator->reachedEnd(), true);
+ EXPECT_TRUE(DocIterator->reachedEnd());
}
TEST(DexIndexIterators, AndWithEmpty) {
@@ -51,10 +51,10 @@ TEST(DexIndexIterators, AndWithEmpty) {
const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
auto AndEmpty = createAnd(create(L0));
- EXPECT_EQ(AndEmpty->reachedEnd(), true);
+ EXPECT_TRUE(AndEmpty->reachedEnd());
auto AndWithEmpty = createAnd(create(L0), create(L1));
- EXPECT_EQ(AndWithEmpty->reachedEnd(), true);
+ EXPECT_TRUE(AndWithEmpty->reachedEnd());
EXPECT_THAT(consume(*AndWithEmpty), ElementsAre());
}
@@ -65,7 +65,7 @@ TEST(DexIndexIterators, AndTwoLists) {
auto And = createAnd(create(L1), create(L0));
- EXPECT_EQ(And->reachedEnd(), false);
+ EXPECT_FALSE(And->reachedEnd());
EXPECT_THAT(consume(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
And = createAnd(create(L0), create(L1));
@@ -94,7 +94,7 @@ TEST(DexIndexIterators, AndThreeLists) {
EXPECT_EQ(And->peek(), 320U);
And->advanceTo(100000);
- EXPECT_EQ(And->reachedEnd(), true);
+ EXPECT_TRUE(And->reachedEnd());
}
TEST(DexIndexIterators, OrWithEmpty) {
@@ -102,10 +102,10 @@ TEST(DexIndexIterators, OrWithEmpty) {
const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
auto OrEmpty = createOr(create(L0));
- EXPECT_EQ(OrEmpty->reachedEnd(), true);
+ EXPECT_TRUE(OrEmpty->reachedEnd());
auto OrWithEmpty = createOr(create(L0), create(L1));
- EXPECT_EQ(OrWithEmpty->reachedEnd(), false);
+ EXPECT_FALSE(OrWithEmpty->reachedEnd());
EXPECT_THAT(consume(*OrWithEmpty),
ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
@@ -117,7 +117,7 @@ TEST(DexIndexIterators, OrTwoLists) {
auto Or = createOr(create(L0), create(L1));
- EXPECT_EQ(Or->reachedEnd(), false);
+ EXPECT_FALSE(Or->reachedEnd());
EXPECT_EQ(Or->peek(), 0U);
Or->advance();
EXPECT_EQ(Or->peek(), 4U);
@@ -136,7 +136,7 @@ TEST(DexIndexIterators, OrTwoLists) {
Or->advanceTo(9000);
EXPECT_EQ(Or->peek(), 9000U);
Or->advanceTo(9001);
- EXPECT_EQ(Or->reachedEnd(), true);
+ EXPECT_TRUE(Or->reachedEnd());
Or = createOr(create(L0), create(L1));
@@ -151,7 +151,7 @@ TEST(DexIndexIterators, OrThreeLists) {
auto Or = createOr(create(L0), create(L1), create(L2));
- EXPECT_EQ(Or->reachedEnd(), false);
+ EXPECT_FALSE(Or->reachedEnd());
EXPECT_EQ(Or->peek(), 0U);
Or->advance();
@@ -166,7 +166,7 @@ TEST(DexIndexIterators, OrThreeLists) {
EXPECT_EQ(Or->peek(), 60U);
Or->advanceTo(9001);
- EXPECT_EQ(Or->reachedEnd(), true);
+ EXPECT_TRUE(Or->reachedEnd());
}
// FIXME(kbobyrev): The testcase below is similar to what is expected in real
@@ -208,7 +208,7 @@ TEST(DexIndexIterators, QueryTree) {
// Lower Or Iterator: [0, 1, 5]
createOr(create(L2), create(L3), create(L4)));
- EXPECT_EQ(Root->reachedEnd(), false);
+ EXPECT_FALSE(Root->reachedEnd());
EXPECT_EQ(Root->peek(), 1U);
Root->advanceTo(0);
// Advance multiple times. Shouldn't do anything.
@@ -220,7 +220,7 @@ TEST(DexIndexIterators, QueryTree) {
Root->advanceTo(5);
EXPECT_EQ(Root->peek(), 5U);
Root->advanceTo(9000);
- EXPECT_EQ(Root->reachedEnd(), true);
+ EXPECT_TRUE(Root->reachedEnd());
}
TEST(DexIndexIterators, StringRepresentation) {
@@ -264,14 +264,14 @@ TEST(DexIndexIterators, Limit) {
TEST(DexIndexIterators, True) {
auto TrueIterator = createTrue(0U);
- EXPECT_THAT(TrueIterator->reachedEnd(), true);
+ EXPECT_TRUE(TrueIterator->reachedEnd());
EXPECT_THAT(consume(*TrueIterator), ElementsAre());
PostingList L0 = {1, 2, 5, 7};
TrueIterator = createTrue(7U);
EXPECT_THAT(TrueIterator->peek(), 0);
auto AndIterator = createAnd(create(L0), move(TrueIterator));
- EXPECT_THAT(AndIterator->reachedEnd(), false);
+ EXPECT_FALSE(AndIterator->reachedEnd());
EXPECT_THAT(consume(*AndIterator), ElementsAre(1, 2, 5));
}
More information about the cfe-commits
mailing list