daiyousei-qz created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
daiyousei-qz requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D150635
Files:
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/ConfigYAMLTests.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
@@ -77,6 +77,7 @@
C.InlayHints.Parameters = false;
C.InlayHints.DeducedTypes = false;
C.InlayHints.Designators = false;
+ C.InlayHints.EndDefinitionComments = false;
return C;
}
@@ -122,6 +123,17 @@
assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...);
}
+template <typename... ExpectedHints>
+void assertEndDefinitionHints(llvm::StringRef AnnotatedSource,
+ uint32_t MinLines, ExpectedHints... Expected) {
+ Config Cfg;
+ Cfg.InlayHints.EndDefinitionComments = true;
+ Cfg.InlayHints.EndDefinitionCommentMinLines = MinLines;
+ WithContextValue WithCfg(Config::Key, std::move(Cfg));
+ assertHints(InlayHintKind::EndDefinitionComments, AnnotatedSource,
+ Expected...);
+}
+
TEST(ParameterHints, Smoke) {
assertParameterHints(R"cpp(
void foo(int param);
@@ -1550,6 +1562,165 @@
ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"},
ExpectedHint{"c: ", "param3"});
}
+
+TEST(EndDefinitionHints, Functions) {
+ assertEndDefinitionHints(R"cpp(
+ $foo[[int foo() {
+ return 41;
+ }]]
+ $bar[[int bar() {}]]
+
+ // No hint because this isn't a definition
+ int buz();
+ )cpp",
+ 0, ExpectedHint{" /* foo */ ", "foo"},
+ ExpectedHint{" /* bar */ ", "bar"});
+}
+
+TEST(EndDefinitionHints, Methods) {
+ assertEndDefinitionHints(R"cpp(
+ class Test {
+ public:
+ $ctor[[Test() = default]];
+ $dtor[[~Test() {
+ }]]
+
+ $method[[void method() {}]]
+
+ // No hint because this isn't a definition
+ void method2();
+ } x;
+ )cpp",
+ 0, ExpectedHint{" /* Test */ ", "ctor"},
+ ExpectedHint{" /* ~Test */ ", "dtor"},
+ ExpectedHint{" /* method */ ", "method"});
+}
+
+TEST(EndDefinitionHints, OverloadedOperators) {
+ assertEndDefinitionHints(R"cpp(
+ struct S {
+ $opAdd[[S operator+(int) const {
+ return *this;
+ }]]
+
+ $opBool[[operator bool() const {
+ return true;
+ }]]
+
+ $opInt[[operator int() const = delete]];
+
+ // No hint because this isn't a definition
+ operator float() const;
+ } x;
+ )cpp",
+ 0, ExpectedHint{" /* operator+ */ ", "opAdd"},
+ ExpectedHint{" /* operator bool */ ", "opBool"},
+ ExpectedHint{" /* operator int */ ", "opInt"});
+}
+
+TEST(EndDefinitionHints, Namespaces) {
+ assertEndDefinitionHints(
+ R"cpp(
+ $anon[[namespace {
+ }]]
+
+ $ns[[namespace ns {
+ void foo();
+ }]]
+ )cpp",
+ 0, ExpectedHint{" /* namespace <anonymous> */ ", "anon"},
+ ExpectedHint{" /* namespace ns */ ", "ns"});
+}
+
+TEST(EndDefinitionHints, Types) {
+ assertEndDefinitionHints(R"cpp(
+ $S[[struct S {
+ }]];
+
+ $C[[class C {
+ }]];
+
+ $U[[union U {
+ }]];
+
+ $E1[[enum E1 {
+ }]];
+
+ $E2[[enum class E2 {
+ }]];
+ )cpp",
+ 0, ExpectedHint{" /* struct S */ ", "S"},
+ ExpectedHint{" /* class C */ ", "C"},
+ ExpectedHint{" /* union U */ ", "U"},
+ ExpectedHint{" /* enum E1 */ ", "E1"},
+ ExpectedHint{" /* enum class E2 */ ", "E2"});
+}
+
+TEST(EndDefinitionHints, BundledTypeVariableDecl) {
+ assertEndDefinitionHints(
+ R"cpp(
+ struct {
+ int x;
+ } s;
+
+ $anon[[struct {
+ int x;
+ }]]
+
+ s2;
+ )cpp",
+ 0, ExpectedHint{" /* struct <anonymous> */ ", "anon"});
+}
+
+TEST(EndDefinitionHints, TrailingSemicolon) {
+ assertEndDefinitionHints(R"cpp(
+ $S1[[struct S1 {
+ }]];
+
+ $S2[[struct S2 {
+ }]]
+
+ ;
+
+ $S3[[struct S3 {
+ }]] ;; ;;
+ )cpp",
+ 0, ExpectedHint{" /* struct S1 */ ", "S1"},
+ ExpectedHint{" /* struct S2 */ ", "S2"},
+ ExpectedHint{" /* struct S3 */ ", "S3"});
+}
+
+TEST(EndDefinitionHints, TrailingText) {
+ assertEndDefinitionHints(R"cpp(
+ $S1[[struct S1 {
+ }]] ;
+
+ // No hint for S2 because of the trailing comment
+ struct S2 {
+ }; /* Put anything here */
+
+ // No hint for S3 because of the trailing source code
+ struct S3 {}; $S4[[struct S4 {}]];
+
+ // No hint for ns because of the trailing comment
+ namespace ns {
+
+ } // namespace ns
+ )cpp",
+ 0, ExpectedHint{" /* struct S1 */ ", "S1"},
+ ExpectedHint{" /* struct S4 */ ", "S4"});
+}
+
+TEST(EndDefinitionHints, MinLineConfig) {
+ assertEndDefinitionHints(R"cpp(
+ struct S1 {};
+
+ $S2[[struct S2 {
+ }]];
+ )cpp",
+ 2, ExpectedHint{" /* struct S2 */ ", "S2"});
+}
+
// FIXME: Low-hanging fruit where we could omit a type hint:
// - auto x = TypeName(...);
// - auto x = (TypeName) (...);
Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -236,6 +236,8 @@
InlayHints:
Enabled: No
ParameterNames: Yes
+ EndDefinitionComments: Yes
+ EndDefinitionCommentMinLines: 10
)yaml");
auto Results =
Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
@@ -244,6 +246,10 @@
EXPECT_THAT(Results[0].InlayHints.Enabled, llvm::ValueIs(val(false)));
EXPECT_THAT(Results[0].InlayHints.ParameterNames, llvm::ValueIs(val(true)));
EXPECT_EQ(Results[0].InlayHints.DeducedTypes, std::nullopt);
+ EXPECT_THAT(Results[0].InlayHints.EndDefinitionComments,
+ llvm::ValueIs(val(true)));
+ EXPECT_THAT(Results[0].InlayHints.EndDefinitionCommentMinLines,
+ llvm::ValueIs(val(10)));
}
TEST(ParseYAML, IncludesIgnoreHeader) {
Index: clang-tools-extra/clangd/Protocol.h
===================================================================
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -1647,6 +1647,16 @@
/// 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.
+ EndDefinitionComments,
+
/// Other ideas for hints that are not currently implemented:
///
/// * Chaining hints, showing the types of intermediate expressions
Index: clang-tools-extra/clangd/Protocol.cpp
===================================================================
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1435,6 +1435,8 @@
case InlayHintKind::Parameter:
return 2;
case InlayHintKind::Designator: // This is an extension, don't serialize.
+ case InlayHintKind::EndDefinitionComments: // This is an extension, don't
+ // serialize.
return nullptr;
}
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
@@ -1468,6 +1470,8 @@
return "type";
case InlayHintKind::Designator:
return "designator";
+ case InlayHintKind::EndDefinitionComments:
+ return "end-definition";
}
llvm_unreachable("Unknown clang.clangd.InlayHintKind");
};
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -274,6 +274,33 @@
addReturnTypeHint(D, FTL.getRParenLoc());
}
}
+ if (Cfg.InlayHints.EndDefinitionComments &&
+ D->isThisDeclarationADefinition()) {
+ addEndDefinitionCommentHint(*D);
+ }
+ return true;
+ }
+
+ bool VisitEnumDecl(EnumDecl *D) {
+ if (Cfg.InlayHints.EndDefinitionComments &&
+ D->isThisDeclarationADefinition()) {
+ addEndDefinitionCommentHint(*D);
+ }
+ return true;
+ }
+
+ bool VisitRecordDecl(RecordDecl *D) {
+ if (Cfg.InlayHints.EndDefinitionComments &&
+ D->isThisDeclarationADefinition()) {
+ addEndDefinitionCommentHint(*D);
+ }
+ return true;
+ }
+
+ bool VisitNamespaceDecl(NamespaceDecl *D) {
+ if (Cfg.InlayHints.EndDefinitionComments) {
+ addEndDefinitionCommentHint(*D);
+ }
return true;
}
@@ -539,6 +566,24 @@
return SourcePrefix.endswith("/*");
}
+ // To avoid clash with manual annotation from users, we only show this hint if
+ // there's no character after '}' except for whitespace and ';'.
+ // Note this also allows hint to be shown for cases like:
+ // struct S {
+ // }^;;;;;
+ // However, this is a rare case and we don't want to complicate the logic.
+ bool shouldHintEndDefinitionComment(const NamedDecl &D) {
+ auto &SM = AST.getSourceManager();
+ auto FileLoc = SM.getFileLoc(D.getEndLoc());
+ auto Decomposed = SM.getDecomposedLoc(FileLoc);
+ if (Decomposed.first != MainFileID)
+ return false;
+
+ StringRef SourceSuffix =
+ MainFileBuf.substr(Decomposed.second + 1).ltrim("; \v\f\r");
+ return SourceSuffix.empty() || SourceSuffix.starts_with("\n");
+ };
+
// If "E" spells a single unqualified identifier, return that name.
// Otherwise, return an empty string.
static StringRef getSpelledIdentifier(const Expr *E) {
@@ -616,6 +661,16 @@
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.
@@ -631,20 +686,18 @@
CHECK_KIND(Parameter, Parameters);
CHECK_KIND(Type, DeducedTypes);
CHECK_KIND(Designator, Designators);
+ CHECK_KIND(EndDefinitionComments, EndDefinitionComments);
#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.
@@ -699,6 +752,53 @@
/*Prefix=*/"", Text, /*Suffix=*/"=");
}
+ void addEndDefinitionCommentHint(const NamedDecl &D) {
+ if (!shouldHintEndDefinitionComment(D))
+ return;
+
+ // Note this range doesn't include the trailing ';' in type definitions.
+ SourceRange R = D.getSourceRange();
+ auto LSPRange = getHintRange(R);
+ if (!LSPRange ||
+ static_cast<uint32_t>(LSPRange->end.line - LSPRange->start.line) + 1 <
+ Cfg.InlayHints.EndDefinitionCommentMinLines)
+ return;
+
+ /// TODO: We could use InlayHintLabelPart to provide language features on
+ /// hints.
+ std::string Label;
+ if (const auto *FD = dyn_cast_or_null<FunctionDecl>(&D)) {
+ Label = printName(AST, D);
+ } else {
+ // We handle type and namespace decls together.
+ // Note we don't use printName here for formatting issues.
+ if (isa<NamespaceDecl>(D))
+ Label += "namespace ";
+ else if (isa<EnumDecl>(D)) {
+ Label += "enum ";
+ if (cast<EnumDecl>(D).isScopedUsingClassTag()) {
+ Label += "class ";
+ }
+ } else if (const RecordDecl *RecordD = dyn_cast_or_null<RecordDecl>(&D)) {
+ if (RecordD->isStruct())
+ Label += "struct ";
+ else if (RecordD->isClass())
+ Label += "class ";
+ else if (RecordD->isUnion())
+ Label += "union ";
+ }
+
+ StringRef Name = getSimpleName(D);
+ if (!Name.empty())
+ Label += Name;
+ else
+ Label += "<anonymous>";
+ }
+
+ addInlayHint(*LSPRange, HintSide::Right,
+ InlayHintKind::EndDefinitionComments, " /* ", Label, " */ ");
+ }
+
std::vector<InlayHint> &Results;
ASTContext &AST;
const syntax::TokenBuffer &Tokens;
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,10 +254,18 @@
if (auto Value = boolValue(N, "Designators"))
F.Designators = *Value;
});
+ Dict.handle("EndDefinitionComments", [&](Node &N) {
+ if (auto Value = boolValue(N, "EndDefinitionComments"))
+ F.EndDefinitionComments = *Value;
+ });
Dict.handle("TypeNameLimit", [&](Node &N) {
if (auto Value = uint32Value(N, "TypeNameLimit"))
F.TypeNameLimit = *Value;
});
+ Dict.handle("EndDefinitionCommentMinLines", [&](Node &N) {
+ if (auto Value = uint32Value(N, "EndDefinitionCommentMinLines"))
+ F.EndDefinitionCommentMinLines = *Value;
+ });
Dict.parse(N);
}
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,8 +322,12 @@
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.
+ std::optional<Located<bool>> EndDefinitionComments;
/// Limit the length of type name hints. (0 means no limit)
std::optional<Located<uint32_t>> TypeNameLimit;
+ ///
+ std::optional<Located<uint32_t>> EndDefinitionCommentMinLines;
};
InlayHintsBlock InlayHints;
};
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,11 +611,21 @@
Out.Apply.push_back([Value(**F.Designators)](const Params &, Config &C) {
C.InlayHints.Designators = Value;
});
+ if (F.EndDefinitionComments)
+ Out.Apply.push_back(
+ [Value(**F.EndDefinitionComments)](const Params &, Config &C) {
+ C.InlayHints.EndDefinitionComments = Value;
+ });
if (F.TypeNameLimit)
Out.Apply.push_back(
[Value(**F.TypeNameLimit)](const Params &, Config &C) {
C.InlayHints.TypeNameLimit = Value;
});
+ if (F.EndDefinitionCommentMinLines)
+ Out.Apply.push_back(
+ [Value(**F.EndDefinitionCommentMinLines)](const Params &, Config &C) {
+ C.InlayHints.EndDefinitionCommentMinLines = Value;
+ });
}
constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,8 +147,13 @@
bool Parameters = true;
bool DeducedTypes = true;
bool Designators = true;
+ bool EndDefinitionComments = true;
// Limit the length of type names in inlay hints. (0 means no limit)
uint32_t TypeNameLimit = 32;
+
+ // The minimal number of lines of the definition to show the
+ // end-definition comment hints.
+ uint32_t EndDefinitionCommentMinLines = 2;
} InlayHints;
};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
