[clang][clangd] Improve signature help for variadic functions.
This covers both C-style variadic functions and template variadic w/
parameter packs.
Previously we would return no signatures when working with template
variadic functions once activeParameter reached the position of the
parameter pack (except when it was the only param, then we'd still
show it when no arguments were given). With this commit, we now show
signathure help correctly.
Additionally, this commit fixes the activeParameter value in LSP output
of clangd in the presence of variadic functions (both kinds). LSP does
not allow the activeParamter to be higher than the number of parameters
in the active signature. With "..." or parameter pack being just one
argument, for all but first argument passed to "..." we'd report
incorrect activeParameter value. Clients such as VSCode would then treat
it as 0, as suggested in the spec) and highlight the wrong parameter.
In the future, we should add support for per-signature activeParamter
value, which exists in LSP since 3.16.0. This is not part of this
commit.
Differential Revision: https://reviews.llvm.org/D111318
GitOrigin-RevId: 55a79318c60d8a39329195f43bf43b89da9a638e
diff --git a/clangd/CodeComplete.cpp b/clangd/CodeComplete.cpp
index f033b5e..ac6c3b0 100644
--- a/clangd/CodeComplete.cpp
+++ b/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<FunctionProtoType>()) {
+ 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<int>::max() &&
"too many arguments");
+
SigHelp.activeParameter = static_cast<int>(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<int>(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/clangd/unittests/CodeCompleteTests.cpp b/clangd/unittests/CodeCompleteTests.cpp
index 81dfdcb..5612a0b 100644
--- a/clangd/unittests/CodeCompleteTests.cpp
+++ b/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<typename T, typename ...Args>
+ 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<typename T, typename ...Args>
+ 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());
