[PATCH] D130260: [clangd] Make forwarding parameter detection logic resilient

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang-tools-extra.

This could crash when our heuristic picks the wrong function. Make sure
there is enough parameters in the candidate to prevent those crashes.

Also special case copy/move constructors to make the heuristic work in
presence of those.


Repository:
  rG LLVM Github Monorepo

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
@@ -1432,26 +1432,30 @@
 TEST(ParameterHints, ArgPacksAndConstructors) {
   assertParameterHints(
       R"cpp(
-    struct Foo{ Foo(int x); };
+    struct Foo{ Foo(); Foo(int x); };
     void foo(Foo a, int b);
-
     template <typename... Args>
-    void bar(Args... args) { foo(args...); }
-
+    void bar(Args... args) {
+      foo(args...);
+    }
     template <typename... Args>
-    void baz(Args... args) { foo(Foo(args...), args...); }
+    void baz(Args... args) { foo($param1[[Foo{args...}]], $param2[[1]]); }
 
     template <typename... Args>
-    void bax(Args... args) { foo({args...}, args...); }
+    void bax(Args... args) { foo($param3[[{args...}]], args...); }
 
     void foo() {
-      bar($param1[[Foo{2}]], $param2[[42]]);
-      baz($param3[[42]]);
-      bax($param4[[42]]);
+      bar($param4[[Foo{}]], $param5[[42]]);
+      bar($param6[[42]], $param7[[42]]);
+      baz($param8[[42]]);
+      bax($param9[[42]]);
     }
   )cpp",
       ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"},
-      ExpectedHint{"x: ", "param3"}, ExpectedHint{"x: ", "param4"});
+      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:
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() != dyn_cast<ParmVarDecl>(RefArg->getDecl()))
           continue;
         return std::distance(Args.begin(), It);
@@ -871,6 +873,13 @@
   // Maps std::forward(E) to E, identity otherwise.
   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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D130260.446456.patch
Type: text/x-patch
Size: 3550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220721/690b8ea3/attachment-0001.bin>