https://github.com/MythreyaK updated https://github.com/llvm/llvm-project/pull/136106
>From 64410e0c5bbdc3f631e2efecef475768d48ef233 Mon Sep 17 00:00:00 2001 From: daiyousei-qz <qyzhe...@outlook.com> Date: Tue, 14 Nov 2023 20:42:10 -0800 Subject: [PATCH 1/4] Improve BlockEnd presentation including: 1. Explicitly state a function call 2. Print literal nullptr 3. Escape for abbreviated string 4. Adjust min line limit to 10 --- clang-tools-extra/clangd/InlayHints.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 40a824618f782..a1bc9956ec628 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -112,7 +112,9 @@ std::string summarizeExpr(const Expr *E) { return getSimpleName(*E->getFoundDecl()).str(); } std::string VisitCallExpr(const CallExpr *E) { - return Visit(E->getCallee()); + std::string Result = Visit(E->getCallee()); + Result += E->getNumArgs() == 0 ? "()" : "(...)"; + return Result; } std::string VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) { @@ -147,6 +149,9 @@ std::string summarizeExpr(const Expr *E) { } // Literals are just printed + std::string VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E) { + return "nullptr"; + } std::string VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E) { return E->getValue() ? "true" : "false"; } @@ -165,12 +170,14 @@ std::string summarizeExpr(const Expr *E) { std::string Result = "\""; if (E->containsNonAscii()) { Result += "..."; - } else if (E->getLength() > 10) { - Result += E->getString().take_front(7); - Result += "..."; } else { llvm::raw_string_ostream OS(Result); - llvm::printEscapedString(E->getString(), OS); + if (E->getLength() > 10) { + llvm::printEscapedString(E->getString().take_front(7), OS); + Result += "..."; + } else { + llvm::printEscapedString(E->getString(), OS); + } } Result.push_back('"'); return Result; @@ -1120,7 +1127,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { // Otherwise, the hint shouldn't be shown. std::optional<Range> computeBlockEndHintRange(SourceRange BraceRange, StringRef OptionalPunctuation) { - constexpr unsigned HintMinLineLimit = 2; + constexpr unsigned HintMinLineLimit = 10; auto &SM = AST.getSourceManager(); auto [BlockBeginFileId, BlockBeginOffset] = >From fe79c934873f5df8c13ef130adf96a05cc48364f Mon Sep 17 00:00:00 2001 From: Mythreya <g...@mythreya.dev> Date: Thu, 17 Apr 2025 01:28:53 -0700 Subject: [PATCH 2/4] Add `InlayHintOptions` --- clang-tools-extra/clangd/InlayHints.cpp | 16 ++++++++++------ clang-tools-extra/clangd/InlayHints.h | 8 +++++++- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index a1bc9956ec628..bdab2b8a9f377 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -415,12 +415,14 @@ struct Callee { class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { public: InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST, - const Config &Cfg, std::optional<Range> RestrictRange) + const Config &Cfg, std::optional<Range> RestrictRange, + InlayHintOptions HintOptions) : Results(Results), AST(AST.getASTContext()), Tokens(AST.getTokens()), Cfg(Cfg), RestrictRange(std::move(RestrictRange)), MainFileID(AST.getSourceManager().getMainFileID()), Resolver(AST.getHeuristicResolver()), - TypeHintPolicy(this->AST.getPrintingPolicy()) { + TypeHintPolicy(this->AST.getPrintingPolicy()), + HintOptions(HintOptions) { bool Invalid = false; llvm::StringRef Buf = AST.getSourceManager().getBufferData(MainFileID, &Invalid); @@ -1127,7 +1129,6 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { // Otherwise, the hint shouldn't be shown. std::optional<Range> computeBlockEndHintRange(SourceRange BraceRange, StringRef OptionalPunctuation) { - constexpr unsigned HintMinLineLimit = 10; auto &SM = AST.getSourceManager(); auto [BlockBeginFileId, BlockBeginOffset] = @@ -1155,7 +1156,7 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { auto RBraceLine = SM.getLineNumber(RBraceFileId, RBraceOffset); // Don't show hint on trivial blocks like `class X {};` - if (BlockBeginLine + HintMinLineLimit - 1 > RBraceLine) + if (BlockBeginLine + HintOptions.HintMinLineLimit - 1 > RBraceLine) return std::nullopt; // This is what we attach the hint to, usually "}" or "};". @@ -1185,17 +1186,20 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { StringRef MainFileBuf; const HeuristicResolver *Resolver; PrintingPolicy TypeHintPolicy; + InlayHintOptions HintOptions; }; } // namespace std::vector<InlayHint> inlayHints(ParsedAST &AST, - std::optional<Range> RestrictRange) { + std::optional<Range> RestrictRange, + InlayHintOptions HintOptions) { std::vector<InlayHint> Results; const auto &Cfg = Config::current(); if (!Cfg.InlayHints.Enabled) return Results; - InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange)); + InlayHintVisitor Visitor(Results, AST, Cfg, std::move(RestrictRange), + HintOptions); Visitor.TraverseAST(AST.getASTContext()); // De-duplicate hints. Duplicates can sometimes occur due to e.g. explicit diff --git a/clang-tools-extra/clangd/InlayHints.h b/clang-tools-extra/clangd/InlayHints.h index 6a0236a0ab08a..544f3c8c2d03a 100644 --- a/clang-tools-extra/clangd/InlayHints.h +++ b/clang-tools-extra/clangd/InlayHints.h @@ -22,10 +22,16 @@ namespace clang { namespace clangd { class ParsedAST; +struct InlayHintOptions { + // Minimum lines for BlockEnd inlay-hints to be shown + int HintMinLineLimit{2}; +}; + /// Compute and return inlay hints for a file. /// If RestrictRange is set, return only hints whose location is in that range. std::vector<InlayHint> inlayHints(ParsedAST &AST, - std::optional<Range> RestrictRange); + std::optional<Range> RestrictRange, + InlayHintOptions HintOptions = {}); } // namespace clangd } // namespace clang >From e387e66d03450d569fd8eed15611599a8a702da7 Mon Sep 17 00:00:00 2001 From: Mythreya <g...@mythreya.dev> Date: Fri, 18 Apr 2025 21:23:28 -0700 Subject: [PATCH 3/4] Update tests --- .../clangd/unittests/InlayHintTests.cpp | 92 ++++++++++++------- 1 file changed, 61 insertions(+), 31 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 77d78b8777fe3..e22e670928c1b 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -36,9 +36,12 @@ namespace { using ::testing::ElementsAre; using ::testing::IsEmpty; -std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind) { +constexpr InlayHintOptions DefaultInlayHintOpts{}; + +std::vector<InlayHint> hintsOfKind(ParsedAST &AST, InlayHintKind Kind, + InlayHintOptions Opts) { std::vector<InlayHint> Result; - for (auto &Hint : inlayHints(AST, /*RestrictRange=*/std::nullopt)) { + for (auto &Hint : inlayHints(AST, /*RestrictRange=*/std::nullopt, Opts)) { if (Hint.kind == Kind) Result.push_back(Hint); } @@ -90,7 +93,7 @@ Config noHintsConfig() { template <typename... ExpectedHints> void assertHintsWithHeader(InlayHintKind Kind, llvm::StringRef AnnotatedSource, - llvm::StringRef HeaderContent, + llvm::StringRef HeaderContent, InlayHintOptions Opts, ExpectedHints... Expected) { Annotations Source(AnnotatedSource); TestTU TU = TestTU::withCode(Source.code()); @@ -98,18 +101,18 @@ void assertHintsWithHeader(InlayHintKind Kind, llvm::StringRef AnnotatedSource, TU.HeaderCode = HeaderContent; auto AST = TU.build(); - EXPECT_THAT(hintsOfKind(AST, Kind), + EXPECT_THAT(hintsOfKind(AST, Kind, Opts), ElementsAre(HintMatcher(Expected, Source)...)); // Sneak in a cross-cutting check that hints are disabled by config. // We'll hit an assertion failure if addInlayHint still gets called. WithContextValue WithCfg(Config::Key, noHintsConfig()); - EXPECT_THAT(inlayHints(AST, std::nullopt), IsEmpty()); + EXPECT_THAT(inlayHints(AST, std::nullopt, Opts), IsEmpty()); } template <typename... ExpectedHints> void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource, - ExpectedHints... Expected) { - return assertHintsWithHeader(Kind, AnnotatedSource, "", + InlayHintOptions Opts, ExpectedHints... Expected) { + return assertHintsWithHeader(Kind, AnnotatedSource, "", Opts, std::move(Expected)...); } @@ -120,14 +123,16 @@ template <typename... ExpectedHints> void assertParameterHints(llvm::StringRef AnnotatedSource, ExpectedHints... Expected) { ignore(Expected.Side = Left...); - assertHints(InlayHintKind::Parameter, AnnotatedSource, Expected...); + assertHints(InlayHintKind::Parameter, AnnotatedSource, DefaultInlayHintOpts, + Expected...); } template <typename... ExpectedHints> void assertTypeHints(llvm::StringRef AnnotatedSource, ExpectedHints... Expected) { ignore(Expected.Side = Right...); - assertHints(InlayHintKind::Type, AnnotatedSource, Expected...); + assertHints(InlayHintKind::Type, AnnotatedSource, DefaultInlayHintOpts, + Expected...); } template <typename... ExpectedHints> @@ -136,16 +141,25 @@ void assertDesignatorHints(llvm::StringRef AnnotatedSource, Config Cfg; Cfg.InlayHints.Designators = true; WithContextValue WithCfg(Config::Key, std::move(Cfg)); - assertHints(InlayHintKind::Designator, AnnotatedSource, Expected...); + assertHints(InlayHintKind::Designator, AnnotatedSource, DefaultInlayHintOpts, + Expected...); } template <typename... ExpectedHints> -void assertBlockEndHints(llvm::StringRef AnnotatedSource, - ExpectedHints... Expected) { +void assertBlockEndHintsWithOpts(llvm::StringRef AnnotatedSource, + InlayHintOptions Opts, + ExpectedHints... Expected) { Config Cfg; Cfg.InlayHints.BlockEnd = true; WithContextValue WithCfg(Config::Key, std::move(Cfg)); - assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Expected...); + assertHints(InlayHintKind::BlockEnd, AnnotatedSource, Opts, Expected...); +} + +template <typename... ExpectedHints> +void assertBlockEndHints(llvm::StringRef AnnotatedSource, + ExpectedHints... Expected) { + assertBlockEndHintsWithOpts(AnnotatedSource, DefaultInlayHintOpts, + Expected...); } TEST(ParameterHints, Smoke) { @@ -1226,7 +1240,9 @@ TEST(ParameterHints, IncludeAtNonGlobalScope) { ASSERT_TRUE(bool(AST)); // Ensure the hint for the call in foo.inc is NOT materialized in foo.cc. - EXPECT_EQ(hintsOfKind(*AST, InlayHintKind::Parameter).size(), 0u); + EXPECT_EQ( + hintsOfKind(*AST, InlayHintKind::Parameter, DefaultInlayHintOpts).size(), + 0u); } TEST(TypeHints, Smoke) { @@ -1488,12 +1504,12 @@ TEST(DefaultArguments, Smoke) { void baz(int = 5) { if (false) baz($unnamed[[)]]; }; )cpp"; - assertHints(InlayHintKind::DefaultArgument, Code, + assertHints(InlayHintKind::DefaultArgument, Code, DefaultInlayHintOpts, ExpectedHint{"A: 4", "default1", Left}, ExpectedHint{", B: 1, C: foo()", "default2", Left}, ExpectedHint{"5", "unnamed", Left}); - assertHints(InlayHintKind::Parameter, Code, + assertHints(InlayHintKind::Parameter, Code, DefaultInlayHintOpts, ExpectedHint{"A: ", "explicit", Left}); } @@ -1528,14 +1544,14 @@ TEST(DefaultArguments, WithoutParameterNames) { } )cpp"; - assertHints(InlayHintKind::DefaultArgument, Code, + assertHints(InlayHintKind::DefaultArgument, Code, DefaultInlayHintOpts, ExpectedHint{"...", "abbreviated", Left}, ExpectedHint{", Baz{}", "paren", Left}, ExpectedHint{", Baz{}", "brace1", Left}, ExpectedHint{", Baz{}", "brace2", Left}, ExpectedHint{", Baz{}", "brace3", Left}); - assertHints(InlayHintKind::Parameter, Code); + assertHints(InlayHintKind::Parameter, Code, DefaultInlayHintOpts); } TEST(TypeHints, Deduplication) { @@ -1573,7 +1589,8 @@ TEST(TypeHints, Aliased) { TU.ExtraArgs.push_back("-xc"); auto AST = TU.build(); - EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type), IsEmpty()); + EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type, DefaultInlayHintOpts), + IsEmpty()); } TEST(TypeHints, CallingConvention) { @@ -1589,7 +1606,8 @@ TEST(TypeHints, CallingConvention) { TU.PredefineMacros = true; // for the __cdecl auto AST = TU.build(); - EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type), IsEmpty()); + EXPECT_THAT(hintsOfKind(AST, InlayHintKind::Type, DefaultInlayHintOpts), + IsEmpty()); } TEST(TypeHints, Decltype) { @@ -1671,7 +1689,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) { )cpp"; assertHintsWithHeader( - InlayHintKind::Type, VectorIntPtr, Header, + InlayHintKind::Type, VectorIntPtr, Header, DefaultInlayHintOpts, ExpectedHint{": int *", "no_modifier"}, ExpectedHint{": int **", "ptr_modifier"}, ExpectedHint{": int *&", "ref_modifier"}, @@ -1695,7 +1713,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) { )cpp"; assertHintsWithHeader( - InlayHintKind::Type, VectorInt, Header, + InlayHintKind::Type, VectorInt, Header, DefaultInlayHintOpts, ExpectedHint{": int", "no_modifier"}, ExpectedHint{": int *", "ptr_modifier"}, ExpectedHint{": int &", "ref_modifier"}, @@ -1722,6 +1740,7 @@ TEST(TypeHints, SubstTemplateParameterAliases) { )cpp"; assertHintsWithHeader(InlayHintKind::Type, TypeAlias, Header, + DefaultInlayHintOpts, ExpectedHint{": Short", "short_name"}, ExpectedHint{": static_vector<int>", "vector_name"}); } @@ -2122,30 +2141,41 @@ TEST(BlockEndHints, PrintRefs) { R"cpp( namespace ns { int Var; - int func(); + int func1(); + int func2(int, int); struct S { int Field; - int method() const; + int method1() const; + int method2(int, int) const; }; // suppress } // suppress void foo() { + int int_a {}; while (ns::Var) { $var[[}]] - while (ns::func()) { - $func[[}]] + while (ns::func1()) { + $func1[[}]] + + while (ns::func2(int_a, int_a)) { + $func2[[}]] while (ns::S{}.Field) { $field[[}]] - while (ns::S{}.method()) { - $method[[}]] + while (ns::S{}.method1()) { + $method1[[}]] + + while (ns::S{}.method2(int_a, int_a)) { + $method2[[}]] } // suppress )cpp", ExpectedHint{" // while Var", "var"}, - ExpectedHint{" // while func", "func"}, + ExpectedHint{" // while func1()", "func1"}, + ExpectedHint{" // while func2(...)", "func2"}, ExpectedHint{" // while Field", "field"}, - ExpectedHint{" // while method", "method"}); + ExpectedHint{" // while method1()", "method1"}, + ExpectedHint{" // while method2(...)", "method2"}); } TEST(BlockEndHints, PrintConversions) { @@ -2305,7 +2335,7 @@ TEST(BlockEndHints, PointerToMemberFunction) { $ptrmem[[}]] } // suppress )cpp", - ExpectedHint{" // if", "ptrmem"}); + ExpectedHint{" // if ()", "ptrmem"}); } // FIXME: Low-hanging fruit where we could omit a type hint: >From 1dceb5be51014cbb332aab32d17162d0821dd74d Mon Sep 17 00:00:00 2001 From: Mythreya <g...@mythreya.dev> Date: Sun, 20 Apr 2025 18:08:32 -0700 Subject: [PATCH 4/4] Add BlockEndHints.MinLineLimit test Add a new test `MinLineLimit` that ensures hints are generated only when the line threshold is met. Limit is set through `InlayHintOptions.HintMinLineLimit` --- .../clangd/unittests/InlayHintTests.cpp | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index e22e670928c1b..bef60f0306f5f 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -2338,6 +2338,44 @@ TEST(BlockEndHints, PointerToMemberFunction) { ExpectedHint{" // if ()", "ptrmem"}); } +TEST(BlockEndHints, MinLineLimit) { + assertBlockEndHintsWithOpts( + R"cpp( + namespace ns { + int Var; + int func1(); + int func2(int, int); + struct S { + int Field; + int method1() const; + int method2(int, int) const; + $struct[[}]]; + $namespace[[}]] + void foo() { + int int_a {}; + while (ns::Var) { + $var[[}]] + + while (ns::func1()) { + $func1[[}]] + + while (ns::func2(int_a, int_a)) { + $func2[[}]] + + while (ns::S{}.Field) { + $field[[}]] + + while (ns::S{}.method1()) { + $method1[[}]] + + while (ns::S{}.method2(int_a, int_a)) { + $method2[[}]] + $foo[[}]] + )cpp", + InlayHintOptions{10}, ExpectedHint{" // namespace ns", "namespace"}, + ExpectedHint{" // foo", "foo"}); +} + // FIXME: Low-hanging fruit where we could omit a type hint: // - auto x = TypeName(...); // - auto x = (TypeName) (...); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits