[clang-tools-extra] 665914f - [clangd] Improve `BlockEnd` inlayhint presentation (#136106)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 22 23:36:52 PDT 2025
Author: Mythreya
Date: 2025-04-23T02:36:48-04:00
New Revision: 665914fea1433409015a87fef2837218bcd21460
URL: https://github.com/llvm/llvm-project/commit/665914fea1433409015a87fef2837218bcd21460
DIFF: https://github.com/llvm/llvm-project/commit/665914fea1433409015a87fef2837218bcd21460.diff
LOG: [clangd] Improve `BlockEnd` inlayhint presentation (#136106)
* Only show for blocks 10 lines or taller (including braces)
* Add parens for function call: "// if foo" -> "// if foo()" or "// if foo(...)"
* Print literal nullptr
* Escaping for abbreviated strings
Fixes https://github.com/clangd/clangd/issues/1807.
Based on the original PR at https://github.com/llvm/llvm-project/pull/72345.
Co-authored-by: daiyousei-qz <qyzheng2 at outlook.com>
Added:
Modified:
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/InlayHints.h
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 40a824618f782..bdab2b8a9f377 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;
@@ -408,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);
@@ -1120,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 = 2;
auto &SM = AST.getSourceManager();
auto [BlockBeginFileId, BlockBeginOffset] =
@@ -1148,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 "};".
@@ -1178,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..612434b728456 100644
--- a/clang-tools-extra/clangd/InlayHints.h
+++ b/clang-tools-extra/clangd/InlayHints.h
@@ -22,10 +22,17 @@ namespace clang {
namespace clangd {
class ParsedAST;
+struct InlayHintOptions {
+ // Minimum height of a code block in lines for a BlockEnd hint to be shown
+ // Includes the lines containing the braces
+ int HintMinLineLimit = 10;
+};
+
/// 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
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 030e499577706..c3331d20730f1 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -36,9 +36,12 @@ namespace {
using ::testing::ElementsAre;
using ::testing::IsEmpty;
-std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) {
+constexpr InlayHintOptions DefaultOptsForTests{2};
+
+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);
}
@@ -90,7 +93,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());
@@ -98,18 +101,18 @@ 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.
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 +123,16 @@ template <typename... ExpectedHints>
void assertParameterHints(llvm::StringRef AnnotatedSource,
ExpectedHints... Expected) {
ignore(Expected.Side = Left...);
- assertHints(InlayHintKind::Parameter, AnnotatedSource, Expected...);
+ assertHints(InlayHintKind::Parameter, AnnotatedSource, DefaultOptsForTests,
+ Expected...);
}
template <typename... ExpectedHints>
void assertTypeHints(llvm::StringRef AnnotatedSource,
ExpectedHints... Expected) {
ignore(Expected.Side = Right...);
- assertHints(InlayHintKind::Type, AnnotatedSource, Expected...);
+ assertHints(InlayHintKind::Type, AnnotatedSource, DefaultOptsForTests,
+ Expected...);
}
template <typename... ExpectedHints>
@@ -136,16 +141,25 @@ 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, DefaultOptsForTests,
+ Expected...);
}
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, Expected...);
+ assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Opts, Expected...);
+}
+
+template <typename... ExpectedHints>
+void assertBlockEndHints(llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+ assertBlockEndHintsWithOpts(AnnotatedSource, DefaultOptsForTests,
+ Expected...);
}
TEST(ParameterHints, Smoke) {
@@ -1226,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, DefaultOptsForTests).size(),
+ 0u);
}
TEST(TypeHints, Smoke) {
@@ -1488,12 +1504,12 @@ TEST(DefaultArguments, Smoke) {
void baz(int = 5) { if (false) baz($unnamed[[)]]; };
)cpp";
- assertHints(InlayHintKind::DefaultArgument, Code,
+ assertHints(InlayHintKind::DefaultArgument, Code, DefaultOptsForTests,
ExpectedHint{"A: 4", "default1", Left},
ExpectedHint{", B: 1, C: foo()", "default2", Left},
ExpectedHint{"5", "unnamed", Left});
- assertHints(InlayHintKind::Parameter, Code,
+ assertHints(InlayHintKind::Parameter, Code, DefaultOptsForTests,
ExpectedHint{"A: ", "explicit", Left});
}
@@ -1528,14 +1544,14 @@ TEST(DefaultArguments, WithoutParameterNames) {
}
)cpp";
- assertHints(InlayHintKind::DefaultArgument, Code,
+ assertHints(InlayHintKind::DefaultArgument, Code, DefaultOptsForTests,
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, DefaultOptsForTests);
}
TEST(TypeHints, Deduplication) {
@@ -1573,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, DefaultOptsForTests),
+ IsEmpty());
}
TEST(TypeHints, CallingConvention) {
@@ -1590,7 +1607,7 @@ TEST(TypeHints, CallingConvention) {
auto AST = TU.build();
EXPECT_THAT(
- hintsOfKind(AST, InlayHintKind::Type),
+ hintsOfKind(AST, InlayHintKind::Type, DefaultOptsForTests),
ElementsAre(HintMatcher(ExpectedHint{"-> void", "lambda"}, Source)));
}
@@ -1673,7 +1690,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
)cpp";
assertHintsWithHeader(
- InlayHintKind::Type, VectorIntPtr, Header,
+ InlayHintKind::Type, VectorIntPtr, Header, DefaultOptsForTests,
ExpectedHint{": int *", "no_modifier"},
ExpectedHint{": int **", "ptr_modifier"},
ExpectedHint{": int *&", "ref_modifier"},
@@ -1697,7 +1714,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
)cpp";
assertHintsWithHeader(
- InlayHintKind::Type, VectorInt, Header,
+ InlayHintKind::Type, VectorInt, Header, DefaultOptsForTests,
ExpectedHint{": int", "no_modifier"},
ExpectedHint{": int *", "ptr_modifier"},
ExpectedHint{": int &", "ref_modifier"},
@@ -1724,6 +1741,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
)cpp";
assertHintsWithHeader(InlayHintKind::Type, TypeAlias, Header,
+ DefaultOptsForTests,
ExpectedHint{": Short", "short_name"},
ExpectedHint{": static_vector<int>", "vector_name"});
}
@@ -2016,6 +2034,7 @@ TEST(BlockEndHints, If) {
assertBlockEndHints(
R"cpp(
void foo(bool cond) {
+ void* ptr;
if (cond)
;
@@ -2041,13 +2060,17 @@ TEST(BlockEndHints, If) {
if (int i = 0; i > 10) {
$init_cond[[}]]
+
+ if (ptr != nullptr) {
+ $null_check[[}]]
} // suppress
)cpp",
ExpectedHint{" // if cond", "simple"},
ExpectedHint{" // if cond", "ifelse"}, ExpectedHint{" // if", "elseif"},
ExpectedHint{" // if !cond", "inner"},
ExpectedHint{" // if cond", "outer"}, ExpectedHint{" // if X", "init"},
- ExpectedHint{" // if i > 10", "init_cond"});
+ ExpectedHint{" // if i > 10", "init_cond"},
+ ExpectedHint{" // if ptr != nullptr", "null_check"});
}
TEST(BlockEndHints, Loops) {
@@ -2124,30 +2147,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) {
@@ -2307,7 +2341,49 @@ TEST(BlockEndHints, PointerToMemberFunction) {
$ptrmem[[}]]
} // suppress
)cpp",
- ExpectedHint{" // if", "ptrmem"});
+ ExpectedHint{" // if ()", "ptrmem"});
+}
+
+TEST(BlockEndHints, MinLineLimit) {
+ InlayHintOptions Opts;
+ Opts.HintMinLineLimit = 10;
+
+ // namespace ns below is exactly 10 lines
+ 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;
+ };
+ $namespace[[}]]
+ void foo() {
+ int int_a {};
+ while (ns::Var) {
+ }
+
+ while (ns::func1()) {
+ }
+
+ while (ns::func2(int_a, int_a)) {
+ }
+
+ while (ns::S{}.Field) {
+ }
+
+ while (ns::S{}.method1()) {
+ }
+
+ while (ns::S{}.method2(int_a, int_a)) {
+ }
+ $foo[[}]]
+ )cpp",
+ Opts, ExpectedHint{" // namespace ns", "namespace"},
+ ExpectedHint{" // foo", "foo"});
}
// FIXME: Low-hanging fruit where we could omit a type hint:
More information about the cfe-commits
mailing list