From c4e68953f6443b9854ce3ec1dc8756789beafcc1 Mon Sep 17 00:00:00 2001 From: Kadir Cetinkaya Date: Wed, 26 Jan 2022 17:55:52 +0100 Subject: [PATCH] [clangd][Hover] Suppress initializers with many tokens This results in excessive memory usage and eats a lot of screen estate. Especially in the cases with lots of nested macro calls. This patch tries to remedy it before the release cut by suppressing the initializers. For better UX we should probably update the expression printer to truncate those (behind some policy). Fixes https://github.com/clangd/clangd/issues/917 Differential Revision: https://reviews.llvm.org/D118260 --- clang-tools-extra/clangd/Hover.cpp | 19 +++++++++++++++---- .../clangd/unittests/HoverTests.cpp | 14 ++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp index c5cb5cd3df53..d1d8142f53cb 100644 --- a/clang-tools-extra/clangd/Hover.cpp +++ b/clang-tools-extra/clangd/Hover.cpp @@ -126,7 +126,17 @@ std::string getNamespaceScope(const Decl *D) { return ""; } -std::string printDefinition(const Decl *D, const PrintingPolicy &PP) { +std::string printDefinition(const Decl *D, PrintingPolicy PP, + const syntax::TokenBuffer &TB) { + if (auto *VD = llvm::dyn_cast(D)) { + if (auto *IE = VD->getInit()) { + // Initializers might be huge and result in lots of memory allocations in + // some catostrophic cases. Such long lists are not useful in hover cards + // anyway. + if (200 < TB.expandedTokens(IE->getSourceRange()).size()) + PP.SuppressInitializers = true; + } + } std::string Definition; llvm::raw_string_ostream OS(Definition); D->print(OS, PP); @@ -568,7 +578,8 @@ std::string synthesizeDocumentation(const NamedDecl *ND) { /// Generate a \p Hover object given the declaration \p D. HoverInfo getHoverContents(const NamedDecl *D, const PrintingPolicy &PP, - const SymbolIndex *Index) { + const SymbolIndex *Index, + const syntax::TokenBuffer &TB) { HoverInfo HI; const ASTContext &Ctx = D->getASTContext(); @@ -630,7 +641,7 @@ HoverInfo getHoverContents(const NamedDecl *D, const PrintingPolicy &PP, HI.Value = toString(ECD->getInitVal(), 10); } - HI.Definition = printDefinition(D, PP); + HI.Definition = printDefinition(D, PP, TB); return HI; } @@ -1029,7 +1040,7 @@ llvm::Optional getHover(ParsedAST &AST, Position Pos, auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias, AST.getHeuristicResolver()); if (!Decls.empty()) { - HI = getHoverContents(Decls.front(), PP, Index); + HI = getHoverContents(Decls.front(), PP, Index, TB); // Layout info only shown when hovering on the field/class itself. if (Decls.front() == N->ASTNode.get()) addLayoutInfo(*Decls.front(), *HI); diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp index 6f5f6ba7b266..b0974d99f0ac 100644 --- a/clang-tools-extra/clangd/unittests/HoverTests.cpp +++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp @@ -3172,6 +3172,20 @@ TEST(Hover, DisableShowAKA) { EXPECT_EQ(H->Type, HoverInfo::PrintedType("m_int")); } +TEST(Hover, HideBigInitializers) { + Annotations T(R"cpp( + #define A(x) x, x, x, x + #define B(x) A(A(A(A(x)))) + int a^rr[] = {B(0)}; + )cpp"); + + TestTU TU = TestTU::withCode(T.code()); + auto AST = TU.build(); + auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr); + + ASSERT_TRUE(H); + EXPECT_EQ(H->Definition, "int arr[]"); +} } // namespace } // namespace clangd } // namespace clang