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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
