Skip to content

Commit 665914f

Browse files
[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 clangd/clangd#1807. Based on the original PR at #72345. Co-authored-by: daiyousei-qz <[email protected]>
1 parent 30c4714 commit 665914f

File tree

3 files changed

+138
-44
lines changed

3 files changed

+138
-44
lines changed

clang-tools-extra/clangd/InlayHints.cpp

+22-11
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ std::string summarizeExpr(const Expr *E) {
112112
return getSimpleName(*E->getFoundDecl()).str();
113113
}
114114
std::string VisitCallExpr(const CallExpr *E) {
115-
return Visit(E->getCallee());
115+
std::string Result = Visit(E->getCallee());
116+
Result += E->getNumArgs() == 0 ? "()" : "(...)";
117+
return Result;
116118
}
117119
std::string
118120
VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
@@ -147,6 +149,9 @@ std::string summarizeExpr(const Expr *E) {
147149
}
148150

149151
// Literals are just printed
152+
std::string VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E) {
153+
return "nullptr";
154+
}
150155
std::string VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E) {
151156
return E->getValue() ? "true" : "false";
152157
}
@@ -165,12 +170,14 @@ std::string summarizeExpr(const Expr *E) {
165170
std::string Result = "\"";
166171
if (E->containsNonAscii()) {
167172
Result += "...";
168-
} else if (E->getLength() > 10) {
169-
Result += E->getString().take_front(7);
170-
Result += "...";
171173
} else {
172174
llvm::raw_string_ostream OS(Result);
173-
llvm::printEscapedString(E->getString(), OS);
175+
if (E->getLength() > 10) {
176+
llvm::printEscapedString(E->getString().take_front(7), OS);
177+
Result += "...";
178+
} else {
179+
llvm::printEscapedString(E->getString(), OS);
180+
}
174181
}
175182
Result.push_back('"');
176183
return Result;
@@ -408,12 +415,14 @@ struct Callee {
408415
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
409416
public:
410417
InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST,
411-
const Config &Cfg, std::optional<Range> RestrictRange)
418+
const Config &Cfg, std::optional<Range> RestrictRange,
419+
InlayHintOptions HintOptions)
412420
: Results(Results), AST(AST.getASTContext()), Tokens(AST.getTokens()),
413421
Cfg(Cfg), RestrictRange(std::move(RestrictRange)),
414422
MainFileID(AST.getSourceManager().getMainFileID()),
415423
Resolver(AST.getHeuristicResolver()),
416-
TypeHintPolicy(this->AST.getPrintingPolicy()) {
424+
TypeHintPolicy(this->AST.getPrintingPolicy()),
425+
HintOptions(HintOptions) {
417426
bool Invalid = false;
418427
llvm::StringRef Buf =
419428
AST.getSourceManager().getBufferData(MainFileID, &Invalid);
@@ -1120,7 +1129,6 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
11201129
// Otherwise, the hint shouldn't be shown.
11211130
std::optional<Range> computeBlockEndHintRange(SourceRange BraceRange,
11221131
StringRef OptionalPunctuation) {
1123-
constexpr unsigned HintMinLineLimit = 2;
11241132

11251133
auto &SM = AST.getSourceManager();
11261134
auto [BlockBeginFileId, BlockBeginOffset] =
@@ -1148,7 +1156,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
11481156
auto RBraceLine = SM.getLineNumber(RBraceFileId, RBraceOffset);
11491157

11501158
// Don't show hint on trivial blocks like `class X {};`
1151-
if (BlockBeginLine + HintMinLineLimit - 1 > RBraceLine)
1159+
if (BlockBeginLine + HintOptions.HintMinLineLimit - 1 > RBraceLine)
11521160
return std::nullopt;
11531161

11541162
// This is what we attach the hint to, usually "}" or "};".
@@ -1178,17 +1186,20 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
11781186
StringRef MainFileBuf;
11791187
const HeuristicResolver *Resolver;
11801188
PrintingPolicy TypeHintPolicy;
1189+
InlayHintOptions HintOptions;
11811190
};
11821191

11831192
} // namespace
11841193

11851194
std::vector<InlayHint> inlayHints(ParsedAST &AST,
1186-
std::optional<Range> RestrictRange) {
1195+
std::optional<Range> RestrictRange,
1196+
InlayHintOptions HintOptions) {
11871197
std::vector<InlayHint> Results;
11881198
const auto &Cfg = Config::current();
11891199
if (!Cfg.InlayHints.Enabled)
11901200
return Results;
1191-
InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange));
1201+
InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange),
1202+
HintOptions);
11921203
Visitor.TraverseAST(AST.getASTContext());
11931204

11941205
// De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit

clang-tools-extra/clangd/InlayHints.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,17 @@ namespace clang {
2222
namespace clangd {
2323
class ParsedAST;
2424

25+
struct InlayHintOptions {
26+
// Minimum height of a code block in lines for a BlockEnd hint to be shown
27+
// Includes the lines containing the braces
28+
int HintMinLineLimit = 10;
29+
};
30+
2531
/// Compute and return inlay hints for a file.
2632
/// If RestrictRange is set, return only hints whose location is in that range.
2733
std::vector<InlayHint> inlayHints(ParsedAST &AST,
28-
std::optional<Range> RestrictRange);
34+
std::optional<Range> RestrictRange,
35+
InlayHintOptions HintOptions = {});
2936

3037
} // namespace clangd
3138
} // namespace clang

0 commit comments

Comments
 (0)