[clang-tools-extra] [clangd] Improve `BlockEnd` inlayhint presentation (PR #136106)
via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 20 18:10:19 PDT 2025
https://github.com/MythreyaK updated https://github.com/llvm/llvm-project/pull/136106
>From a0e3a33eda624bbebd436d6ac97a18348be39e7c Mon Sep 17 00:00:00 2001
From: daiyousei-qz <qyzheng2 at outlook.com>
Date: Tue, 14 Nov 2023 20:42:10 -0800
Subject: [PATCH 1/5] Improve BlockEnd presentation including: 1. Explicitly
state a function call 2. Print literal nullptr 3. Escape for abbreviated
string 4. Adjust min line limit to 10
---
clang-tools-extra/clangd/InlayHints.cpp | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 1b1bcf78c9855..b1e3bd97d4fd9 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -112,7 +112,9 @@ std::string summarizeExpr(const Expr *E) {
return getSimpleName(*E->getFoundDecl()).str();
}
std::string VisitCallExpr(const CallExpr *E) {
- return Visit(E->getCallee());
+ std::string Result = Visit(E->getCallee());
+ Result += E->getNumArgs() == 0 ? "()" : "(...)";
+ return Result;
}
std::string
VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
@@ -147,6 +149,9 @@ std::string summarizeExpr(const Expr *E) {
}
// Literals are just printed
+ std::string VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E) {
+ return "nullptr";
+ }
std::string VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E) {
return E->getValue() ? "true" : "false";
}
@@ -165,12 +170,14 @@ std::string summarizeExpr(const Expr *E) {
std::string Result = "\"";
if (E->containsNonAscii()) {
Result += "...";
- } else if (E->getLength() > 10) {
- Result += E->getString().take_front(7);
- Result += "...";
} else {
llvm::raw_string_ostream OS(Result);
- llvm::printEscapedString(E->getString(), OS);
+ if (E->getLength() > 10) {
+ llvm::printEscapedString(E->getString().take_front(7), OS);
+ Result += "...";
+ } else {
+ llvm::printEscapedString(E->getString(), OS);
+ }
}
Result.push_back('"');
return Result;
@@ -1120,7 +1127,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
// Otherwise, the hint shouldn't be shown.
std::optional<Range> computeBlockEndHintRange(SourceRange BraceRange,
StringRef OptionalPunctuation) {
- constexpr unsigned HintMinLineLimit = 2;
+ constexpr unsigned HintMinLineLimit = 10;
auto &SM = AST.getSourceManager();
auto [BlockBeginFileId, BlockBeginOffset] =
>From 729be505b882f34a29b2e9fc2c1b1adfaf31cc42 Mon Sep 17 00:00:00 2001
From: Mythreya <git at mythreya.dev>
Date: Thu, 17 Apr 2025 01:28:53 -0700
Subject: [PATCH 2/5] Add `InlayHintOptions`
---
clang-tools-extra/clangd/InlayHints.cpp | 16 ++++++++++------
clang-tools-extra/clangd/InlayHints.h | 8 +++++++-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index b1e3bd97d4fd9..298e19d7fe41d 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -415,12 +415,14 @@ struct Callee {
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
public:
InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
- const Config &Cfg, std::optional<Range> RestrictRange)
+ const Config &Cfg, std::optional<Range> RestrictRange,
+ InlayHintOptions HintOptions)
: Results(Results), AST(AST.getASTContext()), Tokens(AST.getTokens()),
Cfg(Cfg), RestrictRange(std::move(RestrictRange)),
MainFileID(AST.getSourceManager().getMainFileID()),
Resolver(AST.getHeuristicResolver()),
- TypeHintPolicy(this->AST.getPrintingPolicy()) {
+ TypeHintPolicy(this->AST.getPrintingPolicy()),
+ HintOptions(HintOptions) {
bool Invalid = false;
llvm::StringRef Buf =
AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -1127,7 +1129,6 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
// Otherwise, the hint shouldn't be shown.
std::optional<Range> computeBlockEndHintRange(SourceRange BraceRange,
StringRef OptionalPunctuation) {
- constexpr unsigned HintMinLineLimit = 10;
auto &SM = AST.getSourceManager();
auto [BlockBeginFileId, BlockBeginOffset] =
@@ -1155,7 +1156,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
auto RBraceLine = SM.getLineNumber(RBraceFileId, RBraceOffset);
// Don't show hint on trivial blocks like `class X {};`
- if (BlockBeginLine + HintMinLineLimit - 1 > RBraceLine)
+ if (BlockBeginLine + HintOptions.HintMinLineLimit - 1 > RBraceLine)
return std::nullopt;
// This is what we attach the hint to, usually "}" or "};".
@@ -1185,17 +1186,20 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
StringRef MainFileBuf;
const HeuristicResolver *Resolver;
PrintingPolicy TypeHintPolicy;
+ InlayHintOptions HintOptions;
};
} // namespace
std::vector<InlayHint> inlayHints(ParsedAST &AST,
- std::optional<Range> RestrictRange) {
+ std::optional<Range> RestrictRange,
+ InlayHintOptions HintOptions) {
std::vector<InlayHint> Results;
const auto &Cfg = Config::current();
if (!Cfg.InlayHints.Enabled)
return Results;
- InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange));
+ InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange),
+ HintOptions);
Visitor.TraverseAST(AST.getASTContext());
// De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit
diff --git a/clang-tools-extra/clangd/InlayHints.h b/clang-tools-extra/clangd/InlayHints.h
index 6a0236a0ab08a..544f3c8c2d03a 100644
--- a/clang-tools-extra/clangd/InlayHints.h
+++ b/clang-tools-extra/clangd/InlayHints.h
@@ -22,10 +22,16 @@ namespace clang {
namespace clangd {
class ParsedAST;
+struct InlayHintOptions {
+ // Minimum lines for BlockEnd inlay-hints to be shown
+ int HintMinLineLimit{2};
+};
+
/// Compute and return inlay hints for a file.
/// If RestrictRange is set, return only hints whose location is in that range.
std::vector<InlayHint> inlayHints(ParsedAST &AST,
- std::optional<Range> RestrictRange);
+ std::optional<Range> RestrictRange,
+ InlayHintOptions HintOptions = {});
} // namespace clangd
} // namespace clang
>From 228fd797eed5dd7f7e1942ac8eba5407f60c85c9 Mon Sep 17 00:00:00 2001
From: Mythreya <git at mythreya.dev>
Date: Fri, 18 Apr 2025 21:23:28 -0700
Subject: [PATCH 3/5] Update tests
---
.../clangd/unittests/InlayHintTests.cpp | 35 +++++++++++--------
1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 77d78b8777fe3..3ae2d6eddc1a5 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -36,6 +36,8 @@ namespace {
using ::testing::ElementsAre;
using ::testing::IsEmpty;
+constexpr InlayHintOptions DefaultInlayHintOpts{};
+
std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
std::vector<InlayHint> Result;
for (auto &Hint : inlayHints(AST, /*RestrictRange=*/std::nullopt)) {
@@ -90,7 +92,7 @@ Config noHintsConfig() {
template <typename... ExpectedHints>
void assertHintsWithHeader(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
- llvm::StringRef HeaderContent,
+ llvm::StringRef HeaderContent, InlayHintOptions Opts,
ExpectedHints... Expected) {
Annotations Source(AnnotatedSource);
TestTU TU = TestTU::withCode(Source.code());
@@ -103,13 +105,13 @@ void assertHintsWithHeader(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
// Sneak in a cross-cutting check that hints are disabled by config.
// We'll hit an assertion failure if addInlayHint still gets called.
WithContextValue WithCfg(Config::Key, noHintsConfig());
- EXPECT_THAT(inlayHints(AST, std::nullopt), IsEmpty());
+ EXPECT_THAT(inlayHints(AST, std::nullopt, Opts), IsEmpty());
}
template <typename... ExpectedHints>
void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
- ExpectedHints... Expected) {
- return assertHintsWithHeader(Kind, AnnotatedSource, "",
+ InlayHintOptions Opts, ExpectedHints... Expected) {
+ return assertHintsWithHeader(Kind, AnnotatedSource, "", Opts,
std::move(Expected)...);
}
@@ -120,14 +122,16 @@ template <typename... ExpectedHints>
void assertParameterHints(llvm::StringRef AnnotatedSource,
ExpectedHints... Expected) {
ignore(Expected.Side = Left...);
- assertHints(InlayHintKind::Parameter, AnnotatedSource, Expected...);
+ assertHints(InlayHintKind::Parameter, AnnotatedSource, DefaultInlayHintOpts,
+ Expected...);
}
template <typename... ExpectedHints>
void assertTypeHints(llvm::StringRef AnnotatedSource,
ExpectedHints... Expected) {
ignore(Expected.Side = Right...);
- assertHints(InlayHintKind::Type, AnnotatedSource, Expected...);
+ assertHints(InlayHintKind::Type, AnnotatedSource, DefaultInlayHintOpts,
+ Expected...);
}
template <typename... ExpectedHints>
@@ -136,7 +140,8 @@ void assertDesignatorHints(llvm::StringRef AnnotatedSource,
Config Cfg;
Cfg.InlayHints.Designators = true;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
- assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...);
+ assertHints(InlayHintKind::Designator, AnnotatedSource, DefaultInlayHintOpts,
+ Expected...);
}
template <typename... ExpectedHints>
@@ -145,7 +150,8 @@ void assertBlockEndHints(llvm::StringRef AnnotatedSource,
Config Cfg;
Cfg.InlayHints.BlockEnd = true;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
- assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Expected...);
+ assertHints(InlayHintKind::BlockEnd, AnnotatedSource, DefaultInlayHintOpts,
+ Expected...);
}
TEST(ParameterHints, Smoke) {
@@ -1488,12 +1494,12 @@ TEST(DefaultArguments, Smoke) {
void baz(int = 5) { if (false) baz($unnamed[[)]]; };
)cpp";
- assertHints(InlayHintKind::DefaultArgument, Code,
+ assertHints(InlayHintKind::DefaultArgument, Code, DefaultInlayHintOpts,
ExpectedHint{"A: 4", "default1", Left},
ExpectedHint{", B: 1, C: foo()", "default2", Left},
ExpectedHint{"5", "unnamed", Left});
- assertHints(InlayHintKind::Parameter, Code,
+ assertHints(InlayHintKind::Parameter, Code, DefaultInlayHintOpts,
ExpectedHint{"A: ", "explicit", Left});
}
@@ -1528,14 +1534,14 @@ TEST(DefaultArguments, WithoutParameterNames) {
}
)cpp";
- assertHints(InlayHintKind::DefaultArgument, Code,
+ assertHints(InlayHintKind::DefaultArgument, Code, DefaultInlayHintOpts,
ExpectedHint{"...", "abbreviated", Left},
ExpectedHint{", Baz{}", "paren", Left},
ExpectedHint{", Baz{}", "brace1", Left},
ExpectedHint{", Baz{}", "brace2", Left},
ExpectedHint{", Baz{}", "brace3", Left});
- assertHints(InlayHintKind::Parameter, Code);
+ assertHints(InlayHintKind::Parameter, Code, DefaultInlayHintOpts);
}
TEST(TypeHints, Deduplication) {
@@ -1671,7 +1677,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
)cpp";
assertHintsWithHeader(
- InlayHintKind::Type, VectorIntPtr, Header,
+ InlayHintKind::Type, VectorIntPtr, Header, DefaultInlayHintOpts,
ExpectedHint{": int *", "no_modifier"},
ExpectedHint{": int **", "ptr_modifier"},
ExpectedHint{": int *&", "ref_modifier"},
@@ -1695,7 +1701,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
)cpp";
assertHintsWithHeader(
- InlayHintKind::Type, VectorInt, Header,
+ InlayHintKind::Type, VectorInt, Header, DefaultInlayHintOpts,
ExpectedHint{": int", "no_modifier"},
ExpectedHint{": int *", "ptr_modifier"},
ExpectedHint{": int &", "ref_modifier"},
@@ -1722,6 +1728,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
)cpp";
assertHintsWithHeader(InlayHintKind::Type, TypeAlias, Header,
+ DefaultInlayHintOpts,
ExpectedHint{": Short", "short_name"},
ExpectedHint{": static_vector<int>", "vector_name"});
}
>From d0449144c866c37b01e0436e4c2f1d893b8b8421 Mon Sep 17 00:00:00 2001
From: Mythreya <git at mythreya.dev>
Date: Sun, 20 Apr 2025 17:52:04 -0700
Subject: [PATCH 4/5] Update tests
---
.../clangd/unittests/InlayHintTests.cpp | 61 +++++++++++++------
1 file changed, 42 insertions(+), 19 deletions(-)
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 3ae2d6eddc1a5..e22e670928c1b 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -38,9 +38,10 @@ using ::testing::IsEmpty;
constexpr InlayHintOptions DefaultInlayHintOpts{};
-std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
+std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind,
+ InlayHintOptions Opts) {
std::vector<InlayHint> Result;
- for (auto &Hint : inlayHints(AST, /*RestrictRange=*/std::nullopt)) {
+ for (auto &Hint : inlayHints(AST, /*RestrictRange=*/std::nullopt, Opts)) {
if (Hint.kind == Kind)
Result.push_back(Hint);
}
@@ -100,7 +101,7 @@ void assertHintsWithHeader(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
TU.HeaderCode = HeaderContent;
auto AST = TU.build();
- EXPECT_THAT(hintsOfKind(AST, Kind),
+ EXPECT_THAT(hintsOfKind(AST, Kind, Opts),
ElementsAre(HintMatcher(Expected, Source)...));
// Sneak in a cross-cutting check that hints are disabled by config.
// We'll hit an assertion failure if addInlayHint still gets called.
@@ -145,13 +146,20 @@ void assertDesignatorHints(llvm::StringRef AnnotatedSource,
}
template <typename... ExpectedHints>
-void assertBlockEndHints(llvm::StringRef AnnotatedSource,
- ExpectedHints... Expected) {
+void assertBlockEndHintsWithOpts(llvm::StringRef AnnotatedSource,
+ InlayHintOptions Opts,
+ ExpectedHints... Expected) {
Config Cfg;
Cfg.InlayHints.BlockEnd = true;
WithContextValue WithCfg(Config::Key, std::move(Cfg));
- assertHints(InlayHintKind::BlockEnd, AnnotatedSource, DefaultInlayHintOpts,
- Expected...);
+ assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Opts, Expected...);
+}
+
+template <typename... ExpectedHints>
+void assertBlockEndHints(llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+ assertBlockEndHintsWithOpts(AnnotatedSource, DefaultInlayHintOpts,
+ Expected...);
}
TEST(ParameterHints, Smoke) {
@@ -1232,7 +1240,9 @@ TEST(ParameterHints, IncludeAtNonGlobalScope) {
ASSERT_TRUE(bool(AST));
// Ensure the hint for the call in foo.inc is NOT materialized in foo.cc.
- EXPECT_EQ(hintsOfKind(*AST, InlayHintKind::Parameter).size(), 0u);
+ EXPECT_EQ(
+ hintsOfKind(*AST, InlayHintKind::Parameter, DefaultInlayHintOpts).size(),
+ 0u);
}
TEST(TypeHints, Smoke) {
@@ -1579,7 +1589,8 @@ TEST(TypeHints, Aliased) {
TU.ExtraArgs.push_back("-xc");
auto AST = TU.build();
- EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type), IsEmpty());
+ EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type, DefaultInlayHintOpts),
+ IsEmpty());
}
TEST(TypeHints, CallingConvention) {
@@ -1595,7 +1606,8 @@ TEST(TypeHints, CallingConvention) {
TU.PredefineMacros = true; // for the __cdecl
auto AST = TU.build();
- EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type), IsEmpty());
+ EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type, DefaultInlayHintOpts),
+ IsEmpty());
}
TEST(TypeHints, Decltype) {
@@ -2129,30 +2141,41 @@ TEST(BlockEndHints, PrintRefs) {
R"cpp(
namespace ns {
int Var;
- int func();
+ int func1();
+ int func2(int, int);
struct S {
int Field;
- int method() const;
+ int method1() const;
+ int method2(int, int) const;
}; // suppress
} // suppress
void foo() {
+ int int_a {};
while (ns::Var) {
$var[[}]]
- while (ns::func()) {
- $func[[}]]
+ while (ns::func1()) {
+ $func1[[}]]
+
+ while (ns::func2(int_a, int_a)) {
+ $func2[[}]]
while (ns::S{}.Field) {
$field[[}]]
- while (ns::S{}.method()) {
- $method[[}]]
+ while (ns::S{}.method1()) {
+ $method1[[}]]
+
+ while (ns::S{}.method2(int_a, int_a)) {
+ $method2[[}]]
} // suppress
)cpp",
ExpectedHint{" // while Var", "var"},
- ExpectedHint{" // while func", "func"},
+ ExpectedHint{" // while func1()", "func1"},
+ ExpectedHint{" // while func2(...)", "func2"},
ExpectedHint{" // while Field", "field"},
- ExpectedHint{" // while method", "method"});
+ ExpectedHint{" // while method1()", "method1"},
+ ExpectedHint{" // while method2(...)", "method2"});
}
TEST(BlockEndHints, PrintConversions) {
@@ -2312,7 +2335,7 @@ TEST(BlockEndHints, PointerToMemberFunction) {
$ptrmem[[}]]
} // suppress
)cpp",
- ExpectedHint{" // if", "ptrmem"});
+ ExpectedHint{" // if ()", "ptrmem"});
}
// FIXME: Low-hanging fruit where we could omit a type hint:
>From 01e830f74074bbc78e31be257602279a8336c7ee Mon Sep 17 00:00:00 2001
From: Mythreya <git at mythreya.dev>
Date: Sun, 20 Apr 2025 18:08:32 -0700
Subject: [PATCH 5/5] Add BlockEndHints.MinLineLimit test
Add a new test `MinLineLimit` that ensures hints
are generated only when the line threshold is met.
Limit is set through `InlayHintOptions.HintMinLineLimit`
---
.../clangd/unittests/InlayHintTests.cpp | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index e22e670928c1b..bef60f0306f5f 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -2338,6 +2338,44 @@ TEST(BlockEndHints, PointerToMemberFunction) {
ExpectedHint{" // if ()", "ptrmem"});
}
+TEST(BlockEndHints, MinLineLimit) {
+ assertBlockEndHintsWithOpts(
+ R"cpp(
+ namespace ns {
+ int Var;
+ int func1();
+ int func2(int, int);
+ struct S {
+ int Field;
+ int method1() const;
+ int method2(int, int) const;
+ $struct[[}]];
+ $namespace[[}]]
+ void foo() {
+ int int_a {};
+ while (ns::Var) {
+ $var[[}]]
+
+ while (ns::func1()) {
+ $func1[[}]]
+
+ while (ns::func2(int_a, int_a)) {
+ $func2[[}]]
+
+ while (ns::S{}.Field) {
+ $field[[}]]
+
+ while (ns::S{}.method1()) {
+ $method1[[}]]
+
+ while (ns::S{}.method2(int_a, int_a)) {
+ $method2[[}]]
+ $foo[[}]]
+ )cpp",
+ InlayHintOptions{10}, ExpectedHint{" // namespace ns", "namespace"},
+ ExpectedHint{" // foo", "foo"});
+}
+
// FIXME: Low-hanging fruit where we could omit a type hint:
// - auto x = TypeName(...);
// - auto x = (TypeName) (...);
More information about the cfe-commits
mailing list