[PATCH] D56597: [clangd] Add Limit parameter for xref.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: unittests/clangd/DexTests.cpp:673
 
-  std::vector<std::string> Files;
-  RefsRequest Req;
-  Req.IDs.insert(Foo.ID);
-  Req.Filter = RefKind::Declaration | RefKind::Definition;
-  Dex(std::vector<Symbol>{Foo, Bar}, Refs).refs(Req, [&](const Ref &R) {
-    Files.push_back(R.Location.FileURI);
-  });
-
-  EXPECT_THAT(Files, ElementsAre("foo.h"));
+  {
+    std::vector<std::string> Files;
----------------
sammccall wrote:
> If you want these to be the same test, there should be some connection between them.
> A natural one is that it's the same query with a different limit - you could extend the original test to have multiple results.
Done, made the query request the same in this test except the limit.


================
Comment at: unittests/clangd/DexTests.cpp:686
+    Req.IDs.insert(Foo.ID);
+    Req.Limit = 1;
+    size_t RefCount = 0;
----------------
sammccall wrote:
> As far as I can tell, this limit makes no difference, there's only one result anyway.
Actually, the filter of the request was different than the case above, we would get 2 refs. Used the same request to avoid confusion.


================
Comment at: unittests/clangd/IndexTests.cpp:306
+  {
+    Request.Limit = 1;
+    size_t RefsCount = 0;
----------------
sammccall wrote:
> why new scope here?
To align with the case above. 


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56597/new/

https://reviews.llvm.org/D56597