[clang] [clang-tools-extra] [clangd] Autocomplete fixes for methods (PR #165916)
Hippolyte Melica via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 31 13:09:32 PDT 2025
https://github.com/etyloppihacilem created https://github.com/llvm/llvm-project/pull/165916
# Clangd completion fixes
Various fixes in clangd completion, including some changes to the Sema lib.
## fix clangd/clangd#1752
Adds support to autocomplete method and function arguments and qualifiers.
Edited unittests to match new behavior.
Added `CodeCompletionString::CK_FunctionQualifier` to differentiate between plain `CodeCompletionString::CK_Informative` (e.g. `", ..."`) and function qualifiers.
No other change to Sema behavior.
## fix clangd/clangd#880
Change completion context if a method definition is detected, to allow private/protected completion.
Change to context is only temporary. No other impact on Sema lib behavior.
## fix clangd/clangd#753
Complete arguments with default value in case of definition, removing the values.
Added unittests.
```cpp
// example.hpp
class B {
public:
B(int a, int b);
};
class Test
{
public:
void foo(int a, int b, int c);
void bar(int a, int b, int c) const;
void defargs(int a, int b, int c = 0, int d=1);
void defargs2(int a, int b = 1, B obj = B(1, 2), int c = 5);
private:
void priv(int a, int b);
};
```
```cpp
// example.cpp
void Test::foo^
// completes
void Test::foo(int a, int b, int c)
void Test::bar^
// completes
void Test::bar(int a, int b, int c) const
void Test::defargs^
// completes
void Test::defargs(int a, int b, int c, int d)
void Test::defargs2^
// completes
void Test::defargs2(int a, int b, B obj, int c)
void Test::priv^
// completes
void Test::priv(int a, int b)
```
It does respect the `--function-arg-placeholders` argument and the `ArgumentLists` parameter.
I don't know if those are unrelated changes, should I maybe open different PR for those three issues ?
>From 4de5ed0e486306897315a46aacc04fde7f39e787 Mon Sep 17 00:00:00 2001
From: Hippolyte Melica <hippolytemelica at gmail.com>
Date: Fri, 31 Oct 2025 12:08:07 +0100
Subject: [PATCH] [clangd] Autocomplete fixes for methods
---
.../clangd/CodeCompletionStrings.cpp | 76 +++++++++++++------
.../clangd/unittests/CodeCompleteTests.cpp | 33 ++++++--
.../unittests/CodeCompletionStringsTests.cpp | 2 +-
.../include/clang/Sema/CodeCompleteConsumer.h | 14 +++-
clang/lib/Sema/CodeCompleteConsumer.cpp | 13 ++++
clang/lib/Sema/SemaCodeComplete.cpp | 26 +++++--
clang/tools/libclang/CIndexCodeCompletion.cpp | 4 +
7 files changed, 132 insertions(+), 36 deletions(-)
diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
index 9c4241b54057a..21c99dfdcc5c1 100644
--- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp
+++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp
@@ -39,16 +39,29 @@ void appendEscapeSnippet(const llvm::StringRef Text, std::string *Out) {
}
}
-void appendOptionalChunk(const CodeCompletionString &CCS, std::string *Out) {
+/// Removes the value for defaults arguments.
+static void addWithoutValue(std::string *Out, const std::string &ToAdd) {
+ size_t Val = ToAdd.find('=');
+ if (Val != ToAdd.npos)
+ *Out += ToAdd.substr(0, Val - 1); // removing value in definition
+ else
+ *Out += ToAdd;
+}
+
+void appendOptionalChunk(const CodeCompletionString &CCS, std::string *Out,
+ bool RemoveValues = false) {
for (const CodeCompletionString::Chunk &C : CCS) {
switch (C.Kind) {
case CodeCompletionString::CK_Optional:
assert(C.Optional &&
"Expected the optional code completion string to be non-null.");
- appendOptionalChunk(*C.Optional, Out);
+ appendOptionalChunk(*C.Optional, Out, RemoveValues);
break;
default:
- *Out += C.Text;
+ if (RemoveValues)
+ addWithoutValue(Out, C.Text);
+ else
+ *Out += C.Text;
break;
}
}
@@ -184,6 +197,7 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
unsigned SnippetArg = 0;
bool HadObjCArguments = false;
bool HadInformativeChunks = false;
+ int IsTemplateArgument = 0;
std::optional<unsigned> TruncateSnippetAt;
for (const auto &Chunk : CCS) {
@@ -252,26 +266,39 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
}
}
break;
- case CodeCompletionString::CK_Text:
+ case CodeCompletionString::CK_FunctionQualifier:
+ if (!IncludeFunctionArguments) // if not a call
+ *Snippet += Chunk.Text;
*Signature += Chunk.Text;
+ break;
+ case CodeCompletionString::CK_Text:
*Snippet += Chunk.Text;
+ *Signature += Chunk.Text;
break;
case CodeCompletionString::CK_Optional:
assert(Chunk.Optional);
// No need to create placeholders for default arguments in Snippet.
appendOptionalChunk(*Chunk.Optional, Signature);
+ if (!IncludeFunctionArguments) { // complete args with default value in
+ // definition
+ appendOptionalChunk(*Chunk.Optional, Snippet, /*RemoveValues=*/true);
+ }
break;
case CodeCompletionString::CK_Placeholder:
*Signature += Chunk.Text;
- ++SnippetArg;
- if (SnippetArg == CursorSnippetArg) {
- // We'd like to make $0 a placeholder too, but vscode does not support
- // this (https://github.com/microsoft/vscode/issues/152837).
- *Snippet += "$0";
+ if (IncludeFunctionArguments || IsTemplateArgument) {
+ ++SnippetArg;
+ if (SnippetArg == CursorSnippetArg) {
+ // We'd like to make $0 a placeholder too, but vscode does not support
+ // this (https://github.com/microsoft/vscode/issues/152837).
+ *Snippet += "$0";
+ } else {
+ *Snippet += "${" + std::to_string(SnippetArg) + ':';
+ appendEscapeSnippet(Chunk.Text, Snippet);
+ *Snippet += '}';
+ }
} else {
- *Snippet += "${" + std::to_string(SnippetArg) + ':';
- appendEscapeSnippet(Chunk.Text, Snippet);
- *Snippet += '}';
+ *Snippet += Chunk.Text;
}
break;
case CodeCompletionString::CK_Informative:
@@ -279,6 +306,10 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
// For example, the word "const" for a const method, or the name of
// the base class for methods that are part of the base class.
*Signature += Chunk.Text;
+ if (strcmp(Chunk.Text, " const") == 0 ||
+ strcmp(Chunk.Text, " volatile") == 0 ||
+ strcmp(Chunk.Text, " restrict") == 0)
+ *Snippet += Chunk.Text;
// Don't put the informative chunks in the snippet.
break;
case CodeCompletionString::CK_ResultType:
@@ -290,21 +321,22 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
llvm_unreachable("Unexpected CK_CurrentParameter while collecting "
"CompletionItems");
break;
+ case CodeCompletionString::CK_LeftAngle:
+ IsTemplateArgument++;
+ *Signature += Chunk.Text;
+ *Snippet += Chunk.Text;
+ break;
+ case CodeCompletionString::CK_RightAngle:
+ IsTemplateArgument--;
+ *Signature += Chunk.Text;
+ *Snippet += Chunk.Text;
+ break;
case CodeCompletionString::CK_LeftParen:
- // We're assuming that a LeftParen in a declaration starts a function
- // call, and arguments following the parenthesis could be discarded if
- // IncludeFunctionArguments is false.
- if (!IncludeFunctionArguments &&
- ResultKind == CodeCompletionResult::RK_Declaration)
- TruncateSnippetAt.emplace(Snippet->size());
- [[fallthrough]];
case CodeCompletionString::CK_RightParen:
case CodeCompletionString::CK_LeftBracket:
case CodeCompletionString::CK_RightBracket:
case CodeCompletionString::CK_LeftBrace:
case CodeCompletionString::CK_RightBrace:
- case CodeCompletionString::CK_LeftAngle:
- case CodeCompletionString::CK_RightAngle:
case CodeCompletionString::CK_Comma:
case CodeCompletionString::CK_Colon:
case CodeCompletionString::CK_SemiColon:
@@ -319,8 +351,6 @@ void getSignature(const CodeCompletionString &CCS, std::string *Signature,
break;
}
}
- if (TruncateSnippetAt)
- *Snippet = Snippet->substr(0, *TruncateSnippetAt);
}
std::string formatDocumentation(const CodeCompletionString &CCS,
diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e2bdb0fe46e37..700331632fb61 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -586,14 +586,14 @@ TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) {
auto Results = completions(TU, P, /*IndexSymbols*/ {}, Opts);
EXPECT_THAT(Results.Completions,
Contains(AllOf(named("method"), signature("(int) const"),
- snippetSuffix(""))));
+ snippetSuffix("(int) const"))));
// We don't have any arguments to deduce against if this isn't a call.
// Thus, we should emit these deducible template arguments explicitly.
EXPECT_THAT(
Results.Completions,
- Contains(AllOf(named("generic"),
- signature("<typename T, typename U>(U, V)"),
- snippetSuffix("<${1:typename T}, ${2:typename U}>"))));
+ Contains(
+ AllOf(named("generic"), signature("<typename T, typename U>(U, V)"),
+ snippetSuffix("<${1:typename T}, ${2:typename U}>(U, V)"))));
}
for (const auto &P : Code.points("canBeCall")) {
@@ -620,6 +620,27 @@ TEST(CompletionTest, HeuristicsForMemberFunctionCompletion) {
}
}
+TEST(CompletionTest, DefaultArgsWithValues) {
+ clangd::CodeCompleteOptions Opts;
+ Opts.EnableSnippets = true;
+ auto Results = completions(
+ R"cpp(
+ struct Arg {
+ Arg(int a, int b);
+ };
+ struct Foo {
+ void foo(int x = 42, int y = 0, Arg arg = Arg(42, 0));
+ };
+ void Foo::foo^
+ )cpp",
+ /*IndexSymbols=*/{}, Opts);
+ EXPECT_THAT(Results.Completions,
+ Contains(AllOf(
+ named("foo"),
+ signature("(int x = 42, int y = 0, Arg arg = Arg(42, 0))"),
+ snippetSuffix("(int x, int y, Arg arg)"))));
+}
+
TEST(CompletionTest, NoSnippetsInUsings) {
clangd::CodeCompleteOptions Opts;
Opts.EnableSnippets = true;
@@ -4490,14 +4511,14 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
EXPECT_THAT(
Result.Completions,
ElementsAre(AllOf(named("foo"), signature("<class self:auto>(int arg)"),
- snippetSuffix("<${1:class self:auto}>"))));
+ snippetSuffix("<${1:class self:auto}>(int arg)"))));
}
{
auto Result = codeComplete(testPath(TU.Filename), Code.point("c3"),
Preamble.get(), Inputs, Opts);
EXPECT_THAT(Result.Completions,
ElementsAre(AllOf(named("bar"), signature("(int arg)"),
- snippetSuffix(""))));
+ snippetSuffix("(int arg)"))));
}
}
diff --git a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
index de5f533d31645..48a439e1d8e21 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompletionStringsTests.cpp
@@ -177,7 +177,7 @@ TEST_F(CompletionStringTest, DropFunctionArguments) {
/*IncludeFunctionArguments=*/false);
// Arguments dropped from snippet, kept in signature.
EXPECT_EQ(Signature, "<typename T, int U>(arg1, arg2)");
- EXPECT_EQ(Snippet, "<${1:typename T}, ${2:int U}>");
+ EXPECT_EQ(Snippet, "<${1:typename T}, ${2:int U}>(arg1, arg2)");
}
TEST_F(CompletionStringTest, IgnoreInformativeQualifier) {
diff --git a/clang/include/clang/Sema/CodeCompleteConsumer.h b/clang/include/clang/Sema/CodeCompleteConsumer.h
index c26f4e33d289c..37821bfb51cda 100644
--- a/clang/include/clang/Sema/CodeCompleteConsumer.h
+++ b/clang/include/clang/Sema/CodeCompleteConsumer.h
@@ -473,6 +473,12 @@ class CodeCompletionString {
/// A piece of text that describes something about the result but
/// should not be inserted into the buffer.
CK_Informative,
+
+ /// A piece of text that holds function qualifiers. Should be inserted
+ /// into the buffer only in definition, not on call.
+ /// e.g. const for a function or a method.
+ CK_FunctionQualifier,
+
/// A piece of text that describes the type of an entity or, for
/// functions and methods, the return type.
CK_ResultType,
@@ -534,7 +540,7 @@ class CodeCompletionString {
union {
/// The text string associated with a CK_Text, CK_Placeholder,
- /// CK_Informative, or CK_Comma chunk.
+ /// CK_Informative, CK_FunctionQualifier, or CK_Comma chunk.
/// The string is owned by the chunk and will be deallocated
/// (with delete[]) when the chunk is destroyed.
const char *Text;
@@ -561,6 +567,9 @@ class CodeCompletionString {
/// Create a new informative chunk.
static Chunk CreateInformative(const char *Informative);
+ /// Create a new declaration informative chunk.
+ static Chunk CreateFunctionQualifier(const char *FunctionQualifier);
+
/// Create a new result type chunk.
static Chunk CreateResultType(const char *ResultType);
@@ -737,6 +746,9 @@ class CodeCompletionBuilder {
/// Add a new informative chunk.
void AddInformativeChunk(const char *Text);
+ /// Add a new function qualifier chunk.
+ void AddFunctionQualifierChunk(const char *Text);
+
/// Add a new result-type chunk.
void AddResultTypeChunk(const char *ResultType);
diff --git a/clang/lib/Sema/CodeCompleteConsumer.cpp b/clang/lib/Sema/CodeCompleteConsumer.cpp
index e3fc7c11f4594..419c2678880d1 100644
--- a/clang/lib/Sema/CodeCompleteConsumer.cpp
+++ b/clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -183,6 +183,7 @@ CodeCompletionString::Chunk::Chunk(ChunkKind Kind, const char *Text)
case CK_Text:
case CK_Placeholder:
case CK_Informative:
+ case CK_FunctionQualifier:
case CK_ResultType:
case CK_CurrentParameter:
this->Text = Text;
@@ -272,6 +273,12 @@ CodeCompletionString::Chunk::CreateInformative(const char *Informative) {
return Chunk(CK_Informative, Informative);
}
+CodeCompletionString::Chunk
+CodeCompletionString::Chunk::CreateFunctionQualifier(
+ const char *FunctionQualifier) {
+ return Chunk(CK_FunctionQualifier, FunctionQualifier);
+}
+
CodeCompletionString::Chunk
CodeCompletionString::Chunk::CreateResultType(const char *ResultType) {
return Chunk(CK_ResultType, ResultType);
@@ -326,6 +333,7 @@ std::string CodeCompletionString::getAsString() const {
OS << "<#" << C.Text << "#>";
break;
case CK_Informative:
+ case CK_FunctionQualifier:
case CK_ResultType:
OS << "[#" << C.Text << "#]";
break;
@@ -461,6 +469,10 @@ void CodeCompletionBuilder::AddInformativeChunk(const char *Text) {
Chunks.push_back(Chunk::CreateInformative(Text));
}
+void CodeCompletionBuilder::AddFunctionQualifierChunk(const char *Text) {
+ Chunks.push_back(Chunk::CreateFunctionQualifier(Text));
+}
+
void CodeCompletionBuilder::AddResultTypeChunk(const char *ResultType) {
Chunks.push_back(Chunk::CreateResultType(ResultType));
}
@@ -728,6 +740,7 @@ static std::string getOverloadAsString(const CodeCompletionString &CCS) {
for (auto &C : CCS) {
switch (C.Kind) {
case CodeCompletionString::CK_Informative:
+ case CodeCompletionString::CK_FunctionQualifier:
case CodeCompletionString::CK_ResultType:
OS << "[#" << C.Text << "#]";
break;
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index aa93507ab5c30..0c2bd10373555 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -3440,17 +3440,17 @@ static void AddFunctionTypeQuals(CodeCompletionBuilder &Result,
// Handle single qualifiers without copying
if (Quals.hasOnlyConst()) {
- Result.AddInformativeChunk(" const");
+ Result.AddFunctionQualifierChunk(" const");
return;
}
if (Quals.hasOnlyVolatile()) {
- Result.AddInformativeChunk(" volatile");
+ Result.AddFunctionQualifierChunk(" volatile");
return;
}
if (Quals.hasOnlyRestrict()) {
- Result.AddInformativeChunk(" restrict");
+ Result.AddFunctionQualifierChunk(" restrict");
return;
}
@@ -3462,7 +3462,7 @@ static void AddFunctionTypeQuals(CodeCompletionBuilder &Result,
QualsStr += " volatile";
if (Quals.hasRestrict())
QualsStr += " restrict";
- Result.AddInformativeChunk(Result.getAllocator().CopyString(QualsStr));
+ Result.AddFunctionQualifierChunk(Result.getAllocator().CopyString(QualsStr));
}
static void
@@ -6859,12 +6859,26 @@ void SemaCodeCompletion::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
// resolves to a dependent record.
DeclContext *Ctx = SemaRef.computeDeclContext(SS, /*EnteringContext=*/true);
+ // This is used to change context to access private methods for completion
+ DeclContext *SavedContext = nullptr;
+ if (!SemaRef.CurContext->isFunctionOrMethod() &&
+ !SemaRef.CurContext->isRecord()) {
+ // We are in global scope or namespace
+ // -> likely a method definition
+ SavedContext = SemaRef.CurContext;
+ SemaRef.CurContext = Ctx; // Simulate that we are in class scope
+ // to access private methods
+ }
+
// Try to instantiate any non-dependent declaration contexts before
// we look in them. Bail out if we fail.
NestedNameSpecifier NNS = SS.getScopeRep();
if (NNS && !NNS.isDependent()) {
- if (Ctx == nullptr || SemaRef.RequireCompleteDeclContext(SS, Ctx))
+ if (Ctx == nullptr || SemaRef.RequireCompleteDeclContext(SS, Ctx)) {
+ if (SavedContext)
+ SemaRef.CurContext = SavedContext;
return;
+ }
}
ResultBuilder Results(SemaRef, CodeCompleter->getAllocator(),
@@ -6910,6 +6924,8 @@ void SemaCodeCompletion::CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
/*IncludeDependentBases=*/true,
CodeCompleter->loadExternal());
}
+ if (SavedContext)
+ SemaRef.CurContext = SavedContext;
HandleCodeCompleteResults(&SemaRef, CodeCompleter,
Results.getCompletionContext(), Results.data(),
diff --git a/clang/tools/libclang/CIndexCodeCompletion.cpp b/clang/tools/libclang/CIndexCodeCompletion.cpp
index 81448b4d11342..816afd28e6568 100644
--- a/clang/tools/libclang/CIndexCodeCompletion.cpp
+++ b/clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -70,6 +70,8 @@ clang_getCompletionChunkKind(CXCompletionString completion_string,
case CodeCompletionString::CK_Placeholder:
return CXCompletionChunk_Placeholder;
case CodeCompletionString::CK_Informative:
+ case CodeCompletionString::CK_FunctionQualifier:
+ // as FunctionQualifier are informative, except for completion.
return CXCompletionChunk_Informative;
case CodeCompletionString::CK_ResultType:
return CXCompletionChunk_ResultType;
@@ -120,6 +122,7 @@ CXString clang_getCompletionChunkText(CXCompletionString completion_string,
case CodeCompletionString::CK_Placeholder:
case CodeCompletionString::CK_CurrentParameter:
case CodeCompletionString::CK_Informative:
+ case CodeCompletionString::CK_FunctionQualifier:
case CodeCompletionString::CK_LeftParen:
case CodeCompletionString::CK_RightParen:
case CodeCompletionString::CK_LeftBracket:
@@ -159,6 +162,7 @@ clang_getCompletionChunkCompletionString(CXCompletionString completion_string,
case CodeCompletionString::CK_Placeholder:
case CodeCompletionString::CK_CurrentParameter:
case CodeCompletionString::CK_Informative:
+ case CodeCompletionString::CK_FunctionQualifier:
case CodeCompletionString::CK_LeftParen:
case CodeCompletionString::CK_RightParen:
case CodeCompletionString::CK_LeftBracket:
More information about the cfe-commits
mailing list