[clang-tools-extra] 9e6a342 - [clangd] Implement end-definition-comment inlay hints
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 15 16:44:42 PDT 2023
Author: daiyousei-qz
Date: 2023-07-16T01:19:02+02:00
New Revision: 9e6a342fdac8978ef6d3e373cbbc7425e3dfe0f8
URL: https://github.com/llvm/llvm-project/commit/9e6a342fdac8978ef6d3e373cbbc7425e3dfe0f8
DIFF: https://github.com/llvm/llvm-project/commit/9e6a342fdac8978ef6d3e373cbbc7425e3dfe0f8.diff
LOG: [clangd] Implement end-definition-comment inlay hints
This patch implements a new inlay hint feature proposed in https://github.com/clangd/clangd/issues/1634. It introduces a new inlay hint kind BlockEnd which shows a comment-like hint after a definition brace pair, including function/type/namespace. For example,
```
void foo() {
} ^
```
In the code shown above, a hint should be displayed at ^ labelling `// foo`. Such hint only shows when there's no trailing character after the position except whitespaces and optionally ';'.
Also, new configurations are introduced in the inlay hints block
```
InlayHints:
BlockEnd: Yes # toggling the feature
```
Reviewed By: sammccall
Differential Revision: https://reviews.llvm.org/D150635
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index b46fedac3f88e9..daae8d1c0c833e 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -144,6 +144,7 @@ struct Config {
bool Parameters = true;
bool DeducedTypes = true;
bool Designators = true;
+ bool BlockEnd = false;
// Limit the length of type names in inlay hints. (0 means no limit)
uint32_t TypeNameLimit = 32;
} InlayHints;
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 64d65399072333..d4ff7ae3181bc3 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -606,6 +606,10 @@ struct FragmentCompiler {
Out.Apply.push_back([Value(**F.Designators)](const Params &, Config &C) {
C.InlayHints.Designators = Value;
});
+ if (F.BlockEnd)
+ Out.Apply.push_back([Value(**F.BlockEnd)](const Params &, Config &C) {
+ C.InlayHints.BlockEnd = Value;
+ });
if (F.TypeNameLimit)
Out.Apply.push_back(
[Value(**F.TypeNameLimit)](const Params &, Config &C) {
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 0300f7a5cb73f2..a59d4124b0828a 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -318,6 +318,8 @@ struct Fragment {
std::optional<Located<bool>> DeducedTypes;
/// Show designators in aggregate initialization.
std::optional<Located<bool>> Designators;
+ /// Show defined symbol names at the end of a definition block.
+ std::optional<Located<bool>> BlockEnd;
/// Limit the length of type name hints. (0 means no limit)
std::optional<Located<uint32_t>> TypeNameLimit;
};
diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
index 371c9d3ed4d525..cf3cec472bf8a3 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -252,6 +252,10 @@ class Parser {
if (auto Value = boolValue(N, "Designators"))
F.Designators = *Value;
});
+ Dict.handle("BlockEnd", [&](Node &N) {
+ if (auto Value = boolValue(N, "BlockEnd"))
+ F.BlockEnd = *Value;
+ });
Dict.handle("TypeNameLimit", [&](Node &N) {
if (auto Value = uint32Value(N, "TypeNameLimit"))
F.TypeNameLimit = *Value;
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 941a8fdfdfdfee..1347fa0d2b29e0 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -327,6 +327,33 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
addReturnTypeHint(D, FTL.getRParenLoc());
}
}
+ if (Cfg.InlayHints.BlockEnd && D->isThisDeclarationADefinition()) {
+ // We use `printName` here to properly print name of ctor/dtor/operator
+ // overload.
+ if (const Stmt *Body = D->getBody())
+ addBlockEndHint(Body->getSourceRange(), "", printName(AST, *D), "");
+ }
+ return true;
+ }
+
+ bool VisitTagDecl(TagDecl *D) {
+ if (Cfg.InlayHints.BlockEnd && D->isThisDeclarationADefinition()) {
+ std::string DeclPrefix = D->getKindName().str();
+ if (const auto *ED = dyn_cast<EnumDecl>(D)) {
+ if (ED->isScoped())
+ DeclPrefix += ED->isScopedUsingClassTag() ? " class" : " struct";
+ };
+ addBlockEndHint(D->getBraceRange(), DeclPrefix, getSimpleName(*D), ";");
+ }
+ return true;
+ }
+
+ bool VisitNamespaceDecl(NamespaceDecl *D) {
+ if (Cfg.InlayHints.BlockEnd) {
+ // For namespace, the range actually starts at the namespace keyword. But
+ // it should be fine since it's usually very short.
+ addBlockEndHint(D->getSourceRange(), "namespace", getSimpleName(*D), "");
+ }
return true;
}
@@ -673,6 +700,16 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
void addInlayHint(SourceRange R, HintSide Side, InlayHintKind Kind,
llvm::StringRef Prefix, llvm::StringRef Label,
llvm::StringRef Suffix) {
+ auto LSPRange = getHintRange(R);
+ if (!LSPRange)
+ return;
+
+ addInlayHint(*LSPRange, Side, Kind, Prefix, Label, Suffix);
+ }
+
+ void addInlayHint(Range LSPRange, HintSide Side, InlayHintKind Kind,
+ llvm::StringRef Prefix, llvm::StringRef Label,
+ llvm::StringRef Suffix) {
// We shouldn't get as far as adding a hint if the category is disabled.
// We'd like to disable as much of the analysis as possible above instead.
// Assert in debug mode but add a dynamic check in production.
@@ -688,20 +725,18 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
CHECK_KIND(Parameter, Parameters);
CHECK_KIND(Type, DeducedTypes);
CHECK_KIND(Designator, Designators);
+ CHECK_KIND(BlockEnd, BlockEnd);
#undef CHECK_KIND
}
- auto LSPRange = getHintRange(R);
- if (!LSPRange)
- return;
- Position LSPPos = Side == HintSide::Left ? LSPRange->start : LSPRange->end;
+ Position LSPPos = Side == HintSide::Left ? LSPRange.start : LSPRange.end;
if (RestrictRange &&
(LSPPos < RestrictRange->start || !(LSPPos < RestrictRange->end)))
return;
bool PadLeft = Prefix.consume_front(" ");
bool PadRight = Suffix.consume_back(" ");
Results.push_back(InlayHint{LSPPos, (Prefix + Label + Suffix).str(), Kind,
- PadLeft, PadRight, *LSPRange});
+ PadLeft, PadRight, LSPRange});
}
// Get the range of the main file that *exactly* corresponds to R.
@@ -748,6 +783,76 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
TypeName.size() < Cfg.InlayHints.TypeNameLimit;
}
+ void addBlockEndHint(SourceRange BraceRange, StringRef DeclPrefix,
+ StringRef Name, StringRef OptionalPunctuation) {
+ auto HintRange = computeBlockEndHintRange(BraceRange, OptionalPunctuation);
+ if (!HintRange)
+ return;
+
+ std::string Label = DeclPrefix.str();
+ if (!Label.empty() && !Name.empty())
+ Label += ' ';
+ Label += Name;
+
+ constexpr unsigned HintMaxLengthLimit = 60;
+ if (Label.length() > HintMaxLengthLimit)
+ return;
+
+ addInlayHint(*HintRange, HintSide::Right, InlayHintKind::BlockEnd, " // ",
+ Label, "");
+ }
+
+ // Compute the LSP range to attach the block end hint to, if any allowed.
+ // 1. "}" is the last non-whitespace character on the line. The range of "}"
+ // is returned.
+ // 2. After "}", if the trimmed trailing text is exactly
+ // `OptionalPunctuation`, say ";". The range of "} ... ;" is returned.
+ // 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] =
+ SM.getDecomposedLoc(SM.getFileLoc(BraceRange.getBegin()));
+ auto RBraceLoc = SM.getFileLoc(BraceRange.getEnd());
+ auto [RBraceFileId, RBraceOffset] = SM.getDecomposedLoc(RBraceLoc);
+
+ // Because we need to check the block satisfies the minimum line limit, we
+ // require both source location to be in the main file. This prevents hint
+ // to be shown in weird cases like '{' is actually in a "#include", but it's
+ // rare anyway.
+ if (BlockBeginFileId != MainFileID || RBraceFileId != MainFileID)
+ return std::nullopt;
+
+ StringRef RestOfLine = MainFileBuf.substr(RBraceOffset).split('\n').first;
+ if (!RestOfLine.starts_with("}"))
+ return std::nullopt;
+
+ StringRef TrimmedTrailingText = RestOfLine.drop_front().trim();
+ if (!TrimmedTrailingText.empty() &&
+ TrimmedTrailingText != OptionalPunctuation)
+ return std::nullopt;
+
+ auto BlockBeginLine = SM.getLineNumber(BlockBeginFileId, BlockBeginOffset);
+ auto RBraceLine = SM.getLineNumber(RBraceFileId, RBraceOffset);
+
+ // Don't show hint on trivial blocks like `class X {};`
+ if (BlockBeginLine + HintMinLineLimit - 1 > RBraceLine)
+ return std::nullopt;
+
+ // This is what we attach the hint to, usually "}" or "};".
+ StringRef HintRangeText = RestOfLine.take_front(
+ TrimmedTrailingText.empty()
+ ? 1
+ : TrimmedTrailingText.bytes_end() - RestOfLine.bytes_begin());
+
+ Position HintStart = sourceLocToPosition(SM, RBraceLoc);
+ Position HintEnd = sourceLocToPosition(
+ SM, RBraceLoc.getLocWithOffset(HintRangeText.size()));
+ return Range{HintStart, HintEnd};
+ }
+
std::vector<InlayHint> &Results;
ASTContext &AST;
const syntax::TokenBuffer &Tokens;
diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index b16771c8953931..e44aee2d478194 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1458,7 +1458,9 @@ llvm::json::Value toJSON(const InlayHintKind &Kind) {
return 1;
case InlayHintKind::Parameter:
return 2;
- case InlayHintKind::Designator: // This is an extension, don't serialize.
+ case InlayHintKind::Designator:
+ case InlayHintKind::BlockEnd:
+ // This is an extension, don't serialize.
return nullptr;
}
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
@@ -1492,6 +1494,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, InlayHintKind Kind) {
return "type";
case InlayHintKind::Designator:
return "designator";
+ case InlayHintKind::BlockEnd:
+ return "block-end";
}
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
};
diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index cc2ad99451a2f2..09fcfb55f4911e 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1672,6 +1672,16 @@ enum class InlayHintKind {
/// This is a clangd extension.
Designator = 3,
+ /// A hint after function, type or namespace definition, indicating the
+ /// defined symbol name of the definition.
+ ///
+ /// An example of a decl name hint in this position:
+ /// void func() {
+ /// } ^
+ /// Uses comment-like syntax like "// func".
+ /// This is a clangd extension.
+ BlockEnd = 4,
+
/// Other ideas for hints that are not currently implemented:
///
/// * Chaining hints, showing the types of intermediate expressions
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 59ec9aa4776dae..9eb652983d39b7 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -77,6 +77,7 @@ Config noHintsConfig() {
C.InlayHints.Parameters = false;
C.InlayHints.DeducedTypes = false;
C.InlayHints.Designators = false;
+ C.InlayHints.BlockEnd = false;
return C;
}
@@ -122,6 +123,15 @@ void assertDesignatorHints(llvm::StringRef AnnotatedSource,
assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...);
}
+template <typename... ExpectedHints>
+void assertBlockEndHints(llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+ Config Cfg;
+ Cfg.InlayHints.BlockEnd = true;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+ assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Expected...);
+}
+
TEST(ParameterHints, Smoke) {
assertParameterHints(R"cpp(
void foo(int param);
@@ -1629,6 +1639,194 @@ TEST(ParameterHints, DoesntExpandAllArgs) {
ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"},
ExpectedHint{"c: ", "param3"});
}
+
+TEST(BlockEndHints, Functions) {
+ assertBlockEndHints(R"cpp(
+ int foo() {
+ return 41;
+ $foo[[}]]
+
+ template<int X>
+ int bar() {
+ // No hint for lambda for now
+ auto f = []() {
+ return X;
+ };
+ return f();
+ $bar[[}]]
+
+ // No hint because this isn't a definition
+ int buz();
+
+ struct S{};
+ bool operator==(S, S) {
+ return true;
+ $opEqual[[}]]
+ )cpp",
+ ExpectedHint{" // foo", "foo"},
+ ExpectedHint{" // bar", "bar"},
+ ExpectedHint{" // operator==", "opEqual"});
+}
+
+TEST(BlockEndHints, Methods) {
+ assertBlockEndHints(R"cpp(
+ struct Test {
+ // No hint because there's no function body
+ Test() = default;
+
+ ~Test() {
+ $dtor[[}]]
+
+ void method1() {
+ $method1[[}]]
+
+ // No hint because this isn't a definition
+ void method2();
+
+ template <typename T>
+ void method3() {
+ $method3[[}]]
+
+ // No hint because this isn't a definition
+ template <typename T>
+ void method4();
+
+ Test operator+(int) const {
+ return *this;
+ $opIdentity[[}]]
+
+ operator bool() const {
+ return true;
+ $opBool[[}]]
+
+ // No hint because there's no function body
+ operator int() const = delete;
+ } x;
+
+ void Test::method2() {
+ $method2[[}]]
+
+ template <typename T>
+ void Test::method4() {
+ $method4[[}]]
+ )cpp",
+ ExpectedHint{" // ~Test", "dtor"},
+ ExpectedHint{" // method1", "method1"},
+ ExpectedHint{" // method3", "method3"},
+ ExpectedHint{" // operator+", "opIdentity"},
+ ExpectedHint{" // operator bool", "opBool"},
+ ExpectedHint{" // Test::method2", "method2"},
+ ExpectedHint{" // Test::method4", "method4"});
+}
+
+TEST(BlockEndHints, Namespaces) {
+ assertBlockEndHints(
+ R"cpp(
+ namespace {
+ void foo();
+ $anon[[}]]
+
+ namespace ns {
+ void bar();
+ $ns[[}]]
+ )cpp",
+ ExpectedHint{" // namespace", "anon"},
+ ExpectedHint{" // namespace ns", "ns"});
+}
+
+TEST(BlockEndHints, Types) {
+ assertBlockEndHints(
+ R"cpp(
+ struct S {
+ $S[[};]]
+
+ class C {
+ $C[[};]]
+
+ union U {
+ $U[[};]]
+
+ enum E1 {
+ $E1[[};]]
+
+ enum class E2 {
+ $E2[[};]]
+ )cpp",
+ ExpectedHint{" // struct S", "S"}, ExpectedHint{" // class C", "C"},
+ ExpectedHint{" // union U", "U"}, ExpectedHint{" // enum E1", "E1"},
+ ExpectedHint{" // enum class E2", "E2"});
+}
+
+TEST(BlockEndHints, TrailingSemicolon) {
+ assertBlockEndHints(R"cpp(
+ // The hint is placed after the trailing ';'
+ struct S1 {
+ $S1[[} ;]]
+
+ // The hint is always placed in the same line with the closing '}'.
+ // So in this case where ';' is missing, it is attached to '}'.
+ struct S2 {
+ $S2[[}]]
+
+ ;
+
+ // No hint because only one trailing ';' is allowed
+ struct S3 {
+ };;
+
+ // No hint because trailing ';' is only allowed for class/struct/union/enum
+ void foo() {
+ };
+
+ // Rare case, but yes we'll have a hint here.
+ struct {
+ int x;
+ $anon[[}]]
+
+ s2;
+ )cpp",
+ ExpectedHint{" // struct S1", "S1"},
+ ExpectedHint{" // struct S2", "S2"},
+ ExpectedHint{" // struct", "anon"});
+}
+
+TEST(BlockEndHints, TrailingText) {
+ assertBlockEndHints(R"cpp(
+ struct S1 {
+ $S1[[} ;]]
+
+ // No hint for S2 because of the trailing comment
+ struct S2 {
+ }; /* Put anything here */
+
+ struct S3 {
+ // No hint for S4 because of the trailing source code
+ struct S4 {
+ };$S3[[};]]
+
+ // No hint for ns because of the trailing comment
+ namespace ns {
+ } // namespace ns
+ )cpp",
+ ExpectedHint{" // struct S1", "S1"},
+ ExpectedHint{" // struct S3", "S3"});
+}
+
+TEST(BlockEndHints, Macro) {
+ assertBlockEndHints(R"cpp(
+ #define DECL_STRUCT(NAME) struct NAME {
+ #define RBRACE }
+
+ DECL_STRUCT(S1)
+ $S1[[};]]
+
+ // No hint because we require a '}'
+ DECL_STRUCT(S2)
+ RBRACE;
+ )cpp",
+ ExpectedHint{" // struct S1", "S1"});
+}
+
// 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