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 @@ -144,6 +144,7 @@ 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 --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -606,6 +606,10 @@ 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 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -318,6 +318,8 @@ std::optional> DeducedTypes; /// Show designators in aggregate initialization. std::optional> Designators; + /// Show defined symbol names at the end of a definition block. + std::optional> BlockEnd; /// Limit the length of type name hints. (0 means no limit) std::optional> TypeNameLimit; }; 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 @@ -252,6 +252,10 @@ 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 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -327,6 +327,33 @@ 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(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 @@ 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 @@ 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 @@ 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 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 &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 @@ -1672,6 +1672,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. + 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/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp --- a/clang-tools-extra/clangd/Protocol.cpp +++ b/clang-tools-extra/clangd/Protocol.cpp @@ -1458,7 +1458,9 @@ 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 @@ 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/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.BlockEnd = false; return C; } @@ -122,6 +123,15 @@ assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...); } +template +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 @@ ExpectedHint{"a: ", "param1"}, ExpectedHint{"b: ", "param2"}, ExpectedHint{"c: ", "param3"}); } + +TEST(BlockEndHints, Functions) { + assertBlockEndHints(R"cpp( + int foo() { + return 41; + $foo[[}]] + + template + 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 + void method3() { + $method3[[}]] + + // No hint because this isn't a definition + template + 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 + 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) (...);