[PATCH] D41367: [clangd] Support filtering by fixing scopes in fuzzyFind.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

just nits, all this stuff up to you



================
Comment at: clangd/index/Index.h:131
+  /// un-qualified identifiers and should not contain qualifiers like "::". If
+  /// any scope below is provided, \p Query is only matched against symbols in
+  /// the scope (excl. symbols in nested scopes).
----------------
I think the rest of the comment isn't needed now - the first two sentences are enough (plus the comment on Scopes)


================
Comment at: clangd/index/MemIndex.cpp:40
     for (const auto Pair : Index) {
+      if (Matched >= Req.MaxCandidateCount)
+        return false;
----------------
This is too early - we may not actually have an N+1th match :-(


================
Comment at: clangd/index/SymbolCollector.cpp:63
+std::pair<llvm::StringRef, llvm::StringRef>
+splitQualifiedName(llvm::StringRef QName) {
+  size_t Pos = QName.rfind("::");
----------------
If you'll never have leading ::, assert that.
Otherwise strip it (I think an assert is better unless you actually expect it)


================
Comment at: unittests/clangd/FileIndexTests.cpp:127
+  Req.Query = "";
+  Req.Scopes.push_back("ns");
   EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X"));
----------------
nit: i usually find `Req.Scopes = {"ns"}` more clear, up to you


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41367