Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clangd] Fix qualifier not being dropped for using declaration referring to scoped enum #77766

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tom-anders
Copy link
Contributor

Relevant issue: clangd/clangd#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.

Differential Revision: https://reviews.llvm.org/D141800

…ing to scoped enum

Relevant issue: clangd/clangd#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.

Differential Revision: https://reviews.llvm.org/D141800
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/pr-subscribers-clangd

Author: Tom Praschan (tom-anders)

Changes

Relevant issue: clangd/clangd#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.

Differential Revision: https://reviews.llvm.org/D141800


Full diff: https://github.com/llvm/llvm-project/pull/77766.diff

2 Files Affected:

  • (modified) clang-tools-extra/clangd/CodeComplete.cpp (+30-2)
  • (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+19)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..0345c014982134 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -333,7 +333,8 @@ std::string removeFirstTemplateArg(llvm::StringRef Signature) {
 // 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,
@@ -395,6 +396,8 @@ struct CodeCompletionBuilder {
             ShortestQualifier = Qualifier;
         }
         Completion.RequiredQualifier = std::string(ShortestQualifier);
+
+        stripNamespaceForEnumConstantIfUsingDecl(*C.IndexResult, SemaDeclCtx);
       }
     }
     if (C.IdentifierResult) {
@@ -452,6 +455,30 @@ struct CodeCompletionBuilder {
                           });
   }
 
+  // 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.
+  void stripNamespaceForEnumConstantIfUsingDecl(const Symbol &IndexResult,
+                                                DeclContext *SemaDeclCtx) {
+    if (IndexResult.SymInfo.Kind != index::SymbolKind::EnumConstant)
+      return;
+    for (auto *Ctx = SemaDeclCtx; Ctx; Ctx = Ctx->getParent())
+      for (auto *D : Ctx->decls()) {
+        const auto *UD = dyn_cast<UsingShadowDecl>(D);
+        if (!UD)
+          continue;
+        const auto *ED = dyn_cast<EnumDecl>(UD->getTargetDecl());
+        if (!ED || !ED->isScoped())
+          continue;
+        auto EnumName = printQualifiedName(*ED) + "::";
+        if (EnumName == IndexResult.Scope) {
+          Completion.RequiredQualifier = ED->getName().str() + "::";
+          return;
+        }
+      }
+  }
+
   void add(const CompletionCandidate &C, CodeCompletionString *SemaCCS,
            CodeCompletionContext::Kind ContextKind) {
     assert(bool(C.SemaResult) == bool(SemaCCS));
@@ -2086,7 +2113,8 @@ class CodeCompleteFlow {
                           : 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, CCContextKind);
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 6d387fec9b3851..0c3eed7c35e578 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3133,6 +3133,25 @@ TEST(CompletionTest, NoAllScopesCompletionWhenQualified) {
                   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;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants