From 493b1627ca6de748a63c25dfaa8fbdf9b5688271 Mon Sep 17 00:00:00 2001 From: Kirill Bobyrev Date: Fri, 31 Aug 2018 09:17:02 +0000 Subject: [PATCH] [NFC] Cleanup Dex * Use consistent assertion messages in iterators implementations * Silence a bunch of clang-tidy warnings: use `emplace_back` instead of `push_back` where possible, make sure arguments have the same name in header and implementation file, use for loop over ranges where possible Reviewed by: ioeric Differential Revision: https://reviews.llvm.org/D51528 llvm-svn: 341190 --- clang-tools-extra/clangd/index/dex/DexIndex.cpp | 2 +- clang-tools-extra/clangd/index/dex/DexIndex.h | 2 +- clang-tools-extra/clangd/index/dex/Iterator.cpp | 51 +++++++++++++------------ clang-tools-extra/clangd/index/dex/Trigram.cpp | 8 ++-- 4 files changed, 33 insertions(+), 30 deletions(-) diff --git a/clang-tools-extra/clangd/index/dex/DexIndex.cpp b/clang-tools-extra/clangd/index/dex/DexIndex.cpp index dd993a9..292ed82 100644 --- a/clang-tools-extra/clangd/index/dex/DexIndex.cpp +++ b/clang-tools-extra/clangd/index/dex/DexIndex.cpp @@ -30,7 +30,7 @@ namespace { // * Types std::vector generateSearchTokens(const Symbol &Sym) { std::vector Result = generateIdentifierTrigrams(Sym.Name); - Result.push_back(Token(Token::Kind::Scope, Sym.Scope)); + Result.emplace_back(Token::Kind::Scope, Sym.Scope); return Result; } diff --git a/clang-tools-extra/clangd/index/dex/DexIndex.h b/clang-tools-extra/clangd/index/dex/DexIndex.h index d9b2cd1..8631a23 100644 --- a/clang-tools-extra/clangd/index/dex/DexIndex.h +++ b/clang-tools-extra/clangd/index/dex/DexIndex.h @@ -41,7 +41,7 @@ class DexIndex : public SymbolIndex { public: /// \brief (Re-)Build index for `Symbols`. All symbol pointers must remain /// accessible as long as `Symbols` is kept alive. - void build(std::shared_ptr> Symbols); + void build(std::shared_ptr> Syms); /// \brief Build index from a symbol slab. static std::unique_ptr build(SymbolSlab Slab); diff --git a/clang-tools-extra/clangd/index/dex/Iterator.cpp b/clang-tools-extra/clangd/index/dex/Iterator.cpp index bf4274a..9e4606a 100644 --- a/clang-tools-extra/clangd/index/dex/Iterator.cpp +++ b/clang-tools-extra/clangd/index/dex/Iterator.cpp @@ -30,23 +30,26 @@ public: /// Advances cursor to the next item. void advance() override { - assert(!reachedEnd() && "DocumentIterator can't advance at the end."); + assert(!reachedEnd() && "DOCUMENT iterator can't advance() at the end."); ++Index; } /// Applies binary search to advance cursor to the next item with DocID equal /// or higher than the given one. void advanceTo(DocID ID) override { - assert(!reachedEnd() && "DocumentIterator can't advance at the end."); + assert(!reachedEnd() && "DOCUMENT iterator can't advance() at the end."); Index = std::lower_bound(Index, std::end(Documents), ID); } DocID peek() const override { - assert(!reachedEnd() && "DocumentIterator can't call peek() at the end."); + assert(!reachedEnd() && "DOCUMENT iterator can't peek() at the end."); return *Index; } - float consume() override { return DEFAULT_BOOST_SCORE; } + float consume() override { + assert(!reachedEnd() && "DOCUMENT iterator can't consume() at the end."); + return DEFAULT_BOOST_SCORE; + } size_t estimateSize() const override { return Documents.size(); } @@ -84,7 +87,7 @@ class AndIterator : public Iterator { public: AndIterator(std::vector> AllChildren) : Children(std::move(AllChildren)) { - assert(!Children.empty() && "AndIterator should have at least one child."); + assert(!Children.empty() && "AND iterator should have at least one child."); // Establish invariants. sync(); // When children are sorted by the estimateSize(), sync() calls are more @@ -105,14 +108,14 @@ public: /// Advances all children to the next common item. void advance() override { - assert(!reachedEnd() && "AndIterator can't call advance() at the end."); + assert(!reachedEnd() && "AND iterator can't advance() at the end."); Children.front()->advance(); sync(); } /// Advances all children to the next common item with DocumentID >= ID. void advanceTo(DocID ID) override { - assert(!reachedEnd() && "AndIterator can't call advanceTo() at the end."); + assert(!reachedEnd() && "AND iterator can't advanceTo() at the end."); Children.front()->advanceTo(ID); sync(); } @@ -120,7 +123,7 @@ public: DocID peek() const override { return Children.front()->peek(); } float consume() override { - assert(!reachedEnd() && "AndIterator can't consume() at the end."); + assert(!reachedEnd() && "AND iterator can't consume() at the end."); return std::accumulate( begin(Children), end(Children), DEFAULT_BOOST_SCORE, [&](float Current, const std::unique_ptr &Child) { @@ -192,7 +195,7 @@ class OrIterator : public Iterator { public: OrIterator(std::vector> AllChildren) : Children(std::move(AllChildren)) { - assert(Children.size() > 0 && "Or Iterator must have at least one child."); + assert(Children.size() > 0 && "OR iterator must have at least one child."); } /// Returns true if all children are exhausted. @@ -205,8 +208,7 @@ public: /// Moves each child pointing to the smallest DocID to the next item. void advance() override { - assert(!reachedEnd() && - "OrIterator can't call advance() after it reached the end."); + assert(!reachedEnd() && "OR iterator can't advance() at the end."); const auto SmallestID = peek(); for (const auto &Child : Children) if (!Child->reachedEnd() && Child->peek() == SmallestID) @@ -215,7 +217,7 @@ public: /// Advances each child to the next existing element with DocumentID >= ID. void advanceTo(DocID ID) override { - assert(!reachedEnd() && "Can't advance iterator after it reached the end."); + assert(!reachedEnd() && "OR iterator can't advanceTo() at the end."); for (const auto &Child : Children) if (!Child->reachedEnd()) Child->advanceTo(ID); @@ -224,8 +226,7 @@ public: /// Returns the element under cursor of the child with smallest Child->peek() /// value. DocID peek() const override { - assert(!reachedEnd() && - "OrIterator can't peek() after it reached the end."); + assert(!reachedEnd() && "OR iterator can't peek() at the end."); DocID Result = std::numeric_limits::max(); for (const auto &Child : Children) @@ -238,8 +239,7 @@ public: // Returns the maximum boosting score among all Children when iterator is not // exhausted and points to the given ID, DEFAULT_BOOST_SCORE otherwise. float consume() override { - assert(!reachedEnd() && - "OrIterator can't consume() after it reached the end."); + assert(!reachedEnd() && "OR iterator can't consume() at the end."); const DocID ID = peek(); return std::accumulate( begin(Children), end(Children), DEFAULT_BOOST_SCORE, @@ -284,21 +284,24 @@ public: bool reachedEnd() const override { return Index >= Size; } void advance() override { - assert(!reachedEnd() && "Can't advance iterator after it reached the end."); + assert(!reachedEnd() && "TRUE iterator can't advance() at the end."); ++Index; } void advanceTo(DocID ID) override { - assert(!reachedEnd() && "Can't advance iterator after it reached the end."); + assert(!reachedEnd() && "TRUE iterator can't advanceTo() at the end."); Index = std::min(ID, Size); } DocID peek() const override { - assert(!reachedEnd() && "TrueIterator can't call peek() at the end."); + assert(!reachedEnd() && "TRUE iterator can't peek() at the end."); return Index; } - float consume() override { return DEFAULT_BOOST_SCORE; } + float consume() override { + assert(!reachedEnd() && "TRUE iterator can't consume() at the end."); + return DEFAULT_BOOST_SCORE; + } size_t estimateSize() const override { return Size; } @@ -364,7 +367,7 @@ public: /// Decreases the limit in case the element consumed at top of the query tree /// comes from the underlying iterator. float consume() override { - assert(!reachedEnd() && "LimitIterator can't consume at the end."); + assert(!reachedEnd() && "LimitIterator can't consume() at the end."); --ItemsLeft; return Child->consume(); } @@ -389,7 +392,7 @@ private: std::vector> consume(Iterator &It) { std::vector> Result; for (; !It.reachedEnd(); It.advance()) - Result.push_back(std::make_pair(It.peek(), It.consume())); + Result.emplace_back(It.peek(), It.consume()); return Result; } @@ -417,8 +420,8 @@ std::unique_ptr createBoost(std::unique_ptr Child, } std::unique_ptr createLimit(std::unique_ptr Child, - size_t Size) { - return llvm::make_unique(move(Child), Size); + size_t Limit) { + return llvm::make_unique(move(Child), Limit); } } // namespace dex diff --git a/clang-tools-extra/clangd/index/dex/Trigram.cpp b/clang-tools-extra/clangd/index/dex/Trigram.cpp index 25a14ff..91044fe 100644 --- a/clang-tools-extra/clangd/index/dex/Trigram.cpp +++ b/clang-tools-extra/clangd/index/dex/Trigram.cpp @@ -87,10 +87,10 @@ std::vector generateIdentifierTrigrams(llvm::StringRef Identifier) { if (Roles[I] != Head && Roles[I] != Tail) continue; for (const unsigned J : Next[I]) { - if (!J) + if (J == 0) continue; for (const unsigned K : Next[J]) { - if (!K) + if (K == 0) continue; add({{LowercaseIdentifier[I], LowercaseIdentifier[J], LowercaseIdentifier[K]}}); @@ -113,8 +113,8 @@ std::vector generateQueryTrigrams(llvm::StringRef Query) { // Additional pass is necessary to count valid identifier characters. // Depending on that, this function might return incomplete trigram. unsigned ValidSymbolsCount = 0; - for (size_t I = 0; I < Roles.size(); ++I) - if (Roles[I] == Head || Roles[I] == Tail) + for (const auto Role : Roles) + if (Role == Head || Role == Tail) ++ValidSymbolsCount; std::string LowercaseQuery = Query.lower(); -- 2.7.4