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] Avoid using CompletionItemKind.Text for macro completions from the index #88236

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

HighCommander4
Copy link
Collaborator

This was fixed in clangd/clangd#1484 for Sema completions but the fix did not apply to index completions.

Fixes clangd/clangd#2002

@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2024

@llvm/pr-subscribers-clangd

Author: Nathan Ridge (HighCommander4)

Changes

This was fixed in clangd/clangd#1484 for Sema completions but the fix did not apply to index completions.

Fixes clangd/clangd#2002


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

4 Files Affected:

  • (modified) clang-tools-extra/clangd/CodeComplete.cpp (+9-3)
  • (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+5-2)
  • (modified) clang-tools-extra/clangd/unittests/TestIndex.cpp (+6-1)
  • (modified) clang-tools-extra/clangd/unittests/TestIndex.h (+3-1)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..38bc2552195ef7 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -89,7 +89,9 @@ const CodeCompleteOptions::CodeCompletionRankingModel
 
 namespace {
 
-CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
+CompletionItemKind
+toCompletionItemKind(index::SymbolKind Kind,
+                     const llvm::StringRef *Signature = nullptr) {
   using SK = index::SymbolKind;
   switch (Kind) {
   case SK::Unknown:
@@ -99,7 +101,10 @@ CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
   case SK::NamespaceAlias:
     return CompletionItemKind::Module;
   case SK::Macro:
-    return CompletionItemKind::Text;
+    // Use macro signature (if provided) to tell apart function-like and
+    // object-like macros.
+    return Signature && Signature->contains('(') ? CompletionItemKind::Function
+                                                 : CompletionItemKind::Constant;
   case SK::Enum:
     return CompletionItemKind::Enum;
   case SK::Struct:
@@ -379,7 +384,8 @@ struct CodeCompletionBuilder {
       if (Completion.Scope.empty())
         Completion.Scope = std::string(C.IndexResult->Scope);
       if (Completion.Kind == CompletionItemKind::Missing)
-        Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind);
+        Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind,
+                                               &C.IndexResult->Signature);
       if (Completion.Name.empty())
         Completion.Name = std::string(C.IndexResult->Name);
       if (Completion.FilterText.empty())
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 6d387fec9b3851..abdf239a3dfb19 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -671,7 +671,8 @@ TEST(CompletionTest, Kinds) {
           #define MACRO 10
           int X = ^
       )cpp",
-      {func("indexFunction"), var("indexVariable"), cls("indexClass")});
+      {func("indexFunction"), var("indexVariable"), cls("indexClass"),
+       macro("indexObjMacro"), macro("indexFuncMacro", "(x, y)")});
   EXPECT_THAT(Results.Completions,
               AllOf(has("function", CompletionItemKind::Function),
                     has("variable", CompletionItemKind::Variable),
@@ -680,7 +681,9 @@ TEST(CompletionTest, Kinds) {
                     has("MACRO", CompletionItemKind::Constant),
                     has("indexFunction", CompletionItemKind::Function),
                     has("indexVariable", CompletionItemKind::Variable),
-                    has("indexClass", CompletionItemKind::Class)));
+                    has("indexClass", CompletionItemKind::Class),
+                    has("indexObjMacro", CompletionItemKind::Constant),
+                    has("indexFuncMacro", CompletionItemKind::Function)));
 
   Results = completions("nam^");
   EXPECT_THAT(Results.Completions,
diff --git a/clang-tools-extra/clangd/unittests/TestIndex.cpp b/clang-tools-extra/clangd/unittests/TestIndex.cpp
index 278336bdde2ee5..b13a5d32d17524 100644
--- a/clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ b/clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -38,7 +38,7 @@ static std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle,
 // Helpers to produce fake index symbols for memIndex() or completions().
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
-           llvm::StringRef USRFormat) {
+           llvm::StringRef USRFormat, llvm::StringRef Signature) {
   Symbol Sym;
   std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand!
   size_t Pos = QName.rfind("::");
@@ -55,6 +55,7 @@ Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
   Sym.SymInfo.Kind = Kind;
   Sym.Flags |= Symbol::IndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
+  Sym.Signature = Signature;
   return Sym;
 }
 
