[clang-tools-extra] [clangd] Adapt Inlay Hint support for Deducing This (PR #68177)

Younan Zhang via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 23 07:20:23 PDT 2023


https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/68177

>From f83f28d0d055066bb7660e24e2253a61273f014a Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 4 Oct 2023 11:59:31 +0800
Subject: [PATCH 1/3] [clangd] Adapt Inlay Hint support for Deducing This

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.
---
 clang-tools-extra/clangd/InlayHints.cpp       | 41 +++++++++++++------
 .../clangd/unittests/InlayHintTests.cpp       | 28 +++++++++++++
 2 files changed, 56 insertions(+), 13 deletions(-)

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.
 

>From 1f287c721aa82f87ec6b90c2146dae0468ca7630 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 4 Oct 2023 12:31:34 +0800
Subject: [PATCH 2/3] Format

---
 clang-tools-extra/clangd/InlayHints.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 590261d98a0474f..379e187c424a021 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -872,7 +872,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
     else
       ForwardedParams = {Params.begin(), Params.end()};
 
-    auto ForwardedParamsRef = maybeDropCxxExplicitObjectParameters(ForwardedParams);
+    auto ForwardedParamsRef =
+        maybeDropCxxExplicitObjectParameters(ForwardedParams);
     NameVec ParameterNames = chooseParameterNames(ForwardedParamsRef);
 
     // Exclude setters (i.e. functions with one argument whose name begins with

>From fde9d8e83d6d0f326074d3abca1fa1ede16fdd2c Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Mon, 23 Oct 2023 22:10:09 +0800
Subject: [PATCH 3/3] Rearrange the control flow

---
 clang-tools-extra/clangd/InlayHints.cpp | 26 ++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 379e187c424a021..d034e9181a62b94 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -862,19 +862,20 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
         if (Ctor->isCopyOrMoveConstructor())
           return;
 
-    auto Params = maybeDropCxxExplicitObjectParameters(
-        Callee.Decl ? Callee.Decl->parameters() : Callee.Loc.getParams());
-
+    ArrayRef<const ParmVarDecl *> Params, ForwardedParams;
     // Resolve parameter packs to their forwarded parameter
-    SmallVector<const ParmVarDecl *> ForwardedParams;
-    if (Callee.Decl)
-      ForwardedParams = resolveForwardingParameters(Callee.Decl);
-    else
-      ForwardedParams = {Params.begin(), Params.end()};
+    SmallVector<const ParmVarDecl *> ForwardedParamsStorage;
+    if (Callee.Decl) {
+      Params = maybeDropCxxExplicitObjectParameters(Callee.Decl->parameters());
+      ForwardedParamsStorage = resolveForwardingParameters(Callee.Decl);
+      ForwardedParams =
+          maybeDropCxxExplicitObjectParameters(ForwardedParamsStorage);
+    } else {
+      Params = maybeDropCxxExplicitObjectParameters(Callee.Loc.getParams());
+      ForwardedParamsStorage = {Params.begin(), Params.end()};
+    }
 
-    auto ForwardedParamsRef =
-        maybeDropCxxExplicitObjectParameters(ForwardedParams);
-    NameVec ParameterNames = chooseParameterNames(ForwardedParamsRef);
+    NameVec ParameterNames = chooseParameterNames(ForwardedParams);
 
     // Exclude setters (i.e. functions with one argument whose name begins with
     // "set"), and builtins like std::move/forward/... as their parameter name
@@ -893,8 +894,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
 
       StringRef Name = ParameterNames[I];
       bool NameHint = shouldHintName(Args[I], Name);
-      bool ReferenceHint =
-          shouldHintReference(Params[I], ForwardedParamsRef[I]);
+      bool ReferenceHint = shouldHintReference(Params[I], ForwardedParams[I]);
 
       if (NameHint || ReferenceHint) {
         addInlayHint(Args[I]->getSourceRange(), HintSide::Left,



More information about the cfe-commits mailing list