adamcz created this revision.
adamcz added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
adamcz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D111318
Files:
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang/include/clang/Sema/Overload.h
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaOverload.cpp
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ 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<FunctionProtoType>(Function->getFunctionType()))
+ if (Proto->isTemplateVariadic())
+ return true;
+ if (auto *Pattern = Function->getTemplateInstantiationPattern())
+ if (const auto *Proto =
+ dyn_cast<FunctionProtoType>(Pattern->getFunctionType()))
+ if (Proto->isTemplateVariadic())
+ return true;
+ return false;
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ 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
Index: clang/include/clang/Sema/Overload.h
===================================================================
--- clang/include/clang/Sema/Overload.h
+++ 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
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ 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<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(CompletionTest, IncludedCompletionKinds) {
Annotations Test(R"cpp(#include "^)cpp");
auto TU = TestTU::withCode(Test.code());
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -908,7 +908,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
@@ -918,6 +920,23 @@
if (auto *Pattern = Func->getTemplateInstantiationPattern())
Candidate = OverloadCandidate(Pattern);
}
+ if (static_cast<int>(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);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
