kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674
+ else if (const auto *ND = dyn_cast<NamespaceDecl>(Context)) {
+ if (ND->isInlineNamespace())
+ Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");
----------------
tom-anders wrote:
> kadircet wrote:
> > since we know that the `Context` is a `NamespaceDecl` it should be safe to
> > use `printQualifiedName` always. any reason for the extra branching here
> > (apart from minimizing the change to behaviour)? if not I think we can get
> > rid of the special casing.
> Unfortunately, this fails CompletionTest.EnclosingScopeComesFirst and
> CompletionTest.NoDuplicatedQueryScopes, I think because of anonymous
> namespaces (where printNameSpaceScope would return an empty string, but
> (printQualifiedName(*ND) + "::" does not).
i see. taking a closer look at this `getQueryScopes` is used for two things:
- The scopes to query with fuzzyfind requests, hence this should use the same
"serialization" as symbolcollector (which only strips anon namespaces today,
but initially it were to strip both anon & inline namespaces. it got changed
inside clang without clangd tests catching it).
- The shortening of the fully qualified name in `CodeCompletionBuilder`. Not
having inline namespaces spelled in the available namespaces implies getting
wrong qualifiers (such as the bug you're fixing).
so considering the requirements here:
- when querying index, we actually want to hide inline namespaces (as
`ns::hidden::Foo` should be a viable alternative even if only `ns::` is
accessible). so we should actually fix `printQualifiedName` to set
`SuppressInlineNamespace` in printing policy to restore the old behaviour (and
keep using `printNamespaceScope` here).
- inside `CodeCompletionBuilder`, we shouldn't use the same scopes we use
during index queries. we should use the visible namespaces while preserving
inline namespace information and only ignoring the anonymous namespaces.
hence can we have 2 separate scopes in `CodeCompleteFlow` instead?
One called `QueryScopes`, which has the behavior we have today (fixing
printQualifiedName is a separate issues).
Other called `AccessibleScopes`, which has accessible namespaces spelled
**with** inline namespaces, so that we can get proper qualification during
code-complete.
does that make sense?
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
