kadircet updated this revision to Diff 446531. kadircet added a comment. - Rebase
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130260/new/ https://reviews.llvm.org/D130260 Files: clang-tools-extra/clangd/AST.cpp clang-tools-extra/clangd/unittests/InlayHintTests.cpp Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1429,6 +1429,35 @@ ElementsAre(labelIs(": int"), labelIs(": char"))); } +TEST(ParameterHints, ArgPacksAndConstructors) { + assertParameterHints( + R"cpp( + struct Foo{ Foo(); Foo(int x); }; + void foo(Foo a, int b); + template <typename... Args> + void bar(Args... args) { + foo(args...); + } + template <typename... Args> + void baz(Args... args) { foo($param1[[Foo{args...}]], $param2[[1]]); } + + template <typename... Args> + 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"}); +} + // FIXME: Low-hanging fruit where we could omit a type hint: // - auto x = TypeName(...); // - auto x = (TypeName) (...); Index: clang-tools-extra/clangd/AST.cpp =================================================================== --- clang-tools-extra/clangd/AST.cpp +++ clang-tools-extra/clangd/AST.cpp @@ -786,6 +786,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<PackExpansionExpr>(E) != nullptr; })) { @@ -823,8 +826,7 @@ llvm::Optional<size_t> 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)) { + if (const auto *RefArg = unwrapForward(*It)) { if (Parameters.front() != RefArg->getDecl()) continue; return std::distance(Args.begin(), It); @@ -872,6 +874,13 @@ // 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 that in the AST as so, this might skip explicit calls + // too. + if (const auto *Const = dyn_cast<CXXConstructExpr>(E)) + if (Const->getConstructor()->isCopyOrMoveConstructor()) + E = Const->getArg(0)->IgnoreImplicitAsWritten(); if (const auto *Call = dyn_cast<CallExpr>(E)) { const auto Callee = Call->getBuiltinCallee(); if (Callee == Builtin::BIforward) {
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp =================================================================== --- clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1429,6 +1429,35 @@ ElementsAre(labelIs(": int"), labelIs(": char"))); } +TEST(ParameterHints, ArgPacksAndConstructors) { + assertParameterHints( + R"cpp( + struct Foo{ Foo(); Foo(int x); }; + void foo(Foo a, int b); + template <typename... Args> + void bar(Args... args) { + foo(args...); + } + template <typename... Args> + void baz(Args... args) { foo($param1[[Foo{args...}]], $param2[[1]]); } + + template <typename... Args> + 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"}); +} + // FIXME: Low-hanging fruit where we could omit a type hint: // - auto x = TypeName(...); // - auto x = (TypeName) (...); Index: clang-tools-extra/clangd/AST.cpp =================================================================== --- clang-tools-extra/clangd/AST.cpp +++ clang-tools-extra/clangd/AST.cpp @@ -786,6 +786,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<PackExpansionExpr>(E) != nullptr; })) { @@ -823,8 +826,7 @@ llvm::Optional<size_t> 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)) { + if (const auto *RefArg = unwrapForward(*It)) { if (Parameters.front() != RefArg->getDecl()) continue; return std::distance(Args.begin(), It); @@ -872,6 +874,13 @@ // 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 that in the AST as so, this might skip explicit calls + // too. + if (const auto *Const = dyn_cast<CXXConstructExpr>(E)) + if (Const->getConstructor()->isCopyOrMoveConstructor()) + E = Const->getArg(0)->IgnoreImplicitAsWritten(); if (const auto *Call = dyn_cast<CallExpr>(E)) { const auto Callee = Call->getBuiltinCallee(); if (Callee == Builtin::BIforward) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits