[clangd] Autocomplete fixes for methods #165916
Open
+132
−36
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: Hippolyte Melica (etyloppihacilem) ChangesClangd completion fixesVarious fixes in clangd completion, including some changes to the Sema lib. fix clangd/clangd#1752Adds support to autocomplete method and function arguments and qualifiers. Edited unittests to match new behavior. Added fix clangd/clangd#880Change 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#753Complete arguments with default value in case of definition, removing the values. Added unittests. // 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);
};// 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 I don't know if those are unrelated changes, should I maybe open different PR for those three issues ? Full diff: https://github.com/llvm/llvm-project/pull/165916.diff 7 Files Affected:
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:
|
|
@llvm/pr-subscribers-clang Author: Hippolyte Melica (etyloppihacilem) ChangesClangd completion fixesVarious fixes in clangd completion, including some changes to the Sema lib. fix clangd/clangd#1752Adds support to autocomplete method and function arguments and qualifiers. Edited unittests to match new behavior. Added fix clangd/clangd#880Change 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#753Complete arguments with default value in case of definition, removing the values. Added unittests. // 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);
};// 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 I don't know if those are unrelated changes, should I maybe open different PR for those three issues ? Full diff: https://github.com/llvm/llvm-project/pull/165916.diff 7 Files Affected:
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:
|
etyloppihacilem
force-pushed
the
ArgumentCompletionSquash
branch
from
4de5ed0 to
a8e8582
Compare
HighCommander4
requested review from
HighCommander4,
hokein,
kadircet and
llvm-beanz
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_FunctionQualifierto differentiate between plainCodeCompletionString::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.
It does respect the
--function-arg-placeholdersargument and theArgumentListsparameter.I don't know if those are unrelated changes, should I maybe open different PR for those three issues ?