diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/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; }; diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/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; diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -322,8 +322,12 @@ std::optional> DeducedTypes; /// Show designators in aggregate initialization. std::optional> Designators; + /// Show defined symbol names at the end of a definition. + std::optional> EndDefinitionComments; /// Limit the length of type name hints. (0 means no limit) std::optional> TypeNameLimit; + /// + std::optional> EndDefinitionCommentMinLines; }; InlayHintsBlock InlayHints; }; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/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); } diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/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(EndDefinitionComment, 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(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(&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(D)) + Label += "namespace "; + else if (isa(D)) { + Label += "enum "; + if (cast(D).isScopedUsingClassTag()) { + Label += "class "; + } + } else if (const RecordDecl *RecordD = dyn_cast_or_null(&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 += ""; + } + + addInlayHint(*LSPRange, HintSide::Right, + InlayHintKind::EndDefinitionComment, " /* ", Label, " */ "); + } + std::vector &Results; ASTContext &AST; const syntax::TokenBuffer &Tokens; diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h --- a/clang-tools-extra/clangd/Protocol.h +++ b/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. + EndDefinitionComment = 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/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/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::EndDefinitionComment: // 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::EndDefinitionComment: + return "end-definition-comment"; } llvm_unreachable("Unknown clang.clangd.InlayHintKind"); }; diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/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) { diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/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 +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::EndDefinitionComment, 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 */ ", "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 */ ", "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) (...);