[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

llvm-svn: 340157

diff --git a/clang-tools-extra/clangd/index/dex/Iterator.cpp b/clang-tools-extra/clangd/index/dex/Iterator.cpp
index f967ec4adb1f51ce12ffa283922b53d77b78a7dc..81f0a205e925d0b534812733f32a5cad93db426e 100644 (file)
--- a/clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ b/clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -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;
 };

diff --git a/clang-tools-extra/clangd/index/dex/Iterator.h b/clang-tools-extra/clangd/index/dex/Iterator.h
index 4773022ab24ba7c54b7a257db3fcbdc7af3b9bdd..cd36499a573d68ec202316f40757acf0b384f9d8 100644 (file)
--- a/clang-tools-extra/clangd/index/dex/Iterator.h
+++ b/clang-tools-extra/clangd/index/dex/Iterator.h
@@ -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.

diff --git a/clang-tools-extra/unittests/clangd/DexIndexTests.cpp b/clang-tools-extra/unittests/clangd/DexIndexTests.cpp
index 6a98f307269ac2c40f7064439ea0cb73ce3af91a..8cc2e7d129dce814bc42b8c275c82f3c2030a6b2 100644 (file)
--- a/clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ b/clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -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));
 }