diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp --- a/clang-tools-extra/clangd/AST.cpp +++ b/clang-tools-extra/clangd/AST.cpp @@ -36,6 +36,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/Support/Casting.h" #include "llvm/Support/raw_ostream.h" +#include #include #include @@ -786,6 +787,9 @@ // inspects the given callee with the given args to check whether it // contains Parameters, and sets Info accordingly. void handleCall(FunctionDecl *Callee, typename CallExpr::arg_range Args) { + // Skip functions with less parameters, they can't be the target. + if (Callee->parameters().size() < Parameters.size()) + return; if (std::any_of(Args.begin(), Args.end(), [](const Expr *E) { return dyn_cast(E) != nullptr; })) { @@ -822,12 +826,20 @@ // in the given arguments, if it is there. llvm::Optional findPack(typename CallExpr::arg_range Args) { // find the argument directly referring to the first parameter - for (auto It = Args.begin(); It != Args.end(); ++It) { - const Expr *Arg = *It; - if (const auto *RefArg = unwrapForward(Arg)) { + assert(Parameters.size() <= static_cast(llvm::size(Args))); + for (auto Begin = Args.begin(), End = Args.end() - Parameters.size() + 1; + Begin != End; ++Begin) { + if (const auto *RefArg = unwrapForward(*Begin)) { if (Parameters.front() != RefArg->getDecl()) continue; - return std::distance(Args.begin(), It); + // Check that this expands all the way until the last parameter. + // It's enough to look at the last parameter, because it isn't possible + // to expand without expanding all of them. + auto ParamEnd = Begin + Parameters.size() - 1; + RefArg = unwrapForward(*ParamEnd); + if (!RefArg || Parameters.back() != RefArg->getDecl()) + continue; + return std::distance(Args.begin(), Begin); } } return llvm::None; @@ -872,6 +884,12 @@ // std::forward. static const DeclRefExpr *unwrapForward(const Expr *E) { E = E->IgnoreImplicitAsWritten(); + // There might be an implicit copy/move constructor call on top of the + // forwarded arg. + // FIXME: Maybe mark implicit calls in the AST to properly filter here. + if (const auto *Const = dyn_cast(E)) + if (Const->getConstructor()->isCopyOrMoveConstructor()) + E = Const->getArg(0)->IgnoreImplicitAsWritten(); if (const auto *Call = dyn_cast(E)) { const auto Callee = Call->getBuiltinCallee(); if (Callee == Builtin::BIforward) { diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1429,6 +1429,51 @@ ElementsAre(labelIs(": int"), labelIs(": char"))); } +TEST(ParameterHints, ArgPacksAndConstructors) { + assertParameterHints( + R"cpp( + struct Foo{ Foo(); Foo(int x); }; + void foo(Foo a, int b); + template + void bar(Args... args) { + foo(args...); + } + template + void baz(Args... args) { foo($param1[[Foo{args...}]], $param2[[1]]); } + + template + void bax(Args... args) { foo($param3[[{args...}]], args...); } + + void foo() { + bar($param4[[Foo{}]], $param5[[42]]); + bar($param6[[42]], $param7[[42]]); + baz($param8[[42]]); + bax($param9[[42]]); + } + )cpp", + ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"}, + ExpectedHint{"a: ", "param3"}, ExpectedHint{"a: ", "param4"}, + ExpectedHint{"b: ", "param5"}, ExpectedHint{"a: ", "param6"}, + ExpectedHint{"b: ", "param7"}, ExpectedHint{"x: ", "param8"}, + ExpectedHint{"b: ", "param9"}); +} + +TEST(ParameterHints, DoesntExpandAllArgs) { + assertParameterHints( + R"cpp( + void foo(int x, int y); + int id(int a, int b, int c); + template + void bar(Args... args) { + foo(id($param1[[args]], $param2[[1]], $param3[[args]])...); + } + void foo() { + bar(1, 2); // FIXME: We could have `bar(a: 1, a: 2)` here. + } + )cpp", + ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"}, + ExpectedHint{"c: ", "param3"}); +} // FIXME: Low-hanging fruit where we could omit a type hint: // - auto x = TypeName(...); // - auto x = (TypeName) (...);