@@ -86,6 +87,10 @@ Symbol conceptSym(llvm::StringRef Name) {
   return sym(Name, index::SymbolKind::Concept, "@CT@\\0");
 }
 
+Symbol macro(llvm::StringRef Name, llvm::StringRef ArgList) {
+  return sym(Name, index::SymbolKind::Macro, "@macro@\\0", ArgList);
+}
+
 Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,
                llvm::StringRef USRPrefix) {
   Symbol Sym;
diff --git a/clang-tools-extra/clangd/unittests/TestIndex.h b/clang-tools-extra/clangd/unittests/TestIndex.h
index 9280b0b12a67fe..0699b29392d720 100644
--- a/clang-tools-extra/clangd/unittests/TestIndex.h
+++ b/clang-tools-extra/clangd/unittests/TestIndex.h
@@ -20,7 +20,7 @@ Symbol symbol(llvm::StringRef QName);
 // Helpers to produce fake index symbols with proper SymbolID.
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
-           llvm::StringRef USRFormat);
+           llvm::StringRef USRFormat, llvm::StringRef Signature = {});
 // Creats a function symbol assuming no function arg.
 Symbol func(llvm::StringRef Name);
 // Creates a class symbol.
@@ -35,6 +35,8 @@ Symbol var(llvm::StringRef Name);
 Symbol ns(llvm::StringRef Name);
 // Create a C++20 concept symbol.
 Symbol conceptSym(llvm::StringRef Name);
+// Create a macro symbol.
+Symbol macro(llvm::StringRef Name, llvm::StringRef ArgList = {});
 
 // Create an Objective-C symbol.
 Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,

@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: Nathan Ridge (HighCommander4)

Changes

This was fixed in clangd/clangd#1484 for Sema completions but the fix did not apply to index completions.

Fixes clangd/clangd#2002


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

