sammccall added a comment.
Nice, thanks!
Some suggestions for further tightening the semantics, I'm happy to do these as
a followup if you think it's too invasive here.
================
Comment at: clangd/index/Index.h:127
/// \brief A query string for the fuzzy find. This is matched against
symbols'
- /// qualfified names.
+ /// qualified names. If any scope below is provided, \p Query is only matched
+ /// against symbols in the scope (excl. symbols in nested scopes).
----------------
If i'm reading this right, you still support fuzzy-matching the scopes (scope
information can be in Query, or Scopes, or both). Why? Semantics would be
simpler if query matched the identifier only.
================
Comment at: clangd/index/Index.h:133
+ /// is provided, the matched symbols must be defined in scope "xyz" but not
+ /// "xyz::abc". "" and "::" are interpreted as the global namespace.
+ std::vector<std::string> Scopes;
----------------
nit: can we choose one?
i.e. we should mandate one of
- `{"", "foo", "foo::bar"}`
- `{"", "::foo", "::foo::bar"}`
- `{"", "foo::", "foo::bar::"}`
- `{"::", "::foo::", "::foo::bar::"}`
Allowing multiple options is somewhat confusing to callers and puts a bigger
burden on implementers of this interface.
================
Comment at: clangd/index/MemIndex.cpp:30
std::function<void(const Symbol &)> Callback) const {
- std::string LoweredQuery = llvm::StringRef(Req.Query).lower();
+ std::vector<std::string> FixedPrefixes;
+ for (StringRef Scope : Req.Scopes)
----------------
Rather than doing tricky matches against QName, it may be simpler to split
Symbol::QName into Symbol::Scope + Symbol::Name. In addition to this becoming
exact string matching without parsing, the Symbol::Scopes will have lots of
duplicates that can share storage, reducing the size of each Symbol.
WDYT?
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
