kadircet updated this revision to Diff 446796.
kadircet marked 5 inline comments as done.
kadircet added a comment.
- Address comments
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,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 <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"});
+}
+
+TEST(ParameterHints, DoesntExpandAllArgs) {
+ assertParameterHints(
+ R"cpp(
+ void foo(int x, int y);
+ int id(int a, int b, int c);
+ template <typename... Args>
+ 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) (...);
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ 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 <iterator>
#include <string>
#include <vector>
@@ -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<PackExpansionExpr>(E) != nullptr;
})) {
@@ -822,12 +826,21 @@
// in the given arguments, if it is there.
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)) {
+ assert(Parameters.size() <=
+ static_cast<size_t>(std::distance(Args.begin(), Args.end())));
+ 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 +885,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<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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
