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 01/11] 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 02/11] 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 03/11] 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 04/11] 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 05/11] 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) { >From dbd89e8f2a555957f3e23cfee4d327447c11666c Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 1 Jul 2026 10:32:34 +0200 Subject: [PATCH 06/11] formatting --- clang-tools-extra/clangd/unittests/IndexActionTests.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index c1f580fb892ed..43ef5430ae12c 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -279,8 +279,7 @@ TEST_F(IndexActionTest, DeclParamName) { IndexFileIn IndexFile = runIndexingAction(MainFilePath); - EXPECT_THAT(*IndexFile.Symbols, - ElementsAre(hasSignature("(int, int day)"))); + EXPECT_THAT(*IndexFile.Symbols, ElementsAre(hasSignature("(int, int day)"))); } TEST_F(IndexActionTest, SkipFiles) { >From 52b56bc32ed3fbd59c09f012071934f2d052f049 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Mon, 6 Jul 2026 15:17:32 +0200 Subject: [PATCH 07/11] start iterating from first decl for consistency --- .../clangd/unittests/CodeCompleteTests.cpp | 4 +++- .../clangd/unittests/IndexActionTests.cpp | 4 +++- clang/lib/Sema/SemaCodeComplete.cpp | 12 ++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 16545dcebd023..f5c1c84dbb5e9 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -702,12 +702,14 @@ 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 + // See IndexActionTest.DeclParamName for the other test. clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true; auto FuncFromSource = completions(R"cpp( void sun(int); - void sun(int night); void sun(int day); + void sun(int night); + void sun(int); void position() { sun^; } diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 43ef5430ae12c..3941f3f0dc96c 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -265,13 +265,15 @@ 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 + // See CompletionTest.DeclParamName for the other test. 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 night); void moon(int, int day); + void moon(int month, int day); + void moon(int, int); )cpp"; addFile(MainFilePath, MainCode); diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index f0a431e17d4f1..c45746151677e 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -416,11 +416,12 @@ class ResultBuilder { //@} }; -const FunctionDecl *BetterSingature(const FunctionDecl *Function, +const FunctionDecl *BetterSignature(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) + // Note that `redecls()` traverses in a circular order from the current decl, + // so for consistency we have to first get the first declaration. + for (auto *Redecl : Function->getFirstDecl()->redecls()) + for (unsigned P = Start, N = Redecl->getNumParams(); P != N; ++P) if (Redecl->getParamDecl(P)->getIdentifier()) return Redecl; return Function; @@ -3328,8 +3329,7 @@ static void AddFunctionParameterChunks( bool FirstParameter = true; bool AsInformativeChunk = !(FunctionCanBeCall || IsInDeclarationContext); - // Create consistent result between AST and Index. - const FunctionDecl *BetterSignatureDecl = BetterSingature(Function, Start); + const FunctionDecl *BetterSignatureDecl = BetterSignature(Function, Start); for (unsigned P = Start, N = Function->getNumParams(); P != N; ++P) { const ParmVarDecl *Param = BetterSignatureDecl->getParamDecl(P); >From 529ed052643095ea8bfd3a87ac9e2abd477ee004 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Mon, 6 Jul 2026 15:57:41 +0200 Subject: [PATCH 08/11] wrong order in tests --- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp | 2 +- clang-tools-extra/clangd/unittests/IndexActionTests.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index f5c1c84dbb5e9..860c6d15075b9 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -707,8 +707,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 sun(int); void position() { sun^; diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 3941f3f0dc96c..64166c50fb069 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -271,8 +271,8 @@ TEST_F(IndexActionTest, DeclParamName) { std::string HeaderPath = testPath("zenith.hpp"); std::string HeaderCode = R"cpp( void moon(int, int); - void moon(int, int day); void moon(int month, int day); + void moon(int, int day); void moon(int, int); )cpp"; >From 74bc7a2e182f2360112a7be0bfef18010b30d311 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Wed, 8 Jul 2026 10:49:12 +0200 Subject: [PATCH 09/11] syncinc code completion and signature help --- clang/lib/Sema/SemaCodeComplete.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index c45746151677e..1e8921a332abc 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -418,12 +418,19 @@ class ResultBuilder { const FunctionDecl *BetterSignature(const FunctionDecl *Function, unsigned Start) { + auto ParaCount = Function->getNumParams(); // Note that `redecls()` traverses in a circular order from the current decl, // so for consistency we have to first get the first declaration. - for (auto *Redecl : Function->getFirstDecl()->redecls()) + for (auto *Redecl : Function->getFirstDecl()->redecls()) { + // The callers will expect to be able to use the same index from the initial + // function on the redeclaration. While we do not expect this to happen, + // this is a failsafe. + if (Redecl->getNumParams() > ParaCount) + continue; for (unsigned P = Start, N = Redecl->getNumParams(); P != N; ++P) if (Redecl->getParamDecl(P)->getIdentifier()) return Redecl; + } return Function; } } // namespace @@ -4189,6 +4196,8 @@ static void AddOverloadParameterChunks( bool FirstParameter = true; unsigned NumParams = Function ? Function->getNumParams() : Prototype->getNumParams(); + const FunctionDecl *BetterSignatureDecl = + Function ? BetterSignature(Function, Start) : nullptr; for (unsigned P = Start; P != NumParams; ++P) { if (Function && Function->getParamDecl(P)->hasDefaultArg() && !InOptional) { @@ -4225,8 +4234,8 @@ static void AddOverloadParameterChunks( std::string Placeholder; assert(P < Prototype->getNumParams()); if (Function || PrototypeLoc) { - const ParmVarDecl *Param = - Function ? Function->getParamDecl(P) : PrototypeLoc.getParam(P); + const ParmVarDecl *Param = Function ? BetterSignatureDecl->getParamDecl(P) + : PrototypeLoc.getParam(P); Placeholder = FormatFunctionParameter(Policy, Param); if (Param->hasDefaultArg()) Placeholder += GetDefaultValueString(Param, Context.getSourceManager(), >From a68cc7a8270087bcb3cae56430ab2a56934f12b1 Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Thu, 9 Jul 2026 13:58:40 +0200 Subject: [PATCH 10/11] Added test for signature help --- .../clangd/unittests/CodeCompleteTests.cpp | 24 +++++++++++++++++-- .../clangd/unittests/IndexActionTests.cpp | 5 ++-- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp index 860c6d15075b9..b39663f8427e3 100644 --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -699,10 +699,11 @@ TEST(CompletionTest, PrivateMemberDefinition) { } TEST(CompletionTest, DeclParamName) { - // This is 1/2 regression tests to make sure signatures + // This is 1/3 regression tests to make sure signatures // 1) have consistent variable names between header and source file // 2) find variable names in other declarations - // See IndexActionTest.DeclParamName for the other test. + // See SignatureHelpTest.DeclParamName and IndexActionTest.DeclParamName for + // the other tests. clangd::CodeCompleteOptions Opts; Opts.EnableSnippets = true; auto FuncFromSource = completions(R"cpp( @@ -1696,6 +1697,25 @@ TEST(SignatureHelpTest, Overloads) { EXPECT_EQ(0, Results.activeParameter); } +TEST(SignatureHelpTest, DeclParamName) { + // This is 2/3 regression tests to make sure signatures + // 1) have consistent variable names between header and source file + // 2) find variable names in other declarations + // See CompletionTest.DeclParamName and IndexActionTest.DeclParamName for the + // other tests. + auto FuncFromSource = signatures(R"cpp( + void sun(int); + void sun(int night); + void sun(int day); + void sun(int); + void position() { + sun(^); + } + )cpp"); + EXPECT_THAT(FuncFromSource.signatures, + UnorderedElementsAre(sig("sun([[int day]]) -> void"))); +} + TEST(SignatureHelpTest, FunctionPointers) { llvm::StringLiteral Tests[] = { // Variable of function pointer type diff --git a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp index 64166c50fb069..3cc41be50b53e 100644 --- a/clang-tools-extra/clangd/unittests/IndexActionTests.cpp +++ b/clang-tools-extra/clangd/unittests/IndexActionTests.cpp @@ -262,10 +262,11 @@ TEST_F(IndexActionTest, NoWarnings) { } TEST_F(IndexActionTest, DeclParamName) { - // This is 2/2 regression tests to make sure signatures + // This is 3/3 regression tests to make sure signatures // 1) have consistent variable names between header and source file // 2) find variable names in other declarations - // See CompletionTest.DeclParamName for the other test. + // See CompletionTest.DeclParamName and SignatureHelpTest.DeclParamName for + // the other tests. std::string MainFilePath = testPath("main.cpp"); std::string MainCode = R"cpp( #include "zenith.hpp" )cpp"; std::string HeaderPath = testPath("zenith.hpp"); >From 43192fe371f5896229d850ed478fcfed26697a8f Mon Sep 17 00:00:00 2001 From: Timon Ulrich <[email protected]> Date: Fri, 10 Jul 2026 16:00:36 +0200 Subject: [PATCH 11/11] Review comment fixes --- clang/lib/Sema/SemaCodeComplete.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 1e8921a332abc..f058030d2c6a5 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -416,6 +416,10 @@ class ResultBuilder { //@} }; +// Traverse declarations of the function (in a deterministic order, +// for consistency) to find one which has parameter names. +// For simplicity, consider a redecl to have parameter names +// if at least one parameter has a name. const FunctionDecl *BetterSignature(const FunctionDecl *Function, unsigned Start) { auto ParaCount = Function->getNumParams(); @@ -425,7 +429,7 @@ const FunctionDecl *BetterSignature(const FunctionDecl *Function, // The callers will expect to be able to use the same index from the initial // function on the redeclaration. While we do not expect this to happen, // this is a failsafe. - if (Redecl->getNumParams() > ParaCount) + if (Redecl->getNumParams() < ParaCount) continue; for (unsigned P = Start, N = Redecl->getNumParams(); P != N; ++P) if (Redecl->getParamDecl(P)->getIdentifier()) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
