diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -908,7 +908,9 @@ SigHelp.activeSignature = 0; assert(CurrentArg <= (unsigned)std::numeric_limits::max() && "too many arguments"); + SigHelp.activeParameter = static_cast(CurrentArg); + for (unsigned I = 0; I < NumCandidates; ++I) { OverloadCandidate Candidate = Candidates[I]; // We want to avoid showing instantiated signatures, because they may be @@ -918,6 +920,23 @@ if (auto *Pattern = Func->getTemplateInstantiationPattern()) Candidate = OverloadCandidate(Pattern); } + if (static_cast(I) == SigHelp.activeSignature && + Candidate.getFunction()) { + // The activeParameter in LSP relates to the activeSignature. There is + // another, per-signature field, but we currently do not use it and not + // all clients might support it. + // FIXME: Add support for per-signature activeParameter field. + // + // This only matters for variadic functions/templates, where CurrentArg + // might be higher than the number of parameters. When that happens, we + // assume the last parameter is variadic and assume all further args are + // part of it. + int NumParams = Candidate.getFunction()->getNumParams(); + if (Candidate.getFunction()->isVariadic()) + ++NumParams; + SigHelp.activeParameter = + std::min(SigHelp.activeParameter, NumParams - 1); + } const auto *CCS = Candidate.CreateSignatureString( CurrentArg, S, *Allocator, CCTUInfo, true); diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp --- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp +++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp @@ -2601,6 +2601,53 @@ } } +TEST(SignatureHelpTest, Variadic) { + const std::string Header = R"cpp( + void fun(int x, ...) {} + void test() {)cpp"; + const std::string ExpectedSig = "fun([[int x]], [[...]]) -> void"; + + { + const auto Result = signatures(Header + "fun(^);}"); + EXPECT_EQ(0, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "fun(1, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "fun(1, 2, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } +} + +TEST(SignatureHelpTest, VariadicTemplate) { + const std::string Header = R"cpp( + template + void fun(T t, Args ...args) {} + void test() {)cpp"; + const std::string ExpectedSig = "fun([[T t]], [[Args args...]]) -> void"; + + { + const auto Result = signatures(Header + "fun(^);}"); + EXPECT_EQ(0, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "fun(1, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "fun(1, 2, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } +} + TEST(CompletionTest, IncludedCompletionKinds) { Annotations Test(R"cpp(#include "^)cpp"); auto TU = TestTU::withCode(Test.code()); diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h --- a/clang/include/clang/Sema/Overload.h +++ b/clang/include/clang/Sema/Overload.h @@ -1193,6 +1193,19 @@ return Info; } + // Returns true if signature help is relevant despite number of arguments + // exceeding parameters. Specifically, it returns true when: + // * Function is variadic + // * Function is template variadic + // * Function is an instantiation of template variadic function + // The last case may seem strange. The idea is that if we added one more + // argument, we'd end up with a function similar to Function. Since, in the + // context of signature help and/or code completion, we do not know what the + // type of the next argument (that the user is typing) will be, this is as + // good candidate as we can get, despite the fact that it takes one less + // parameter. + bool isVariadicOrInstantiatedFromVariadicTemplate(FunctionDecl *Function); + } // namespace clang #endif // LLVM_CLANG_SEMA_OVERLOAD_H diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -5818,7 +5818,7 @@ if (Candidate.Function) { if (Candidate.Function->isDeleted()) continue; - if (!Candidate.Function->isVariadic() && + if (!isVariadicOrInstantiatedFromVariadicTemplate(Candidate.Function) && Candidate.Function->getNumParams() <= ArgSize && // Having zero args is annoying, normally we don't surface a function // with 2 params, if you already have 2 params, because you are diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp --- a/clang/lib/Sema/SemaOverload.cpp +++ b/clang/lib/Sema/SemaOverload.cpp @@ -6459,9 +6459,19 @@ // list (8.3.5). if (TooManyArguments(NumParams, Args.size(), PartialOverloading) && !Proto->isVariadic()) { - Candidate.Viable = false; - Candidate.FailureKind = ovl_fail_too_many_arguments; - return; + // When PartialOverloading is true (code completion/signature help), we want + // to keep a candidate with less params as long as it's either + // [template] variadic or was instantiated from variadic template. In those + // cases, if we were to add another argument (which the user is currently + // doing) the signature would remain valid. In case of variadic templates it + // would actually be a different function (with one extra parameter), but + // for the purpose of signature help that's good enough. + if (!PartialOverloading || + !isVariadicOrInstantiatedFromVariadicTemplate(Function)) { + Candidate.Viable = false; + Candidate.FailureKind = ovl_fail_too_many_arguments; + return; + } } // (C++ 13.3.2p2): A candidate function having more than m parameters @@ -15242,3 +15252,21 @@ FunctionDecl *Fn) { return FixOverloadedFunctionReference(E.get(), Found, Fn); } + +bool clang::isVariadicOrInstantiatedFromVariadicTemplate( + FunctionDecl *Function) { + if (!Function) + return false; + if (Function->isVariadic()) + return true; + if (const auto *Proto = + dyn_cast(Function->getFunctionType())) + if (Proto->isTemplateVariadic()) + return true; + if (auto *Pattern = Function->getTemplateInstantiationPattern()) + if (const auto *Proto = + dyn_cast(Pattern->getFunctionType())) + if (Proto->isTemplateVariadic()) + return true; + return false; +}