[clangd] Add Config.Hover.MacroContentsLimit #155105
Merged
+63
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
JaagupAverin
changed the title
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clangd Author: Jaagup Averin (JaagupAverin) ChangesCurrently macro expansions are hard capped at 2048. This PR adds the CLI option Full diff: https://github.com/llvm/llvm-project/pull/155105.diff 9 Files Affected:
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index b445dcf2bbd2e..503935a0a4797 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -1268,6 +1268,7 @@ void ClangdLSPServer::onDocumentHighlight(
void ClangdLSPServer::onHover(const TextDocumentPositionParams &Params,
Callback<std::optional<Hover>> Reply) {
Server->findHover(Params.textDocument.uri.file(), Params.position,
+ Opts.HoverContentsLimit,
[Reply = std::move(Reply),
this](llvm::Expected<std::optional<HoverInfo>> H) mutable {
if (!H)
diff --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 6ada3fd9e6e47..7c2c44fea1b4a 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -64,6 +64,9 @@ class ClangdLSPServer : private ClangdServer::Callbacks,
/// Limit the number of references returned (0 means no limit).
size_t ReferencesLimit = 0;
+ /// Limit the number of characters returned when hovering a symbol.
+ size_t HoverContentsLimit = 0;
+
/// Flag to hint the experimental modules support is enabled.
bool EnableExperimentalModulesSupport = false;
};
diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index ac1e9aa5f0ff1..01ec164a56e59 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -843,14 +843,16 @@ void ClangdServer::findDocumentHighlights(
}
void ClangdServer::findHover(PathRef File, Position Pos,
+ size_t HoverContentsLimit,
Callback<std::optional<HoverInfo>> CB) {
- auto Action = [File = File.str(), Pos, CB = std::move(CB),
+ auto Action = [File = File.str(), Pos, HoverContentsLimit, CB = std::move(CB),
this](llvm::Expected<InputsAndAST> InpAST) mutable {
if (!InpAST)
return CB(InpAST.takeError());
format::FormatStyle Style = getFormatStyleForFile(
File, InpAST->Inputs.Contents, *InpAST->Inputs.TFS, false);
- CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index));
+ CB(clangd::getHover(InpAST->AST, Pos, std::move(Style), Index,
+ HoverContentsLimit));
};
WorkScheduler->runWithAST("Hover", File, std::move(Action), Transient);
diff --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 4a1eae188f7eb..6a3fa55d1fcc3 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -270,7 +270,7 @@ class ClangdServer {
Callback<std::vector<DocumentHighlight>> CB);
/// Get code hover for a given position.
- void findHover(PathRef File, Position Pos,
+ void findHover(PathRef File, Position Pos, size_t HoverContentsLimit,
Callback<std::optional<HoverInfo>> CB);
/// Get information about type hierarchy for a given position.
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index a7cf45c632827..f352a16c12ecb 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -755,7 +755,7 @@ HoverInfo evaluateMacroExpansion(unsigned int SpellingBeginOffset,
/// Generate a \p Hover object given the macro \p MacroDecl.
HoverInfo getHoverContents(const DefinedMacro &Macro, const syntax::Token &Tok,
- ParsedAST &AST) {
+ ParsedAST &AST, size_t HoverContentsLimit) {
HoverInfo HI;
SourceManager &SM = AST.getSourceManager();
HI.Name = std::string(Macro.Name);
@@ -795,7 +795,7 @@ HoverInfo getHoverContents(const DefinedMacro &Macro, const syntax::Token &Tok,
for (const auto &ExpandedTok : Expansion->Expanded) {
ExpansionText += ExpandedTok.text(SM);
ExpansionText += " ";
- if (ExpansionText.size() > 2048) {
+ if (HoverContentsLimit && ExpansionText.size() > HoverContentsLimit) {
ExpansionText.clear();
break;
}
@@ -1250,7 +1250,8 @@ void maybeAddUsedSymbols(ParsedAST &AST, HoverInfo &HI, const Inclusion &Inc) {
std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
const format::FormatStyle &Style,
- const SymbolIndex *Index) {
+ const SymbolIndex *Index,
+ size_t HoverContentsLimit) {
static constexpr trace::Metric HoverCountMetric(
"hover", trace::Metric::Counter, "case");
PrintingPolicy PP =
@@ -1297,7 +1298,7 @@ std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
HighlightRange = Tok.range(SM).toCharRange(SM);
if (auto M = locateMacroAt(Tok, AST.getPreprocessor())) {
HoverCountMetric.record(1, "macro");
- HI = getHoverContents(*M, Tok, AST);
+ HI = getHoverContents(*M, Tok, AST, HoverContentsLimit);
if (auto DefLoc = M->Info->getDefinitionLoc(); DefLoc.isValid()) {
include_cleaner::Macro IncludeCleanerMacro{
AST.getPreprocessor().getIdentifierInfo(Tok.text(SM)), DefLoc};
diff --git a/clang-tools-extra/clangd/Hover.h b/clang-tools-extra/clangd/Hover.h
index 614180a7b9846..4dbd7c050421d 100644
--- a/clang-tools-extra/clangd/Hover.h
+++ b/clang-tools-extra/clangd/Hover.h
@@ -163,10 +163,13 @@ inline bool operator==(const HoverInfo::Param &LHS,
std::tie(RHS.Type, RHS.Name, RHS.Default);
}
+constexpr size_t DefaultHoverContentsLimit = 2048;
+
/// Get the hover information when hovering at \p Pos.
-std::optional<HoverInfo> getHover(ParsedAST &AST, Position Pos,
- const format::FormatStyle &Style,
- const SymbolIndex *Index);
+std::optional<HoverInfo>
+getHover(ParsedAST &AST, Position Pos, const format::FormatStyle &Style,
+ const SymbolIndex *Index,
+ size_t HoverContentsLimit = DefaultHoverContentsLimit);
} // namespace clangd
} // namespace clang
diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index df8d075e80596..d103ef3f5aed3 100644
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -441,7 +441,7 @@ class Checker {
unsigned Definitions = locateSymbolAt(*AST, Pos, &Index).size();
vlog(" definition: {0}", Definitions);
- auto Hover = getHover(*AST, Pos, Style, &Index);
+ auto Hover = getHover(*AST, Pos, Style, &Index, Opts.HoverContentsLimit);
vlog(" hover: {0}", Hover.has_value());
unsigned DocHighlights = findDocumentHighlights(*AST, Pos).size();
diff --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index f287439f10cab..5e8755d643d7f 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -328,6 +328,15 @@ opt<int> ReferencesLimit{
init(1000),
};
+opt<int> HoverContentsLimit{
+ "limit-hover-contents",
+ cat(Features),
+ desc("Limit the number of characters returned when hovering a symbol. "
+ "Texts longer than this value will be dropped (not truncated). "
+ "0 means no limit (default=2048)"),
+ init(DefaultHoverContentsLimit),
+};
+
opt<int> RenameFileLimit{
"rename-file-limit",
cat(Features),
@@ -925,6 +934,7 @@ clangd accepts flags on the commandline, and in the CLANGD_FLAGS environment var
Opts.BackgroundIndex = EnableBackgroundIndex;
Opts.BackgroundIndexPriority = BackgroundIndexPriority;
Opts.ReferencesLimit = ReferencesLimit;
+ Opts.HoverContentsLimit = HoverContentsLimit;
Opts.Rename.LimitFiles = RenameFileLimit;
auto PAI = createProjectAwareIndex(
[SupportContainedRefs = Opts.EnableOutgoingCalls](
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 6407349d9af9b..fe0f4334746ca 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -4461,6 +4461,46 @@ constexpr u64 pow_with_mod(u64 a, u64 b, u64 p) {
EXPECT_TRUE(H->Type);
}
+TEST(Hover, HoverContentsLimit) {
+ const char *const Code =
+ R"cpp(
+ #define C(A) A##A // Concatenate
+ #define E(A) C(A) // Expand
+ #define Z0032 00000000000000000000000000000000
+ #define Z0064 E(Z0032)
+ #define Z0128 E(Z0064)
+ #define Z0256 E(Z0128)
+ #define Z0512 E(Z0256)
+ #define Z1024 E(Z0512)
+ #define Z2048 E(Z1024)
+ #define Z4096 E(Z2048) // 4096 zeroes
+ int main() { return [[^Z4096]]; }
+ )cpp";
+
+ struct {
+ size_t HoverContentsLimit;
+ const std::string ExpectedDefinition;
+ } Cases[] = {
+ // With a limit of 2048, the macro expansion should get dropped.
+ {2048, "#define Z4096 E(Z2048)"},
+ // With a limit of 8192, the macro expansion should be fully expanded.
+ {8192, std::string("#define Z4096 E(Z2048)\n\n") +
+ std::string("// Expands to\n") + std::string(4096, '0')},
+ };
+ for (const auto &Case : Cases) {
+ SCOPED_TRACE(Code);
+
+ Annotations T(Code);
+ TestTU TU = TestTU::withCode(T.code());
+ auto AST = TU.build();
+ auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr,
+ Case.HoverContentsLimit);
+ ASSERT_TRUE(H);
+
+ EXPECT_EQ(H->Definition, Case.ExpectedDefinition);
+ }
+};
+
TEST(Hover, FunctionParameters) {
struct {
const char *const Code;
|
|
Some high-level feedback:
|
This option allows to override the default limit of 2048 characters when hovering a macro.
JaagupAverin
force-pushed
the
limit-hover-contents
branch
from
82c5665 to
b3c11b0
Compare
|
Hey. Thanks for the response and patience. |
JaagupAverin
changed the title
JaagupAverin
changed the title
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
HighCommander4
approved these changes
Aug 28, 2025
|
@JaagupAverin Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently macro expansions are hard capped at 2048.
This PR adds the CLI optionThis PR adds the--limit-hover-contentswhich is passed down in place of the hard coded value.Config.Hover.MacroContentsLimitconfig for overriding the default.Resolves #153355.
Documentation: llvm/clangd-www#136