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

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 19 03:19:53 PST 2017

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

  rCTE Clang Tools Extra


More information about the cfe-commits mailing list