https://github.com/timon-ul updated https://github.com/llvm/llvm-project/pull/206716
>From 3b0399fec94e04741c7057e3d5b50878aa8e8a7e Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Tue, 30 Jun 2026 01:35:56 +0200 Subject: [PATCH 1/5] Initial draft: signature parameter names --- .../clangd/CodeCompletionStrings.cpp | 11 ++++++++ .../clangd/CodeCompletionStrings.h | 3 +++ .../clangd/index/SymbolCollector.cpp | 22 ++++++++++++++++ .../clangd/index/SymbolCollector.h | 2 ++ .../clangd/unittests/CodeCompleteTests.cpp | 18 +++++++++++++ .../clangd/unittests/IndexActionTests.cpp | 25 +++++++++++++++++++ 6 files changed, 81 insertions(+) diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index 9c4241b54057a..bec816ce8d379 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -160,6 +160,17 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { return Doc; } +bool isLikelyMissingParameterName(llvm::StringRef Signature) { + size_t LPara = Signature.find('('); + size_t Space = Signature.find(' ', LPara); + size_t RightEnd = Signature.find(',', LPara); + if (Signature.npos == RightEnd) + RightEnd = Signature.find(')', Space); + if (LPara < Space && Space < RightEnd) + return false; + return true; +} + void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, CodeCompletionResult::ResultKind ResultKind, diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index fa81ad64d406c..24b215e5b6c65 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -35,6 +35,9 @@ std::string getDocComment(const ASTContext &Ctx, /// Similar to getDocComment, but returns the comment for a NamedDecl. std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D); +/// Heuristic to see if the signature only has types and no parameter names. +bool isLikelyMissingParameterName(llvm::StringRef Signature); + /// Formats the signature for an item, as a display string and snippet. /// e.g. for const_reference std::vector<T>::at(size_type) const, this returns: /// *Signature = "(size_type) const" diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index b9602b36e2bba..661f93429c7e3 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -725,6 +725,11 @@ bool SymbolCollector::handleDeclOccurrence( BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly); SkipDocCheckInDef = true; } + if (ND != OriginalDecl && + BasicSymbol->Flags & Symbol::IndexedForCodeCompletion && + isLikelyMissingParameterName(BasicSymbol->Signature)) { + getNewSignature(*BasicSymbol, *OriginalDecl); + } if (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef); @@ -1065,6 +1070,23 @@ void SymbolCollector::finish() { FilesWithObjCConstructs.clear(); } +void SymbolCollector::getNewSignature(const Symbol &OldSymbol, + const NamedDecl &ND) { + CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0); + const auto *CCS = SymbolCompletion.CreateCodeCompletionString( + *ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator, + *CompletionTUInfo, + /*IncludeBriefComments*/ false); + std::string Signature; + std::string SnippetSuffix; + getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, + SymbolCompletion.CursorKind); + Symbol UpdatedSymbol = OldSymbol; + UpdatedSymbol.Signature = Signature; + UpdatedSymbol.CompletionSnippetSuffix = SnippetSuffix; + Symbols.insert(UpdatedSymbol); +} + const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, bool IsMainFileOnly) { auto &Ctx = ND.getASTContext(); diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index 4d51d747639b1..f9b8e9a7337e3 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -165,6 +165,8 @@ class SymbolCollector : public index::IndexDataConsumer { SmallVector<const CXXConstructorDecl *, 1> findIndirectConstructors(const Decl *D); + void getNewSignature(const Symbol &OldSymbol, const NamedDecl &ND); + const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); void addDefinition(const NamedDecl &, const Symbol &DeclSymbol, diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 5808b2145965f..037983a80300d 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -698,6 +698,24 @@ TEST(CompletionTest, PrivateMemberDefinition) { snippetSuffix("(int a, int b)")))); } +TEST(CompletionTest, DeclParamName) { + // This is 1/2 regression tests to make sure signatures + // 1) have consistent variable names between header and source file + // 2) find variable names in other declarations + clangd::CodeCompleteOptions Opts; + Opts.EnableSnippets = true; + auto FuncFromSource = completions(R"cpp( + void sun(int); + void sun(int day); + void sun(int); + void position() { + sun^; + } + )cpp", + {}, Opts); + EXPECT_THAT(FuncFromSource.Completions, Contains(signature("(int day)"))); +} + TEST(CompletionTest, DefaultArgsWithValues) { clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true; diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 025027fdcee5d..5b8029654a73d 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -11,6 +11,7 @@ #include "URI.h" #include "index/IndexAction.h" #include "index/Serialization.h" +#include "index/Symbol.h" #include "clang/Basic/SourceLocation.h" #include "clang/Basic/SourceManager.h" #include "clang/Tooling/Tooling.h" @@ -38,6 +39,8 @@ MATCHER_P(hasDigest, Digest, "") { return arg.Digest == Digest; } MATCHER_P(hasName, Name, "") { return arg.Name == Name; } +MATCHER_P(hasSignature, Signature, "") { return arg.Signature == Signature; } + MATCHER(hasSameURI, "") { llvm::StringRef URI = ::testing::get<0>(arg); const std::string &Path = ::testing::get<1>(arg); @@ -258,6 +261,28 @@ TEST_F(IndexActionTest, NoWarnings) { EXPECT_THAT(*IndexFile.Symbols, ElementsAre(hasName("foo"), hasName("bar"))); } +TEST_F(IndexActionTest, DeclParamName) { + // This is 2/2 regression tests to make sure signatures + // 1) have consistent variable names between header and source file + // 2) find variable names in other declarations + std::string MainFilePath = testPath("main.cpp"); + std::string MainCode = R"cpp( #include "zenith.hpp" )cpp"; + std::string HeaderPath = testPath("zenith.hpp"); + std::string HeaderCode = R"cpp( + void moon(int, int); + void moon(int month, int day); + void moon(int, int); + )cpp"; + + addFile(MainFilePath, MainCode); + addFile(HeaderPath, HeaderCode); + + IndexFileIn IndexFile = runIndexingAction(MainFilePath); + + EXPECT_THAT(*IndexFile.Symbols, + ElementsAre(hasSignature("(int month, int day)"))); +} + TEST_F(IndexActionTest, SkipFiles) { std::string MainFilePath = testPath("main.cpp"); addFile(MainFilePath, R"cpp( >From 3dcf8773d0b663388692f4f7a5db0258c1cedfd5 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Tue, 30 Jun 2026 14:03:01 +0200 Subject: [PATCH 2/5] Better approach for finding the right signature --- .../clangd/CodeCompletionStrings.cpp | 11 ---------- .../clangd/CodeCompletionStrings.h | 3 --- .../clangd/index/SymbolCollector.cpp | 22 ------------------- .../clangd/index/SymbolCollector.h | 2 -- clang/lib/Sema/SemaCodeComplete.cpp | 7 ++++++ 5 files changed, 7 insertions(+), 38 deletions(-) diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.cpp b/clang-tools-extra/clangd/CodeCompletionStrings.cpp index bec816ce8d379..9c4241b54057a 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.cpp +++ b/clang-tools-extra/clangd/CodeCompletionStrings.cpp @@ -160,17 +160,6 @@ std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &Decl) { return Doc; } -bool isLikelyMissingParameterName(llvm::StringRef Signature) { - size_t LPara = Signature.find('('); - size_t Space = Signature.find(' ', LPara); - size_t RightEnd = Signature.find(',', LPara); - if (Signature.npos == RightEnd) - RightEnd = Signature.find(')', Space); - if (LPara < Space && Space < RightEnd) - return false; - return true; -} - void getSignature(const CodeCompletionString &CCS, std::string *Signature, std::string *Snippet, CodeCompletionResult::ResultKind ResultKind, diff --git a/clang-tools-extra/clangd/CodeCompletionStrings.h b/clang-tools-extra/clangd/CodeCompletionStrings.h index 24b215e5b6c65..fa81ad64d406c 100644 --- a/clang-tools-extra/clangd/CodeCompletionStrings.h +++ b/clang-tools-extra/clangd/CodeCompletionStrings.h @@ -35,9 +35,6 @@ std::string getDocComment(const ASTContext &Ctx, /// Similar to getDocComment, but returns the comment for a NamedDecl. std::string getDeclComment(const ASTContext &Ctx, const NamedDecl &D); -/// Heuristic to see if the signature only has types and no parameter names. -bool isLikelyMissingParameterName(llvm::StringRef Signature); - /// Formats the signature for an item, as a display string and snippet. /// e.g. for const_reference std::vector<T>::at(size_type) const, this returns: /// *Signature = "(size_type) const" diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp index 661f93429c7e3..b9602b36e2bba 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -725,11 +725,6 @@ bool SymbolCollector::handleDeclOccurrence( BasicSymbol = addDeclaration(*ND, std::move(ID), IsMainFileOnly); SkipDocCheckInDef = true; } - if (ND != OriginalDecl && - BasicSymbol->Flags & Symbol::IndexedForCodeCompletion && - isLikelyMissingParameterName(BasicSymbol->Signature)) { - getNewSignature(*BasicSymbol, *OriginalDecl); - } if (Roles & static_cast<unsigned>(index::SymbolRole::Definition)) addDefinition(*OriginalDecl, *BasicSymbol, SkipDocCheckInDef); @@ -1070,23 +1065,6 @@ void SymbolCollector::finish() { FilesWithObjCConstructs.clear(); } -void SymbolCollector::getNewSignature(const Symbol &OldSymbol, - const NamedDecl &ND) { - CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0); - const auto *CCS = SymbolCompletion.CreateCodeCompletionString( - *ASTCtx, *PP, CodeCompletionContext::CCC_Symbol, *CompletionAllocator, - *CompletionTUInfo, - /*IncludeBriefComments*/ false); - std::string Signature; - std::string SnippetSuffix; - getSignature(*CCS, &Signature, &SnippetSuffix, SymbolCompletion.Kind, - SymbolCompletion.CursorKind); - Symbol UpdatedSymbol = OldSymbol; - UpdatedSymbol.Signature = Signature; - UpdatedSymbol.CompletionSnippetSuffix = SnippetSuffix; - Symbols.insert(UpdatedSymbol); -} - const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID, bool IsMainFileOnly) { auto &Ctx = ND.getASTContext(); diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h index f9b8e9a7337e3..4d51d747639b1 100644 --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -165,8 +165,6 @@ class SymbolCollector : public index::IndexDataConsumer { SmallVector<const CXXConstructorDecl *, 1> findIndirectConstructors(const Decl *D); - void getNewSignature(const Symbol &OldSymbol, const NamedDecl &ND); - const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol); void addDefinition(const NamedDecl &, const Symbol &DeclSymbol, diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index bef5eea9ed442..f724fb44f184e 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -3320,6 +3320,13 @@ static void AddFunctionParameterChunks( for (unsigned P = Start, N = Function->getNumParams(); P != N; ++P) { const ParmVarDecl *Param = Function->getParamDecl(P); + // Select a parameter with identifier if possible for better signature + if (!Param->getIdentifier()) { + for (auto *Redecl : Function->redecls()) { + if (auto *RParam = Redecl->getParamDecl(P); RParam->getIdentifier()) + Param = RParam; + } + } if (Param->hasDefaultArg() && !InOptional && !IsInDeclarationContext && !AsInformativeChunk) { >From 52df79312bcaee81e898df5e3a14da2ee47772ff Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Tue, 30 Jun 2026 14:28:08 +0200 Subject: [PATCH 3/5] actual consistent results between AST and Index --- .../clangd/unittests/CodeCompleteTests.cpp | 2 +- .../clangd/unittests/IndexActionTests.cpp | 2 +- clang/lib/Sema/SemaCodeComplete.cpp | 10 ++++------ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 037983a80300d..f2eae8a883cd1 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -707,7 +707,7 @@ TEST(CompletionTest, DeclParamName) { auto FuncFromSource = completions(R"cpp( void sun(int); void sun(int day); - void sun(int); + void sun(int night); void position() { sun^; } diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 5b8029654a73d..ef760abe94ea0 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -271,7 +271,7 @@ TEST_F(IndexActionTest, DeclParamName) { std::string HeaderCode = R"cpp( void moon(int, int); void moon(int month, int day); - void moon(int, int); + void moon(int month, int night); )cpp"; addFile(MainFilePath, MainCode); diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index f724fb44f184e..1b833d37237fa 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -3320,12 +3320,10 @@ static void AddFunctionParameterChunks( for (unsigned P = Start, N = Function->getNumParams(); P != N; ++P) { const ParmVarDecl *Param = Function->getParamDecl(P); - // Select a parameter with identifier if possible for better signature - if (!Param->getIdentifier()) { - for (auto *Redecl : Function->redecls()) { - if (auto *RParam = Redecl->getParamDecl(P); RParam->getIdentifier()) - Param = RParam; - } + // Select the first parameter name for consistent results with Index. + for (auto *Redecl : Function->redecls()) { + if (auto *RParam = Redecl->getParamDecl(P); RParam->getIdentifier()) + Param = RParam; } if (Param->hasDefaultArg() && !InOptional && !IsInDeclarationContext && >From fb1b5532a564f70f55250983d5c4a9f4242fef17 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 1 Jul 2026 09:58:30 +0200 Subject: [PATCH 4/5] Select first function with any param name --- .../clangd/unittests/CodeCompleteTests.cpp | 2 +- .../clangd/unittests/IndexActionTests.cpp | 2 +- clang/lib/Sema/SemaCodeComplete.cpp | 20 +++++++++++++------ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index f2eae8a883cd1..16545dcebd023 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -706,8 +706,8 @@ TEST(CompletionTest, DeclParamName) { Opts.EnableSnippets = true; auto FuncFromSource = completions(R"cpp( void sun(int); - void sun(int day); void sun(int night); + void sun(int day); void position() { sun^; } diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index ef760abe94ea0..f3ff10dcca9a5 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -270,8 +270,8 @@ TEST_F(IndexActionTest, DeclParamName) { std::string HeaderPath = testPath("zenith.hpp"); std::string HeaderCode = R"cpp( void moon(int, int); - void moon(int month, int day); void moon(int month, int night); + void moon(int month, int day); )cpp"; addFile(MainFilePath, MainCode); diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 1b833d37237fa..f0a431e17d4f1 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -415,6 +415,16 @@ class ResultBuilder { bool IsImpossibleToSatisfy(const NamedDecl *ND) const; //@} }; + +const FunctionDecl *BetterSingature(const FunctionDecl *Function, + unsigned Start) { + // Note that `redecls()` traverses from last to first declaration. + for (auto *Redecl : Function->redecls()) + for (unsigned P = Start, N = Function->getNumParams(); P != N; ++P) + if (Redecl->getParamDecl(P)->getIdentifier()) + return Redecl; + return Function; +} } // namespace void PreferredTypeBuilder::enterReturn(Sema &S, SourceLocation Tok) { @@ -3318,13 +3328,11 @@ static void AddFunctionParameterChunks( bool FirstParameter = true; bool AsInformativeChunk = !(FunctionCanBeCall || IsInDeclarationContext); + // Create consistent result between AST and Index. + const FunctionDecl *BetterSignatureDecl = BetterSingature(Function, Start); + for (unsigned P = Start, N = Function->getNumParams(); P != N; ++P) { - const ParmVarDecl *Param = Function->getParamDecl(P); - // Select the first parameter name for consistent results with Index. - for (auto *Redecl : Function->redecls()) { - if (auto *RParam = Redecl->getParamDecl(P); RParam->getIdentifier()) - Param = RParam; - } + const ParmVarDecl *Param = BetterSignatureDecl->getParamDecl(P); if (Param->hasDefaultArg() && !InOptional && !IsInDeclarationContext && !AsInformativeChunk) { >From 1b451572eb79da4a57a2d050869687d2e6473c37 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 1 Jul 2026 10:14:43 +0200 Subject: [PATCH 5/5] Better test for current behaviour --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index f3ff10dcca9a5..c1f580fb892ed 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -271,7 +271,7 @@ TEST_F(IndexActionTest, DeclParamName) { std::string HeaderCode = R"cpp( void moon(int, int); void moon(int month, int night); - void moon(int month, int day); + void moon(int, int day); )cpp"; addFile(MainFilePath, MainCode); @@ -280,7 +280,7 @@ TEST_F(IndexActionTest, DeclParamName) { IndexFileIn IndexFile = runIndexingAction(MainFilePath); EXPECT_THAT(*IndexFile.Symbols, - ElementsAre(hasSignature("(int month, int day)"))); + ElementsAre(hasSignature("(int, int day)"))); } TEST_F(IndexActionTest, SkipFiles) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
