upsj created this revision.
upsj added reviewers: nridge, sammccall, kadircet.
upsj added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
upsj requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
The `ForwardingCallVisitor` previously picked up on (constructor or function)
calls inside a single argument of a variadic function, where the corresponding
pack expression is (partially) expanded, but only consists of a single entry.
This mismatch between expecting the full pack and finding only a single entry
caused a crash, which the first half of this diff fixes.
The second half deals with the failure of `resolveForwardingParameters` to
detect forwarded parameters when implicit conversions happen via constructor
calls, not normal `ImplicitCastExpr`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D130259
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
@@ -680,6 +680,71 @@
ExpectedHint{"C: ", "param3"});
}
+TEST(ParameterHints, VariadicWithConstructorConversion) {
+ // variadic call that involves a conversion via a (potentially implicitly
+ // provided) constructor, here move-construction Foo&& -> Foo
+ assertParameterHints(
+ R"cpp(
+ struct Foo {};
+ void foo(Foo s, int x);
+
+ template <typename... _Args>
+ void bar(_Args ...__args) {
+ foo(__args...);
+ }
+
+ void foo() {
+ bar($param1[[Foo{}]], $param2[[3]]);
+ }
+ )cpp",
+ ExpectedHint{"s: ", "param1"}, ExpectedHint{"x: ", "param2"});
+}
+
+TEST(ParameterHints, VariadicWithOperatorConversion) {
+ // variadic call that involves a conversion via an operator, here Foo -> Bar
+ assertParameterHints(
+ R"cpp(
+ struct Bar {};
+ struct Foo {
+ operator Bar();
+ };
+ void foo(Bar s, int x);
+
+ template <typename... _Args>
+ void bar(_Args ...__args) {
+ foo(__args...);
+ }
+
+ void foo() {
+ bar($param1[[Foo{}]], $param2[[3]]);
+ }
+ )cpp" /*, ExpectedHint{"s: ", "param1"}, ExpectedHint{"x: ", "param2"}*/);
+}
+
+TEST(ParameterHints, VariadicWithFunctionCall) {
+ // variadic call that involves an explicit transformation via a function, here
+ // we don't output an inlay hint, since it's not clear what transform does.
+ assertParameterHints(
+ R"cpp(
+ struct Bar {};
+ struct Foo {
+ operator Bar();
+ };
+ template <typename T>
+ T transform(T);
+ void foo(Bar s, int x);
+
+ template <typename... _Args>
+ void bar(_Args ...__args) {
+ foo(transform(__args)...);
+ }
+
+ void foo() {
+ bar(Foo{}, 3);
+ }
+ )cpp");
+}
+
TEST(ParameterHints, VariadicReferenceHint) {
assertParameterHints(R"cpp(
void foo(int&);
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -832,10 +832,26 @@
}
return false;
});
- if (FirstMatch == Args.end()) {
- return llvm::None;
+ if (FirstMatch != Args.end()) {
+ const auto NumArgs =
+ static_cast<size_t>(std::distance(Args.begin(), Args.end()));
+ const auto FirstMatchIdx =
+ static_cast<size_t>(std::distance(Args.begin(), FirstMatch));
+ // Now we found the first parameter, check whether the last one also fits
+ // This is necessary because we may be matching a function call inside an
+ // argument of the variadic function if we didn't successfully match the
+ // outer variadic call.
+ const auto LastMatchIdx = FirstMatchIdx + Parameters.size() - 1;
+ if (LastMatchIdx < NumArgs) {
+ if (const auto *RefArg =
+ unwrapArgument(*(Args.begin() + LastMatchIdx))) {
+ if (Parameters.back() == dyn_cast<ParmVarDecl>(RefArg->getDecl())) {
+ return FirstMatchIdx;
+ }
+ }
+ }
}
- return std::distance(Args.begin(), FirstMatch);
+ return llvm::None;
}
static FunctionDecl *getCalleeDeclOrUniqueOverload(CallExpr *E) {
@@ -881,7 +897,7 @@
return E;
}
- // Maps std::forward(E) to E, nullptr otherwise
+ // Maps std::forward(E) to E
static const Expr *unwrapForward(const Expr *E) {
if (const auto *Call = dyn_cast<CallExpr>(E)) {
const auto Callee = Call->getBuiltinCallee();
@@ -892,10 +908,26 @@
return E;
}
+ // Maps an argument implicitly calling a converting constructor to the
+ // constructor's argument
+ static const Expr *unwrapConstructorConversion(const Expr *E) {
+ if (const auto *CBTE = dyn_cast<CXXBindTemporaryExpr>(E)) {
+ E = CBTE->getSubExpr();
+ }
+ if (const auto *CCE = dyn_cast<CXXConstructExpr>(E)) {
+ const auto *C = CCE->getConstructor();
+ if (!C->isExplicit() && C->getNumParams() == 1) {
+ E = CCE->getArg(0);
+ }
+ }
+ return E;
+ }
+
// Maps std::forward(DeclRefExpr) to DeclRefExpr, removing any intermediate
// implicit casts, nullptr otherwise
static const DeclRefExpr *unwrapArgument(const Expr *E) {
E = unwrapImplicitCast(E);
+ E = unwrapConstructorConversion(E);
E = unwrapForward(E);
E = unwrapImplicitCast(E);
return dyn_cast<DeclRefExpr>(E);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
