[clangd] Add padding to struct hover information #115665
Conversation
HI.Padding is already available for a struct's member variables but the struct itself is not displaying how much padding has been added to it. This commit sums up all padding within a struct and adds it to a struct's hover information.
|
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. |
|
@llvm/pr-subscribers-clangd @llvm/pr-subscribers-clang-tools-extra Author: Kevin Kremer (Xoltus) ChangesHi team, I've written a small patch as a small quality of life improvement for my development and figured it may be worth sharing. Please let me know what you think: HI.Padding is already available for a struct's member variables but the struct itself is not displaying how much padding has been added to it. This commit sums up all padding within a struct and adds it to a struct's hover information. results in I've also added additional tests to HoverTests.cpp. Full diff: https://github.com/llvm/llvm-project/pull/115665.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 298fa79e3fd0ba..9ef32a777d73fe 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -1006,6 +1006,33 @@ void addLayoutInfo(const NamedDecl &ND, HoverInfo &HI) {
HI.Size = Size->getQuantity() * 8;
if (!RD->isDependentType() && RD->isCompleteDefinition())
HI.Align = Ctx.getTypeAlign(RD->getTypeForDecl());
+ if (HI.Size && !RD->field_empty()) {
+ HI.Padding = 0;
+ const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(RD);
+ auto NumFields = std::distance(RD->field_begin(), RD->field_end());
+ auto FieldIt = RD->field_begin();
+ for (; --NumFields; ++FieldIt) {
+ unsigned Offset = Layout.getFieldOffset(FieldIt->getFieldIndex());
+ unsigned NextOffset =
+ Layout.getFieldOffset(FieldIt->getFieldIndex() + 1);
+ if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FieldIt->getType())) {
+ unsigned EndOfField = Offset + Size->getQuantity() * 8;
+ if (NextOffset > EndOfField)
+ HI.Padding = *HI.Padding + NextOffset - EndOfField;
+ } else {
+ HI.Padding.reset();
+ break;
+ }
+ }
+ // We've processed all but the last field. If we still have valid
+ // HI.Padding, finish the calculation
+ auto Size = Ctx.getTypeSizeInCharsIfKnown(FieldIt->getType());
+ if (HI.Padding && Size) {
+ unsigned Offset = Layout.getFieldOffset(FieldIt->getFieldIndex());
+ HI.Padding = *HI.Padding + *HI.Size - Offset - Size->getQuantity() * 8;
+ }
+ }
+
return;
}
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 69f6df46c87ce0..96fb4d8f317328 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -73,6 +73,68 @@ TEST(Hover, Structured) {
HI.Type = "void ()";
HI.Parameters.emplace();
}},
+ {R"cpp(
+ struct [[F^oo]] {
+ char a;
+ long long b;
+ };
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "Foo";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct Foo {}";
+ HI.Size = 128;
+ HI.Padding = 56;
+ HI.Align = 64;
+ }},
+ {R"cpp(
+ struct [[F^oo]] {
+ int b;
+ char a;
+ };
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "Foo";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct Foo {}";
+ HI.Size = 64;
+ HI.Padding = 24;
+ HI.Align = 32;
+ }},
+ {R"cpp(
+ struct [[F^oo]] {
+ double a;
+ char b;
+ double c;
+ };
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "Foo";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct Foo {}";
+ HI.Size = 192;
+ HI.Padding = 46;
+ HI.Align = 64;
+ }}, {R"cpp(
+ struct [[F^oo]] {
+ double a;
+ char b;
+ double c;
+ char d;
+ };
+ )cpp",
+ [](HoverInfo &HI) {
+ HI.NamespaceScope = "";
+ HI.Name = "Foo";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct Foo {}";
+ HI.Size = 256;
+ HI.Padding = 112;
+ HI.Align = 64;
+ }},
// Field
{R"cpp(
namespace ns1 { namespace ns2 {
|
You can test this locally with the following command:git-clang-format --diff 028ea71fdda0c02cd11421cd1d26bec6f378666e 3b64d9d82047159fbd7cb4662d1b666c4431be9c --extensions cpp -- clang-tools-extra/clangd/Hover.cpp clang-tools-extra/clangd/unittests/HoverTests.cppView the diff from clang-format here.diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 96fb4d8f31..269b1f2faf 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -118,7 +118,8 @@ TEST(Hover, Structured) {
HI.Size = 192;
HI.Padding = 46;
HI.Align = 64;
- }}, {R"cpp(
+ }},
+ {R"cpp(
struct [[F^oo]] {
double a;
char b;
|
2 participants
Hi team, I've written a small patch as a small quality of life improvement for my development and figured it may be worth sharing. Please let me know what you think:
HI.Padding is already available for a struct's member variables but the struct itself is not displaying how much padding has been added to it. This commit sums up all padding within a struct and adds it to a struct's hover information.
results in
I've also added additional tests to HoverTests.cpp.