[clangd] Add bool return type to Index::refs API.

hokein · hokein · commit 33e882d5ada0 · 2019-11-13T14:42:30.000+01:00
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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
     }
   };
 

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,7 @@ void SwapIndex::lookup(const LookupRequest &R,`
`65`	`65`	`llvm::function_ref<void(const Symbol &)> CB) const {`
`66`	`66`	`return snapshot()->lookup(R, CB);`
`67`	`67`	`}`
`68`		`-void SwapIndex::refs(const RefsRequest &R,`
	`68`	`+bool SwapIndex::refs(const RefsRequest &R,`
`69`	`69`	`llvm::function_ref<void(const Ref &)> CB) const {`
`70`	`70`	`return snapshot()->refs(R, CB);`
`71`	`71`	`}`
Original file line number	Diff line number	Diff line change
`@@ -67,22 +67,30 @@ void MemIndex::lookup(const LookupRequest &Req,`
`67`	`67`	`}`
`68`	`68`	`}`
`69`	`69`
`70`		`-void MemIndex::refs(const RefsRequest &Req,`
	`70`	`+bool MemIndex::refs(const RefsRequest &Req,`
`71`	`71`	`llvm::function_ref<void(const Ref &)> Callback) const {`
`72`	`72`	`trace::Span Tracer("MemIndex refs");`
`73`	`73`	`uint32_t Remaining =`
`74`	`74`	`Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());`
	`75`	`+ bool More = false;`
`75`	`76`	`for (const auto &ReqID : Req.IDs) {`
`76`	`77`	`auto SymRefs = Refs.find(ReqID);`
`77`	`78`	`if (SymRefs == Refs.end())`
`78`	`79`	`continue;`
`79`	`80`	`for (const auto &O : SymRefs->second) {`
`80`		`- if (Remaining > 0 && static_cast<int>(Req.Filter & O.Kind)) {`
	`81`	`+ if (!static_cast<int>(Req.Filter & O.Kind))`
	`82`	`+ continue;`
	`83`	`+ if (Remaining == 0) {`
	`84`	`+ More = true;`
	`85`	`+ break;`
	`86`	`+ }`
	`87`	`+ if (Remaining > 0) {`
`81`	`88`	`--Remaining;`
`82`	`89`	`Callback(O);`
`83`	`90`	`}`
`84`	`91`	`}`
`85`	`92`	`}`
	`93`	`+ return More;`
`86`	`94`	`}`
`87`	`95`
`88`	`96`	`void MemIndex::relations(`
Original file line number	Diff line number	Diff line change
`@@ -249,18 +249,26 @@ void Dex::lookup(const LookupRequest &Req,`
`249`	`249`	`}`
`250`	`250`	`}`
`251`	`251`
`252`		`-void Dex::refs(const RefsRequest &Req,`
	`252`	`+bool Dex::refs(const RefsRequest &Req,`
`253`	`253`	`llvm::function_ref<void(const Ref &)> Callback) const {`
`254`	`254`	`trace::Span Tracer("Dex refs");`
`255`	`255`	`uint32_t Remaining =`
`256`	`256`	`Req.Limit.getValueOr(std::numeric_limits<uint32_t>::max());`
	`257`	`+ bool More = false;`
`257`	`258`	`for (const auto &ID : Req.IDs)`
`258`	`259`	`for (const auto &Ref : Refs.lookup(ID)) {`
`259`		`- if (Remaining > 0 && static_cast<int>(Req.Filter & Ref.Kind)) {`
	`260`	`+ if (!static_cast<int>(Req.Filter & Ref.Kind))`
	`261`	`+ continue;`
	`262`	`+ if (Remaining == 0) {`
	`263`	`+ More = true;`
	`264`	`+ break;`
	`265`	`+ }`
	`266`	`+ if (Remaining > 0) {`
`260`	`267`	`--Remaining;`
`261`	`268`	`Callback(Ref);`
`262`	`269`	`}`
`263`	`270`	`}`
	`271`	`+ return More;`
`264`	`272`	`}`
`265`	`273`
`266`	`274`	`void Dex::relations(`