tom-anders created this revision.
tom-anders added reviewers: nridge, kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D141800
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3059,6 +3059,25 @@
AllOf(qualifier(""), scope("na::"), named("ClangdA"))));
}
+// https://github.com/clangd/clangd/issues/1361
+TEST(CompletionTest, ScopedEnumUsingDecl) {
+ clangd::CodeCompleteOptions Opts = {};
+ Opts.AllScopes = true;
+
+ auto Results = completions(
+ R"cpp(
+ namespace ns { enum class Scoped { FooBar }; }
+ using ns::Scoped;
+ void f() {
+ Foo^
+ }
+ )cpp",
+ {enmConstant("ns::Scoped::FooBar")}, Opts);
+ EXPECT_THAT(Results.Completions, UnorderedElementsAre(AllOf(
+ qualifier("Scoped::"), named("FooBar"),
+ kind(CompletionItemKind::EnumMember))));
+}
+
TEST(CompletionTest, AllScopesCompletion) {
clangd::CodeCompleteOptions Opts = {};
Opts.AllScopes = true;
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -305,7 +305,8 @@
// computed from the first candidate, in the constructor.
// Others vary per candidate, so add() must be called for remaining candidates.
struct CodeCompletionBuilder {
- CodeCompletionBuilder(ASTContext *ASTCtx, const CompletionCandidate &C,
+ CodeCompletionBuilder(ASTContext *ASTCtx, DeclContext *SemaDeclCtx,
+ const CompletionCandidate &C,
CodeCompletionString *SemaCCS,
llvm::ArrayRef<std::string> AccessibleScopes,
const IncludeInserter &Includes,
@@ -360,14 +361,39 @@
// If the completion was visible to Sema, no qualifier is needed. This
// avoids unneeded qualifiers in cases like with `using ns::X`.
if (Completion.RequiredQualifier.empty() && !C.SemaResult) {
- llvm::StringRef ShortestQualifier = C.IndexResult->Scope;
- for (llvm::StringRef Scope : AccessibleScopes) {
- llvm::StringRef Qualifier = C.IndexResult->Scope;
- if (Qualifier.consume_front(Scope) &&
- Qualifier.size() < ShortestQualifier.size())
- ShortestQualifier = Qualifier;
+ // With all-scopes-completion, we can complete enum constants of scoped
+ // enums, in which case the completion might not be visible to Sema.
+ // So, if there's a using declaration for the enum class, manually
+ // drop the qualifiers.
+ if (C.IndexResult->SymInfo.Kind == index::SymbolKind::EnumConstant &&
+ SemaDeclCtx && ASTCtx) {
+ for (auto *Ctx = SemaDeclCtx; Ctx; Ctx = Ctx->getParent()) {
+ auto MatchingUsingDecl = llvm::find_if(Ctx->decls(), [&C](Decl *D) {
+ if (const auto *UD = dyn_cast<UsingShadowDecl>(D))
+ if (const auto *ED = dyn_cast<EnumDecl>(UD->getTargetDecl()))
+ if (ED->isScoped() &&
+ printQualifiedName(*ED) + "::" == C.IndexResult->Scope)
+ return true;
+ return false;
+ });
+ if (MatchingUsingDecl != Ctx->decls_end()) {
+ assert(MatchingUsingDecl->isNamed());
+ Completion.RequiredQualifier =
+ dyn_cast<NamedDecl>(*MatchingUsingDecl)->getName().str() +
+ "::";
+ break;
+ }
+ }
+ } else {
+ llvm::StringRef ShortestQualifier = C.IndexResult->Scope;
+ for (llvm::StringRef Scope : AccessibleScopes) {
+ llvm::StringRef Qualifier = C.IndexResult->Scope;
+ if (Qualifier.consume_front(Scope) &&
+ Qualifier.size() < ShortestQualifier.size())
+ ShortestQualifier = Qualifier;
+ }
+ Completion.RequiredQualifier = std::string(ShortestQualifier);
}
- Completion.RequiredQualifier = std::string(ShortestQualifier);
}
}
if (C.IdentifierResult) {
@@ -1983,7 +2009,8 @@
: nullptr;
if (!Builder)
Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : nullptr,
- Item, SemaCCS, AccessibleScopes, *Inserter, FileName,
+ Recorder ? Recorder->CCSema->CurContext : nullptr, Item,
+ SemaCCS, AccessibleScopes, *Inserter, FileName,
CCContextKind, Opts, IsUsingDeclaration, NextTokenKind);
else
Builder->add(Item, SemaCCS);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
