This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Fix qualifier not being dropped for using declaration referring to scoped enum
Needs ReviewPublic

Authored by tom-anders on Jan 15 2023, 10:54 AM.

Details

Reviewers
nridge
kadircet
Summary

Relevant issue: https://github.com/clangd/clangd/issues/1361

The problem here was that writing something like using ns::ScopedEnum
does not cause Sema to visit this scope, to it won't be added into
CodeCompletionBuilder's AccessibleScopes, leading to the qualifier not
being dropped.

To detect this, walk up the DeclContext to check if we have a matching
using declaration. If so, drop the qualifiers.

Diff Detail

Event Timeline

tom-anders created this revision.Jan 15 2023, 10:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 15 2023, 10:54 AM
Herald added a subscriber: arphaman. · View Herald Transcript
tom-anders requested review of this revision.Jan 15 2023, 10:54 AM

Factor out logic into helper function

kadircet added inline comments.Jan 18 2023, 9:04 AM
clang-tools-extra/clangd/CodeComplete.cpp
1993

instead of passing it here, can we just do the traversal as part of getQueryScopes and let these be handled uniformly by the shortest qualifier logic ?

tom-anders added inline comments.Jan 18 2023, 10:48 AM
clang-tools-extra/clangd/CodeComplete.cpp
1993

Yeah that was my initial idea, however the problem is that it looses the information about whether the enum is scoped or not:

  1. QueryScopes only stores strings
  2. The index has no flag for indicating whether an EnumConstant belongs to a scoped or unscoped enum

Thus, we get a small problem, consider the following code:

namespace ns {
enum class Scoped { Foo };
enum class Unscoped { Bar };
}

int main() {
    using ns::Scoped;
    using ns::Unscoped;

    Foo^
    Bar^
}

Here we want to complete Foo^ to ns::Scoped::Foo, but for Bar^ we want only ns::Bar. To resolve this, the scoped/unscoped information needs to be available when building the completion candidate.

Hope this was clear, maybe I missed something.

tom-anders added inline comments.Jan 18 2023, 10:50 AM
clang-tools-extra/clangd/CodeComplete.cpp
1993

Sorry, I meant the completion for Foo^ should be Scoped::Foo here, not ns::Scoped::Foo.

(friendly ping)