Author: Haojian Wu Date: 2019-11-13T14:42:30+01:00 New Revision: 33e882d5ada0b42323be3277816b0817b8e6baa2
URL: https://github.com/llvm/llvm-project/commit/33e882d5ada0b42323be3277816b0817b8e6baa2 DIFF: https://github.com/llvm/llvm-project/commit/33e882d5ada0b42323be3277816b0817b8e6baa2.diff LOG: [clangd] Add bool return type to Index::refs API. Summary: Similar to fuzzyFind, the bool indicates whether there are more xref results. Reviewers: ilya-biryukov Reviewed By: ilya-biryukov Subscribers: merge_guards_bot, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D70139 Added: Modified: clang-tools-extra/clangd/index/Index.cpp clang-tools-extra/clangd/index/Index.h clang-tools-extra/clangd/index/MemIndex.cpp clang-tools-extra/clangd/index/MemIndex.h clang-tools-extra/clangd/index/Merge.cpp clang-tools-extra/clangd/index/Merge.h clang-tools-extra/clangd/index/dex/Dex.cpp clang-tools-extra/clangd/index/dex/Dex.h clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp clang-tools-extra/clangd/unittests/DexTests.cpp clang-tools-extra/clangd/unittests/IndexTests.cpp clang-tools-extra/clangd/unittests/XRefsTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/index/Index.cpp b/clang-tools-extra/clangd/index/Index.cpp index 9b411e00e2ab..9ec4908bbbfc 100644 --- a/clang-tools-extra/clangd/index/Index.cpp +++ b/clang-tools-extra/clangd/index/Index.cpp @@ -65,7 +65,7 @@ void SwapIndex::lookup(const LookupRequest &R, llvm::function_ref<void(const Symbol &)> CB) const { return snapshot()->lookup(R, CB); } -void SwapIndex::refs(const RefsRequest &R, +bool SwapIndex::refs(const RefsRequest &R, llvm::function_ref<void(const Ref &)> CB) const { return snapshot()->refs(R, CB); } diff --git a/clang-tools-extra/clangd/index/Index.h b/clang-tools-extra/clangd/index/Index.h index f579a754b13d..1eb116368f14 100644 --- a/clang-tools-extra/clangd/index/Index.h +++ b/clang-tools-extra/clangd/index/Index.h @@ -107,7 +107,9 @@ class SymbolIndex { /// /// Results should be returned in arbitrary order. /// The returned result must be deep-copied if it's used outside Callback. - virtual void refs(const RefsRequest &Req, + /// + /// Returns true if there will be more results (limited by Req.Limit); + virtual bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const = 0; /// Finds all relations (S, P, O) stored in the index such that S is among @@ -136,7 +138,7 @@ class SwapIndex : public SymbolIndex { llvm::function_ref<void(const Symbol &)>) const override; void lookup(const LookupRequest &, llvm::function_ref<void(const Symbol &)>) const override; - void refs(const RefsRequest &, + bool refs(const RefsRequest &, llvm::function_ref<void(const Ref &)>) const override; void relations(const RelationsRequest &, llvm::function_ref<void(const SymbolID &, const Symbol &)>) diff --git a/clang-tools-extra/clangd/index/MemIndex.cpp b/clang-tools-extra/clangd/index/MemIndex.cpp index 1d46dbd64da8..ea691b46cf37 100644 --- a/clang-tools-extra/clangd/index/MemIndex.cpp +++ b/clang-tools-extra/clangd/index/MemIndex.cpp @@ -67,22 +67,30 @@ void MemIndex::lookup(const LookupRequest &Req, } } -void MemIndex::refs(const RefsRequest &Req, +bool MemIndex::refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const { trace::Span Tracer("MemIndex refs"); uint32_t Remaining = Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max()); + bool More = false; for (const auto &ReqID : Req.IDs) { auto SymRefs = Refs.find(ReqID); if (SymRefs == Refs.end()) continue; for (const auto &O : SymRefs->second) { - if (Remaining > 0 && static_cast<int>(Req.Filter & O.Kind)) { + if (!static_cast<int>(Req.Filter & O.Kind)) + continue; + if (Remaining == 0) { + More = true; + break; + } + if (Remaining > 0) { --Remaining; Callback(O); } } } + return More; } void MemIndex::relations( diff --git a/clang-tools-extra/clangd/index/MemIndex.h b/clang-tools-extra/clangd/index/MemIndex.h index ad74f5a0ecad..10ecd79e7c54 100644 --- a/clang-tools-extra/clangd/index/MemIndex.h +++ b/clang-tools-extra/clangd/index/MemIndex.h @@ -55,7 +55,7 @@ class MemIndex : public SymbolIndex { void lookup(const LookupRequest &Req, llvm::function_ref<void(const Symbol &)> Callback) const override; - void refs(const RefsRequest &Req, + bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const override; void relations(const RelationsRequest &Req, diff --git a/clang-tools-extra/clangd/index/Merge.cpp b/clang-tools-extra/clangd/index/Merge.cpp index 0c9d0c8d3b12..8d8150a13136 100644 --- a/clang-tools-extra/clangd/index/Merge.cpp +++ b/clang-tools-extra/clangd/index/Merge.cpp @@ -89,9 +89,10 @@ void MergedIndex::lookup( Callback(*Sym); } -void MergedIndex::refs(const RefsRequest &Req, +bool MergedIndex::refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const { trace::Span Tracer("MergedIndex refs"); + bool More = false; uint32_t Remaining = Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max()); // We don't want duplicated refs from the static/dynamic indexes, @@ -103,21 +104,26 @@ void MergedIndex::refs(const RefsRequest &Req, // refs were removed (we will report stale ones from the static index). // Ultimately we should explicit check which index has the file instead. llvm::StringSet<> DynamicIndexFileURIs; - Dynamic->refs(Req, [&](const Ref &O) { + More |= Dynamic->refs(Req, [&](const Ref &O) { DynamicIndexFileURIs.insert(O.Location.FileURI); Callback(O); --Remaining; }); - if (Remaining == 0) - return; + if (Remaining == 0 && More) + return More; // We return less than Req.Limit if static index returns more refs for dirty // files. - Static->refs(Req, [&](const Ref &O) { - if (Remaining > 0 && !DynamicIndexFileURIs.count(O.Location.FileURI)) { + More |= Static->refs(Req, [&](const Ref &O) { + if (DynamicIndexFileURIs.count(O.Location.FileURI)) + return; // ignore refs that have been seen from dynamic index. + if (Remaining == 0) + More = true; + if (Remaining > 0) { --Remaining; Callback(O); } }); + return More; } void MergedIndex::relations( diff --git a/clang-tools-extra/clangd/index/Merge.h b/clang-tools-extra/clangd/index/Merge.h index b41d4bcf334f..3288aef120fc 100644 --- a/clang-tools-extra/clangd/index/Merge.h +++ b/clang-tools-extra/clangd/index/Merge.h @@ -40,7 +40,7 @@ class MergedIndex : public SymbolIndex { llvm::function_ref<void(const Symbol &)>) const override; void lookup(const LookupRequest &, llvm::function_ref<void(const Symbol &)>) const override; - void refs(const RefsRequest &, + bool refs(const RefsRequest &, llvm::function_ref<void(const Ref &)>) const override; void relations(const RelationsRequest &, llvm::function_ref<void(const SymbolID &, const Symbol &)>) diff --git a/clang-tools-extra/clangd/index/dex/Dex.cpp b/clang-tools-extra/clangd/index/dex/Dex.cpp index b39f17b51f68..3b4ed71da202 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.cpp +++ b/clang-tools-extra/clangd/index/dex/Dex.cpp @@ -249,18 +249,26 @@ void Dex::lookup(const LookupRequest &Req, } } -void Dex::refs(const RefsRequest &Req, +bool Dex::refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const { trace::Span Tracer("Dex refs"); uint32_t Remaining = Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max()); + bool More = false; for (const auto &ID : Req.IDs) for (const auto &Ref : Refs.lookup(ID)) { - if (Remaining > 0 && static_cast<int>(Req.Filter & Ref.Kind)) { + if (!static_cast<int>(Req.Filter & Ref.Kind)) + continue; + if (Remaining == 0) { + More = true; + break; + } + if (Remaining > 0) { --Remaining; Callback(Ref); } } + return More; } void Dex::relations( diff --git a/clang-tools-extra/clangd/index/dex/Dex.h b/clang-tools-extra/clangd/index/dex/Dex.h index 9e7b987784b0..3a66031a89d0 100644 --- a/clang-tools-extra/clangd/index/dex/Dex.h +++ b/clang-tools-extra/clangd/index/dex/Dex.h @@ -77,7 +77,7 @@ class Dex : public SymbolIndex { void lookup(const LookupRequest &Req, llvm::function_ref<void(const Symbol &)> Callback) const override; - void refs(const RefsRequest &Req, + bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)> Callback) const override; void relations(const RelationsRequest &Req, diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 02d7750607a4..5b50b9fe9f8b 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -1193,8 +1193,10 @@ class IndexRequestCollector : public SymbolIndex { void lookup(const LookupRequest &, llvm::function_ref<void(const Symbol &)>) const override {} - void refs(const RefsRequest &, - llvm::function_ref<void(const Ref &)>) const override {} + bool refs(const RefsRequest &, + llvm::function_ref<void(const Ref &)>) const override { + return false; + } void relations(const RelationsRequest &, llvm::function_ref<void(const SymbolID &, const Symbol &)>) diff --git a/clang-tools-extra/clangd/unittests/DexTests.cpp b/clang-tools-extra/clangd/unittests/DexTests.cpp index 7e4fde9a234f..9402c3c0bf4b 100644 --- a/clang-tools-extra/clangd/unittests/DexTests.cpp +++ b/clang-tools-extra/clangd/unittests/DexTests.cpp @@ -687,14 +687,18 @@ TEST(DexTests, Refs) { Req.Filter = RefKind::Declaration | RefKind::Definition; std::vector<std::string> Files; - Dex(std::vector<Symbol>{Foo, Bar}, Refs, RelationSlab()) - .refs(Req, [&](const Ref &R) { Files.push_back(R.Location.FileURI); }); + EXPECT_FALSE(Dex(std::vector<Symbol>{Foo, Bar}, Refs, RelationSlab()) + .refs(Req, [&](const Ref &R) { + Files.push_back(R.Location.FileURI); + })); EXPECT_THAT(Files, UnorderedElementsAre("foo.h", "foo.cc")); Req.Limit = 1; Files.clear(); - Dex(std::vector<Symbol>{Foo, Bar}, Refs, RelationSlab()) - .refs(Req, [&](const Ref &R) { Files.push_back(R.Location.FileURI); }); + EXPECT_TRUE(Dex(std::vector<Symbol>{Foo, Bar}, Refs, RelationSlab()) + .refs(Req, [&](const Ref &R) { + Files.push_back(R.Location.FileURI); + })); EXPECT_THAT(Files, ElementsAre(AnyOf("foo.h", "foo.cc"))); } diff --git a/clang-tools-extra/clangd/unittests/IndexTests.cpp b/clang-tools-extra/clangd/unittests/IndexTests.cpp index e868828d7868..dcbfc4cf5ea0 100644 --- a/clang-tools-extra/clangd/unittests/IndexTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexTests.cpp @@ -389,7 +389,8 @@ TEST(MergeIndexTest, Refs) { RefsRequest Request; Request.IDs = {Foo.ID}; RefSlab::Builder Results; - Merge.refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O); }); + EXPECT_FALSE( + Merge.refs(Request, [&](const Ref &O) { Results.insert(Foo.ID, O); })); EXPECT_THAT( std::move(Results).build(), ElementsAre(Pair( @@ -400,7 +401,8 @@ TEST(MergeIndexTest, Refs) { Request.Limit = 1; RefSlab::Builder Results2; - Merge.refs(Request, [&](const Ref &O) { Results2.insert(Foo.ID, O); }); + EXPECT_TRUE( + Merge.refs(Request, [&](const Ref &O) { Results2.insert(Foo.ID, O); })); EXPECT_THAT(std::move(Results2).build(), ElementsAre(Pair( _, ElementsAre(AnyOf(FileURI("unittest:///test.cc"), diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp index 9330c6d366dd..9323dd48473a 100644 --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -2161,9 +2161,10 @@ TEST(FindReferences, NeedsIndex) { TEST(FindReferences, NoQueryForLocalSymbols) { struct RecordingIndex : public MemIndex { mutable Optional<llvm::DenseSet<SymbolID>> RefIDs; - void refs(const RefsRequest &Req, + bool refs(const RefsRequest &Req, llvm::function_ref<void(const Ref &)>) const override { RefIDs = Req.IDs; + return false; } }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits