[clang-tools-extra] [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum (PR #77766)

via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 11 05:32:15 PST 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: Tom Praschan (tom-anders)

<details>
<summary>Changes</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.

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
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;

``````````

</details>


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


More information about the cfe-commits mailing list