[PATCH] D158926: [clangd] Show parameter hints for operator()
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcbd6ac6165e6: [clangd] Show parameter hints for operator()
(authored by zyounan).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158926/new/
https://reviews.llvm.org/D158926
Files:
clang-tools-extra/clangd/InlayHints.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
@@ -89,7 +89,7 @@
ExpectedHints... Expected) {
Annotations Source(AnnotatedSource);
TestTU TU = TestTU::withCode(Source.code());
- TU.ExtraArgs.push_back("-std=c++20");
+ TU.ExtraArgs.push_back("-std=c++23");
TU.HeaderCode = HeaderContent;
auto AST = TU.build();
@@ -807,6 +807,42 @@
)cpp");
}
+TEST(ParameterHints, FunctionCallOperator) {
+ assertParameterHints(R"cpp(
+struct W {
+ void operator()(int x);
+};
+struct S : W {
+ using W::operator();
+ static void operator()(int x, int y);
+};
+void bar() {
+ auto l1 = [](int x) {};
+ auto l2 = [](int x) static {};
+
+ S s;
+ s($1[[1]]);
+ s.operator()($2[[1]]);
+ s.operator()($3[[1]], $4[[2]]);
+ S::operator()($5[[1]], $6[[2]]);
+
+ l1($7[[1]]);
+ l1.operator()($8[[1]]);
+ l2($9[[1]]);
+ l2.operator()($10[[1]]);
+
+ void (*ptr)(int a, int b) = &S::operator();
+ ptr($11[[1]], $12[[2]]);
+}
+ )cpp",
+ ExpectedHint{"x: ", "1"}, ExpectedHint{"x: ", "2"},
+ ExpectedHint{"x: ", "3"}, ExpectedHint{"y: ", "4"},
+ ExpectedHint{"x: ", "5"}, ExpectedHint{"y: ", "6"},
+ ExpectedHint{"x: ", "7"}, ExpectedHint{"x: ", "8"},
+ ExpectedHint{"x: ", "9"}, ExpectedHint{"x: ", "10"},
+ ExpectedHint{"a: ", "11"}, ExpectedHint{"b: ", "12"});
+}
+
TEST(ParameterHints, Macros) {
// Handling of macros depends on where the call's argument list comes from.
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -586,11 +586,13 @@
if (!Cfg.InlayHints.Parameters)
return true;
-// Do not show parameter hints for operator calls written using operator
-// syntax or user-defined literals. (Among other reasons, the resulting
+bool IsFunctor = isFunctionObjectCallExpr(E);
+// Do not show parameter hints for user-defined literals or
+// operator calls except for operator(). (Among other reasons, the resulting
// hints can look awkward, e.g. the expression can itself be a function
// argument and then we'd get two hints side by side).
-if (isa(E) || isa(E))
+if ((isa(E) && !IsFunctor) ||
+isa(E))
return true;
auto CalleeDecls = Resolver->resolveCalleeOfCallExpr(E);
@@ -607,7 +609,22 @@
else
return true;
-processCall(Callee, {E->getArgs(), E->getNumArgs()});
+// N4868 [over.call.object]p3 says,
+// The argument list submitted to overload resolution consists of the
+// 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.
+llvm::ArrayRef 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(Callee.Decl);
+ Method && Method->isInstance())
+Args = Args.drop_front(1);
+processCall(Callee, Args);
return true;
}
@@ -1203,6 +1220,12 @@
return Range{HintStart, HintEnd};
}
+ static bool isFunctionObjectCallExpr(CallExpr *E) noexcept {
+if (auto *CallExpr = dyn_cast(E))
+ return CallExpr->getOperator() == OverloadedOperatorKind::OO_Call;
+return false;
+ }
+
std::vector &Results;
ASTContext *
const syntax::TokenBuffer &Tokens;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158926: [clangd] Show parameter hints for operator()
zyounan updated this revision to Diff 556358.
zyounan marked an inline comment as done.
zyounan added a comment.
Address a nit comment
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158926/new/
https://reviews.llvm.org/D158926
Files:
clang-tools-extra/clangd/InlayHints.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
@@ -89,7 +89,7 @@
ExpectedHints... Expected) {
Annotations Source(AnnotatedSource);
TestTU TU = TestTU::withCode(Source.code());
- TU.ExtraArgs.push_back("-std=c++20");
+ TU.ExtraArgs.push_back("-std=c++23");
TU.HeaderCode = HeaderContent;
auto AST = TU.build();
@@ -807,6 +807,42 @@
)cpp");
}
+TEST(ParameterHints, FunctionCallOperator) {
+ assertParameterHints(R"cpp(
+struct W {
+ void operator()(int x);
+};
+struct S : W {
+ using W::operator();
+ static void operator()(int x, int y);
+};
+void bar() {
+ auto l1 = [](int x) {};
+ auto l2 = [](int x) static {};
+
+ S s;
+ s($1[[1]]);
+ s.operator()($2[[1]]);
+ s.operator()($3[[1]], $4[[2]]);
+ S::operator()($5[[1]], $6[[2]]);
+
+ l1($7[[1]]);
+ l1.operator()($8[[1]]);
+ l2($9[[1]]);
+ l2.operator()($10[[1]]);
+
+ void (*ptr)(int a, int b) = &S::operator();
+ ptr($11[[1]], $12[[2]]);
+}
+ )cpp",
+ ExpectedHint{"x: ", "1"}, ExpectedHint{"x: ", "2"},
+ ExpectedHint{"x: ", "3"}, ExpectedHint{"y: ", "4"},
+ ExpectedHint{"x: ", "5"}, ExpectedHint{"y: ", "6"},
+ ExpectedHint{"x: ", "7"}, ExpectedHint{"x: ", "8"},
+ ExpectedHint{"x: ", "9"}, ExpectedHint{"x: ", "10"},
+ ExpectedHint{"a: ", "11"}, ExpectedHint{"b: ", "12"});
+}
+
TEST(ParameterHints, Macros) {
// Handling of macros depends on where the call's argument list comes from.
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -586,11 +586,13 @@
if (!Cfg.InlayHints.Parameters)
return true;
-// Do not show parameter hints for operator calls written using operator
-// syntax or user-defined literals. (Among other reasons, the resulting
+bool IsFunctor = isFunctionObjectCallExpr(E);
+// Do not show parameter hints for user-defined literals or
+// operator calls except for operator(). (Among other reasons, the resulting
// hints can look awkward, e.g. the expression can itself be a function
// argument and then we'd get two hints side by side).
-if (isa(E) || isa(E))
+if ((isa(E) && !IsFunctor) ||
+isa(E))
return true;
auto CalleeDecls = Resolver->resolveCalleeOfCallExpr(E);
@@ -607,7 +609,22 @@
else
return true;
-processCall(Callee, {E->getArgs(), E->getNumArgs()});
+// N4868 [over.call.object]p3 says,
+// The argument list submitted to overload resolution consists of the
+// 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.
+llvm::ArrayRef 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(Callee.Decl);
+ Method && Method->isInstance())
+Args = Args.drop_front(1);
+processCall(Callee, Args);
return true;
}
@@ -1203,6 +1220,12 @@
return Range{HintStart, HintEnd};
}
+ static bool isFunctionObjectCallExpr(CallExpr *E) noexcept {
+if (auto *CallExpr = dyn_cast(E))
+ return CallExpr->getOperator() == OverloadedOperatorKind::OO_Call;
+return false;
+ }
+
std::vector &Results;
ASTContext *
const syntax::TokenBuffer &Tokens;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158926: [clangd] Show parameter hints for operator()
zyounan marked 2 inline comments as done. zyounan added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:626 + Method && Method->isInstance()) +Args = Args.drop_front(1); +processCall(Callee, Args); nridge wrote: > Huh, that's an interesting inconsistency between CXXMemberCallExpr and > CXXOperatorCallExpr (that one include th implied object argument in getArgs() > and the other doesn't) > > As always, thank you for writing thorough tests that give us confidence we're > doing the right thing :) I'm glad to hear that! Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158926/new/ https://reviews.llvm.org/D158926 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158926: [clangd] Show parameter hints for operator()
nridge accepted this revision.
nridge added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/InlayHints.cpp:626
+ Method && Method->isInstance())
+Args = Args.drop_front(1);
+processCall(Callee, Args);
Huh, that's an interesting inconsistency between CXXMemberCallExpr and
CXXOperatorCallExpr (that one include th implied object argument in getArgs()
and the other doesn't)
As always, thank you for writing thorough tests that give us confidence we're
doing the right thing :)
Comment at: clang-tools-extra/clangd/InlayHints.cpp:1222
+ bool isFunctionObjectCallExpr(CallExpr *E) const noexcept {
+if (auto *CallExpr = dyn_cast(E))
nit: this method can be static
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158926/new/
https://reviews.llvm.org/D158926
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D158926: [clangd] Show parameter hints for operator()
zyounan created this revision.
zyounan added a reviewer: nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zyounan requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Closes https://github.com/clangd/clangd/issues/1742
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158926
Files:
clang-tools-extra/clangd/InlayHints.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
@@ -89,7 +89,7 @@
ExpectedHints... Expected) {
Annotations Source(AnnotatedSource);
TestTU TU = TestTU::withCode(Source.code());
- TU.ExtraArgs.push_back("-std=c++20");
+ TU.ExtraArgs.push_back("-std=c++23");
TU.HeaderCode = HeaderContent;
auto AST = TU.build();
@@ -807,6 +807,42 @@
)cpp");
}
+TEST(ParameterHints, FunctionCallOperator) {
+ assertParameterHints(R"cpp(
+struct W {
+ void operator()(int x);
+};
+struct S : W {
+ using W::operator();
+ static void operator()(int x, int y);
+};
+void bar() {
+ auto l1 = [](int x) {};
+ auto l2 = [](int x) static {};
+
+ S s;
+ s($1[[1]]);
+ s.operator()($2[[1]]);
+ s.operator()($3[[1]], $4[[2]]);
+ S::operator()($5[[1]], $6[[2]]);
+
+ l1($7[[1]]);
+ l1.operator()($8[[1]]);
+ l2($9[[1]]);
+ l2.operator()($10[[1]]);
+
+ void (*ptr)(int a, int b) = &S::operator();
+ ptr($11[[1]], $12[[2]]);
+}
+ )cpp",
+ ExpectedHint{"x: ", "1"}, ExpectedHint{"x: ", "2"},
+ ExpectedHint{"x: ", "3"}, ExpectedHint{"y: ", "4"},
+ ExpectedHint{"x: ", "5"}, ExpectedHint{"y: ", "6"},
+ ExpectedHint{"x: ", "7"}, ExpectedHint{"x: ", "8"},
+ ExpectedHint{"x: ", "9"}, ExpectedHint{"x: ", "10"},
+ ExpectedHint{"a: ", "11"}, ExpectedHint{"b: ", "12"});
+}
+
TEST(ParameterHints, Macros) {
// Handling of macros depends on where the call's argument list comes from.
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -586,11 +586,13 @@
if (!Cfg.InlayHints.Parameters)
return true;
-// Do not show parameter hints for operator calls written using operator
-// syntax or user-defined literals. (Among other reasons, the resulting
+bool IsFunctor = isFunctionObjectCallExpr(E);
+// Do not show parameter hints for user-defined literals or
+// operator calls except for operator(). (Among other reasons, the resulting
// hints can look awkward, e.g. the expression can itself be a function
// argument and then we'd get two hints side by side).
-if (isa(E) || isa(E))
+if ((isa(E) && !IsFunctor) ||
+isa(E))
return true;
auto CalleeDecls = Resolver->resolveCalleeOfCallExpr(E);
@@ -607,7 +609,22 @@
else
return true;
-processCall(Callee, {E->getArgs(), E->getNumArgs()});
+// N4868 [over.call.object]p3 says,
+// The argument list submitted to overload resolution consists of the
+// 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.
+llvm::ArrayRef 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(Callee.Decl);
+ Method && Method->isInstance())
+Args = Args.drop_front(1);
+processCall(Callee, Args);
return true;
}
@@ -1202,6 +1219,12 @@
return Range{HintStart, HintEnd};
}
+ bool isFunctionObjectCallExpr(CallExpr *E) const noexcept {
+if (auto *CallExpr = dyn_cast(E))
+ return CallExpr->getOperator() == OverloadedOperatorKind::OO_Call;
+return false;
+ }
+
std::vector &Results;
ASTContext *
const syntax::TokenBuffer &Tokens;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
