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 @@ -872,6 +872,28 @@ SignatureQualitySignals Quality; }; +// Returns the index of the parameter matching argument number "Arg. +// This is usually just "Arg", except for variadic functions/templates, where +// "Arg" 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 paramIndexForArg(const CodeCompleteConsumer::OverloadCandidate &Candidate, + int Arg) { + int NumParams = 0; + if (const auto *F = Candidate.getFunction()) { + NumParams = F->getNumParams(); + if (F->isVariadic()) + ++NumParams; + } else if (auto *T = Candidate.getFunctionType()) { + if (auto *Proto = T->getAs()) { + NumParams = Proto->getNumParams(); + if (Proto->isVariadic()) + ++NumParams; + } + } + return std::min(Arg, std::max(NumParams - 1, 0)); +} + class SignatureHelpCollector final : public CodeCompleteConsumer { public: SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts, @@ -902,7 +924,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 @@ -912,6 +936,14 @@ if (auto *Pattern = Func->getTemplateInstantiationPattern()) Candidate = OverloadCandidate(Pattern); } + if (static_cast(I) == SigHelp.activeSignature) { + // 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. + SigHelp.activeParameter = + paramIndexForArg(Candidate, SigHelp.activeParameter); + } 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 @@ -2622,6 +2622,104 @@ } } +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(SignatureHelpTest, VariadicMethod) { + const std::string Header = R"cpp( + class C { + template + void fun(T t, Args ...args) {} + }; + void test() {C c; )cpp"; + const std::string ExpectedSig = "fun([[T t]], [[Args args...]]) -> void"; + + { + const auto Result = signatures(Header + "c.fun(^);}"); + EXPECT_EQ(0, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "c.fun(1, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "c.fun(1, 2, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } +} + +TEST(SignatureHelpTest, VariadicType) { + const std::string Header = R"cpp( + void fun(int x, ...) {} + auto get_fun() { return fun; } + void test() { + )cpp"; + const std::string ExpectedSig = "([[int]], [[...]]) -> void"; + + { + const auto Result = signatures(Header + "get_fun()(^);}"); + EXPECT_EQ(0, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "get_fun()(1, ^);}"); + EXPECT_EQ(1, Result.activeParameter); + EXPECT_THAT(Result.signatures, UnorderedElementsAre(Sig(ExpectedSig))); + } + { + const auto Result = signatures(Header + "get_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 @@ -1204,6 +1204,20 @@ return Info; } + // Returns false if signature help is relevant despite number of arguments + // exceeding parameters. Specifically, it returns false when + // PartialOverloading is true and one of the following: + // * 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 shouldEnforceArgLimit(bool PartialOverloading, 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,8 @@ if (Candidate.Function) { if (Candidate.Function->isDeleted()) continue; - if (!Candidate.Function->isVariadic() && + if (shouldEnforceArgLimit(/*PartialOverloading=*/true, + 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 @@ -6456,7 +6456,8 @@ // parameters is viable only if it has an ellipsis in its parameter // list (8.3.5). if (TooManyArguments(NumParams, Args.size(), PartialOverloading) && - !Proto->isVariadic()) { + !Proto->isVariadic() && + shouldEnforceArgLimit(PartialOverloading, Function)) { Candidate.Viable = false; Candidate.FailureKind = ovl_fail_too_many_arguments; return; @@ -6946,7 +6947,8 @@ // parameters is viable only if it has an ellipsis in its parameter // list (8.3.5). if (TooManyArguments(NumParams, Args.size(), PartialOverloading) && - !Proto->isVariadic()) { + !Proto->isVariadic() && + shouldEnforceArgLimit(PartialOverloading, Method)) { Candidate.Viable = false; Candidate.FailureKind = ovl_fail_too_many_arguments; return; @@ -15242,3 +15244,21 @@ FunctionDecl *Fn) { return FixOverloadedFunctionReference(E.get(), Found, Fn); } + +bool clang::shouldEnforceArgLimit(bool PartialOverloading, + FunctionDecl *Function) { + if (!PartialOverloading || !Function) + return true; + if (Function->isVariadic()) + return false; + if (const auto *Proto = + dyn_cast(Function->getFunctionType())) + if (Proto->isTemplateVariadic()) + return false; + if (auto *Pattern = Function->getTemplateInstantiationPattern()) + if (const auto *Proto = + dyn_cast(Pattern->getFunctionType())) + if (Proto->isTemplateVariadic()) + return false; + return true; +} diff --git a/clang/test/CodeCompletion/variadic-template.cpp b/clang/test/CodeCompletion/variadic-template.cpp new file mode 100644 --- /dev/null +++ b/clang/test/CodeCompletion/variadic-template.cpp @@ -0,0 +1,18 @@ +template +void fun(T x, Args... args) {} + +void f() { + fun(1, 2, 3, 4); + // The results are quite awkward here, but it's the best we can do for now. + // Tools, including clangd, can unexpand "args" when showing this to the user. + // The important thing is that we provide OVERLOAD signature in all those cases. + // + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | FileCheck --check-prefix=CHECK-1 %s + // CHECK-1: OVERLOAD: [#void#]fun(<#T x#>, Args args...) + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:10 %s -o - | FileCheck --check-prefix=CHECK-2 %s + // CHECK-2: OVERLOAD: [#void#]fun(int x) + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:13 %s -o - | FileCheck --check-prefix=CHECK-3 %s + // CHECK-3: OVERLOAD: [#void#]fun(int x, int args) + // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:16 %s -o - | FileCheck --check-prefix=CHECK-4 %s + // CHECK-4: OVERLOAD: [#void#]fun(int x, int args, int args) +}