[clangd] Adapt Inlay Hint support for Deducing This

[zyn0217]

This is a follow-up for D140828, making Clangd omit the explicit object parameter in a call to member function with Deducing This.

Given that the parent patch is still in its infancy and might undergo several reverting-relanding processes, one can feel free to revert this if encountering any CI failure. And please let me know if I should alter anything.

[llvmbot]

@llvm/pr-subscribers-clangd

Changes

This is a follow-up for D140828, making Clangd omit the explicit object parameter in a call to member function with Deducing This.

Given that the parent patch is still in its infancy and might undergo several reverting-relanding processes, one can feel free to revert this if encountering any CI failure. And please let me know if I should alter anything.

---

Full diff: https://github.com/llvm/llvm-project/pull/68177.diff

2 Files Affected:

• (modified) clang-tools-extra/clangd/InlayHints.cpp (+28-13)
• (modified) clang-tools-extra/clangd/unittests/InlayHintTests.cpp (+28)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index e6e5e11b889bff8..590261d98a0474f 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -528,6 +528,13 @@ static FunctionProtoTypeLoc getPrototypeLoc(Expr *Fn) {
   return {};
 }
 
+ArrayRef<const ParmVarDecl *>
+maybeDropCxxExplicitObjectParameters(ArrayRef<const ParmVarDecl *> Params) {
+  if (!Params.empty() && Params.front()->isExplicitObjectParameter())
+    Params = Params.drop_front(1);
+  return Params;
+}
+
 struct Callee {
   // Only one of Decl or Loc is set.
   // Loc is for calls through function pointers.
@@ -614,15 +621,21 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     // argument expressions present in the function call syntax preceded by the
     // implied object argument (E).
     //
-    // However, we don't have the implied object argument for static
-    // operator() per clang::Sema::BuildCallToObjectOfClassType.
+    // As well as the provision from P0847R7 Deducing This [expr.call]p7:
+    // ...If the function is an explicit object member function and there is an
+    // implied object argument ([over.call.func]), the list of provided
+    // arguments is preceded by the implied object argument for the purposes of
+    // this correspondence...
+    //
+    // However, we don't have the implied object argument
+    // for static operator() per clang::Sema::BuildCallToObjectOfClassType.
     llvm::ArrayRef<const Expr *> Args = {E->getArgs(), E->getNumArgs()};
-    if (IsFunctor)
-      // We don't have the implied object argument through
-      // a function pointer either.
-      if (const CXXMethodDecl *Method =
-              dyn_cast_or_null<CXXMethodDecl>(Callee.Decl);
-          Method && Method->isInstance())
+    // We don't have the implied object argument through a function pointer
+    // either.
+    if (const CXXMethodDecl *Method =
+            dyn_cast_or_null<CXXMethodDecl>(Callee.Decl))
+      if (Method->isInstance() &&
+          (IsFunctor || Method->hasCXXExplicitFunctionObjectParameter()))
         Args = Args.drop_front(1);
     processCall(Callee, Args);
     return true;
@@ -849,8 +862,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
         if (Ctor->isCopyOrMoveConstructor())
           return;
 
-    auto Params =
-        Callee.Decl ? Callee.Decl->parameters() : Callee.Loc.getParams();
+    auto Params = maybeDropCxxExplicitObjectParameters(
+        Callee.Decl ? Callee.Decl->parameters() : Callee.Loc.getParams());
 
     // Resolve parameter packs to their forwarded parameter
     SmallVector<const ParmVarDecl *> ForwardedParams;
@@ -859,7 +872,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     else
       ForwardedParams = {Params.begin(), Params.end()};
 
-    NameVec ParameterNames = chooseParameterNames(ForwardedParams);
+    auto ForwardedParamsRef = maybeDropCxxExplicitObjectParameters(ForwardedParams);
+    NameVec ParameterNames = chooseParameterNames(ForwardedParamsRef);
 
     // Exclude setters (i.e. functions with one argument whose name begins with
     // "set"), and builtins like std::move/forward/... as their parameter name
@@ -878,7 +892,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 
       StringRef Name = ParameterNames[I];
       bool NameHint = shouldHintName(Args[I], Name);
-      bool ReferenceHint = shouldHintReference(Params[I], ForwardedParams[I]);
+      bool ReferenceHint =
+          shouldHintReference(Params[I], ForwardedParamsRef[I]);
 
       if (NameHint || ReferenceHint) {
         addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
@@ -1018,7 +1033,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     return {};
   }
 
-  NameVec chooseParameterNames(SmallVector<const ParmVarDecl *> Parameters) {
+  NameVec chooseParameterNames(ArrayRef<const ParmVarDecl *> Parameters) {
     NameVec ParameterNames;
     for (const auto *P : Parameters) {
       if (isExpandedFromParameterPack(P)) {
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index a8c3546eb80cc85..20c1cdd985dbc01 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -843,6 +843,34 @@ TEST(ParameterHints, FunctionCallOperator) {
                        ExpectedHint{"a: ", "11"}, ExpectedHint{"b: ", "12"});
 }
 
+TEST(ParameterHints, DeducingThis) {
+  assertParameterHints(R"cpp(
+    struct S {
+      template <typename This>
+      auto operator()(this This &&Self, int Param) {
+        return 42;
+      }
+
+      auto function(this auto &Self, int Param) {
+        return Param;
+      }
+    };
+    void work() {
+      S s;
+      s($1[[42]]);
+      s.function($2[[42]]);
+      S()($3[[42]]);
+      auto lambda = [](this auto &Self, char C) -> void {
+        return Self(C);
+      };
+      lambda($4[['A']]);
+    }
+  )cpp",
+                       ExpectedHint{"Param: ", "1"},
+                       ExpectedHint{"Param: ", "2"},
+                       ExpectedHint{"Param: ", "3"}, ExpectedHint{"C: ", "4"});
+}
+
 TEST(ParameterHints, Macros) {
   // Handling of macros depends on where the call's argument list comes from.
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[zyn0217]

@HighCommander4 Friendly ping~

[HighCommander4]

Thanks! I have one suggestion for your consideration, otherwise this looks good to me.

[HighCommander4]

What do you think about the following tweak:

SmallVector<const ParamVarDecl *> ForwardedParamsStorage;
ArrayRef<const ParamVarDecl *> ForwardedParams;
if (Callee.Decl) {
  ForwardedParamsStorage = resolveForwardingParameters(Callee.Decl);
  ForwardedParams = maybeDropCxxExplicitObjectParameters(ForwardedParamsStorage);
} else {
  ForwardedParams = Params;
}

[zyn0217]

Thanks! The control flow looks much clearer now. BTW, I'd like to merge the calculation for Param above into this if block, too.