kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1709
+
+ // The enclosing namespace must be first, it gets a quality boost.
+ if (auto Enclosing = SpecifiedScopes.EnclosingNamespace) {
----------------
i was actually suggesting to put this logic inside
`SpecifiedScope::scopesForIndexQuery` any reason for only including it in this
code path?
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1719
+ });
+ llvm::copy_if(SpecifiedScopes.scopesForQualification(),
+ std::back_inserter(AccessibleScopes),
----------------
`AccessibleScopes` doesn't need any particular ordering. we can use
`scopesForQualification` as-is.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:712
+ std::vector<std::string> EnclosingAtFrontForCompletion;
std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
+ EnclosingAtFrontForIndex.push_back(EnclosingScope);
----------------
tom-anders wrote:
> kadircet wrote:
> > note that this is actually going to skip inline namespaces (and you're
> > using that in the returned set)
> Hm I should probably fix this and add another regression test for this..?
yeah, let's just leave a fixme.
i also think we should be setting it in all cases, i can't think of a reason
for only setting it in a single case. i believe current scope should always get
a boost whenever it's part of query scopes.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1660
// First scope is the (modified) enclosing scope.
QueryScopes = Scopes.scopesForIndexQuery();
ScopeProximity.emplace(QueryScopes);
----------------
tom-anders wrote:
> kadircet wrote:
> > we should be setting `AccessibleScopes` too
> Ah thanks, that's what caused the one failing test. I just copied over
> QueryScopes here for now, looks like this doesn't need any special handling
> for inline namespaces, does it?
> looks like this doesn't need any special handling for inline namespaces, does
> it?
well it probably does, but there isn't much we can do. as this code path
happens with only heuristic parsing of the main-file code.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D140915/new/
https://reviews.llvm.org/D140915
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
