[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