4 Files Affected:

  • (modified) clang-tools-extra/clangd/CodeComplete.cpp (+9-3)
  • (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+5-2)
  • (modified) clang-tools-extra/clangd/unittests/TestIndex.cpp (+6-1)
  • (modified) clang-tools-extra/clangd/unittests/TestIndex.h (+3-1)
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
index 0e5f08cec440ce..38bc2552195ef7 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -89,7 +89,9 @@ const CodeCompleteOptions::CodeCompletionRankingModel
 
 namespace {
 
-CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
+CompletionItemKind
+toCompletionItemKind(index::SymbolKind Kind,
+                     const llvm::StringRef *Signature = nullptr) {
   using SK = index::SymbolKind;
   switch (Kind) {
   case SK::Unknown:
@@ -99,7 +101,10 @@ CompletionItemKind toCompletionItemKind(index::SymbolKind Kind) {
   case SK::NamespaceAlias:
     return CompletionItemKind::Module;
   case SK::Macro:
-    return CompletionItemKind::Text;
+    // Use macro signature (if provided) to tell apart function-like and
+    // object-like macros.
+    return Signature && Signature->contains('(') ? CompletionItemKind::Function
+                                                 : CompletionItemKind::Constant;
   case SK::Enum:
     return CompletionItemKind::Enum;
   case SK::Struct:
@@ -379,7 +384,8 @@ struct CodeCompletionBuilder {
       if (Completion.Scope.empty())
         Completion.Scope = std::string(C.IndexResult->Scope);
       if (Completion.Kind == CompletionItemKind::Missing)
-        Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind);
+        Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind,
+                                               &C.IndexResult->Signature);
       if (Completion.Name.empty())
         Completion.Name = std::string(C.IndexResult->Name);
       if (Completion.FilterText.empty())
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 6d387fec9b3851..abdf239a3dfb19 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -671,7 +671,8 @@ TEST(CompletionTest, Kinds) {
           #define MACRO 10
           int X = ^
       )cpp",
-      {func("indexFunction"), var("indexVariable"), cls("indexClass")});
+      {func("indexFunction"), var("indexVariable"), cls("indexClass"),
+       macro("indexObjMacro"), macro("indexFuncMacro", "(x, y)")});
   EXPECT_THAT(Results.Completions,
               AllOf(has("function", CompletionItemKind::Function),
                     has("variable", CompletionItemKind::Variable),
@@ -680,7 +681,9 @@ TEST(CompletionTest, Kinds) {
                     has("MACRO", CompletionItemKind::Constant),
                     has("indexFunction", CompletionItemKind::Function),
                     has("indexVariable", CompletionItemKind::Variable),
-                    has("indexClass", CompletionItemKind::Class)));
+                    has("indexClass", CompletionItemKind::Class),
+                    has("indexObjMacro", CompletionItemKind::Constant),
+                    has("indexFuncMacro", CompletionItemKind::Function)));
 
   Results = completions("nam^");
   EXPECT_THAT(Results.Completions,
diff --git a/clang-tools-extra/clangd/unittests/TestIndex.cpp b/clang-tools-extra/clangd/unittests/TestIndex.cpp
index 278336bdde2ee5..b13a5d32d17524 100644
--- a/clang-tools-extra/clangd/unittests/TestIndex.cpp
+++ b/clang-tools-extra/clangd/unittests/TestIndex.cpp
@@ -38,7 +38,7 @@ static std::string replace(llvm::StringRef Haystack, llvm::StringRef Needle,
 // Helpers to produce fake index symbols for memIndex() or completions().
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
-           llvm::StringRef USRFormat) {
+           llvm::StringRef USRFormat, llvm::StringRef Signature) {
   Symbol Sym;
   std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand!
   size_t Pos = QName.rfind("::");
@@ -55,6 +55,7 @@ Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
   Sym.SymInfo.Kind = Kind;
   Sym.Flags |= Symbol::IndexedForCodeCompletion;
   Sym.Origin = SymbolOrigin::Static;
+  Sym.Signature = Signature;
   return Sym;
 }
 
@@ -86,6 +87,10 @@ Symbol conceptSym(llvm::StringRef Name) {
   return sym(Name, index::SymbolKind::Concept, "@CT@\\0");
 }
 
+Symbol macro(llvm::StringRef Name, llvm::StringRef ArgList) {
+  return sym(Name, index::SymbolKind::Macro, "@macro@\\0", ArgList);
+}
+
 Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,
                llvm::StringRef USRPrefix) {
   Symbol Sym;
diff --git a/clang-tools-extra/clangd/unittests/TestIndex.h b/clang-tools-extra/clangd/unittests/TestIndex.h
index 9280b0b12a67fe..0699b29392d720 100644
--- a/clang-tools-extra/clangd/unittests/TestIndex.h
+++ b/clang-tools-extra/clangd/unittests/TestIndex.h
@@ -20,7 +20,7 @@ Symbol symbol(llvm::StringRef QName);
 // Helpers to produce fake index symbols with proper SymbolID.
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(llvm::StringRef QName, index::SymbolKind Kind,
-           llvm::StringRef USRFormat);
+           llvm::StringRef USRFormat, llvm::StringRef Signature = {});
 // Creats a function symbol assuming no function arg.
 Symbol func(llvm::StringRef Name);
 // Creates a class symbol.
@@ -35,6 +35,8 @@ Symbol var(llvm::StringRef Name);
 Symbol ns(llvm::StringRef Name);
 // Create a C++20 concept symbol.
 Symbol conceptSym(llvm::StringRef Name);
+// Create a macro symbol.
+Symbol macro(llvm::StringRef Name, llvm::StringRef ArgList = {});
 
 // Create an Objective-C symbol.
 Symbol objcSym(llvm::StringRef Name, index::SymbolKind Kind,

…om the index

This was fixed in clangd/clangd#1484 for Sema
completions but the fix did not apply to index completions.

Fixes clangd/clangd#2002
@HighCommander4 HighCommander4 merged commit 7aa3716 into llvm:main Apr 11, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Macro results from index have completion kind Text (should be Function or Constant)
3 participants