[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc1a00b89add8: [clangd] show layout info when hovering on a
class/field definition. (authored by sammccall).
Changed prior to commit:
https://reviews.llvm.org/D77355?vs=254786=255964#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77355/new/
https://reviews.llvm.org/D77355
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -66,7 +66,7 @@
{R"cpp(
namespace ns1 { namespace ns2 {
struct Foo {
- int [[b^ar]];
+ char [[b^ar]];
};
}}
)cpp",
@@ -75,8 +75,10 @@
HI.LocalScope = "Foo::";
HI.Name = "bar";
HI.Kind = index::SymbolKind::Field;
- HI.Definition = "int bar";
- HI.Type = "int";
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
}},
// Local to class method.
{R"cpp(
@@ -100,7 +102,7 @@
{R"cpp(
namespace ns1 { namespace {
struct {
- int [[b^ar]];
+ char [[b^ar]];
} T;
}}
)cpp",
@@ -109,8 +111,21 @@
HI.LocalScope = "(anonymous struct)::";
HI.Name = "bar";
HI.Kind = index::SymbolKind::Field;
- HI.Definition = "int bar";
- HI.Type = "int";
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
+ }},
+ // Struct definition shows size.
+ {R"cpp(
+ struct [[^X]]{};
+ )cpp",
+ [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.Name = "X";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct X {}";
+ HI.Size = 1;
}},
// Variable with template type
{R"cpp(
@@ -698,6 +713,8 @@
EXPECT_EQ(H->TemplateParameters, Expected.TemplateParameters);
EXPECT_EQ(H->SymRange, Expected.SymRange);
EXPECT_EQ(H->Value, Expected.Value);
+EXPECT_EQ(H->Size, Expected.Size);
+EXPECT_EQ(H->Offset, Expected.Offset);
}
}
@@ -1862,6 +1879,7 @@
{
[](HoverInfo ) {
HI.Kind = index::SymbolKind::Class;
+HI.Size = 10;
HI.TemplateParameters = {
{std::string("typename"), std::string("T"), llvm::None},
{std::string("typename"), std::string("C"),
@@ -1875,6 +1893,7 @@
},
R"(class foo
+Size: 10 bytes
documentation
template class Foo {})",
@@ -1911,19 +1930,23 @@
},
{
[](HoverInfo ) {
-HI.Kind = index::SymbolKind::Variable;
-HI.LocalScope = "test::bar::";
+HI.Kind = index::SymbolKind::Field;
+HI.LocalScope = "test::Bar::";
HI.Value = "value";
HI.Name = "foo";
HI.Type = "type";
HI.Definition = "def";
+HI.Size = 4;
+HI.Offset = 12;
},
- R"(variable foo
+ R"(field foo
Type: type
Value = value
+Offset: 12 bytes
+Size: 4 bytes
-// In test::bar
+// In test::Bar
def)",
},
};
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -70,6 +70,10 @@
llvm::Optional> TemplateParameters;
/// Contains the evaluated value of the symbol if available.
llvm::Optional Value;
+ /// Contains the byte-size of fields and types where it's interesting.
+ llvm::Optional Size;
+ /// Contains the offset of fields within the enclosing class.
+ llvm::Optional Offset;
/// Produce a user-readable information.
markup::Document present() const;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -651,6 +651,28 @@
return punctuationIndicatesLineBreak(Line) || isHardLineBreakIndicator(Rest);
}
+void addLayoutInfo(const NamedDecl , HoverInfo ) {
+ const auto = ND.getASTContext();
+
+ if (auto *RD = llvm::dyn_cast()) {
+if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl()))
+ HI.Size = Size->getQuantity();
+return;
+ }
+
+ if (const auto *FD = llvm::dyn_cast()) {
+const auto *Record = FD->getParent()->getDefinition();
+if (Record && !Record->isDependentType()) {
+ uint64_t OffsetBits = Ctx.getFieldOffset(FD);
+ if (auto Size =
[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.
kadircet accepted this revision.
kadircet marked an inline comment as done.
kadircet added a comment.
This revision is now accepted and ready to land.
thanks!
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:69
struct Foo {
- int [[b^ar]];
+ char [[b^ar]];
};
sammccall wrote:
> kadircet wrote:
> > any reason for changing these from int to char ?
> hardcoding sizeof(int) isn't portable unless we want to set the target
> explicitly
right, the test already sets the target though.
I suppose relying less on it is better, so feel free to keep it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77355/new/
https://reviews.llvm.org/D77355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.
sammccall added inline comments.
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:69
struct Foo {
- int [[b^ar]];
+ char [[b^ar]];
};
kadircet wrote:
> any reason for changing these from int to char ?
hardcoding sizeof(int) isn't portable unless we want to set the target
explicitly
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77355/new/
https://reviews.llvm.org/D77355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.
sammccall updated this revision to Diff 254786.
sammccall marked 5 inline comments as done.
sammccall added a comment.
address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77355/new/
https://reviews.llvm.org/D77355
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -66,7 +66,7 @@
{R"cpp(
namespace ns1 { namespace ns2 {
struct Foo {
- int [[b^ar]];
+ char [[b^ar]];
};
}}
)cpp",
@@ -75,8 +75,10 @@
HI.LocalScope = "Foo::";
HI.Name = "bar";
HI.Kind = index::SymbolKind::Field;
- HI.Definition = "int bar";
- HI.Type = "int";
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
}},
// Local to class method.
{R"cpp(
@@ -100,7 +102,7 @@
{R"cpp(
namespace ns1 { namespace {
struct {
- int [[b^ar]];
+ char [[b^ar]];
} T;
}}
)cpp",
@@ -109,8 +111,21 @@
HI.LocalScope = "(anonymous struct)::";
HI.Name = "bar";
HI.Kind = index::SymbolKind::Field;
- HI.Definition = "int bar";
- HI.Type = "int";
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
+ }},
+ // Struct definition shows size.
+ {R"cpp(
+ struct [[^X]]{};
+ )cpp",
+ [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.Name = "X";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct X {}";
+ HI.Size = 1;
}},
// Variable with template type
{R"cpp(
@@ -646,6 +661,8 @@
EXPECT_EQ(H->TemplateParameters, Expected.TemplateParameters);
EXPECT_EQ(H->SymRange, Expected.SymRange);
EXPECT_EQ(H->Value, Expected.Value);
+EXPECT_EQ(H->Size, Expected.Size);
+EXPECT_EQ(H->Offset, Expected.Offset);
}
}
@@ -1810,6 +1827,7 @@
{
[](HoverInfo ) {
HI.Kind = index::SymbolKind::Class;
+HI.Size = 10;
HI.TemplateParameters = {
{std::string("typename"), std::string("T"), llvm::None},
{std::string("typename"), std::string("C"),
@@ -1823,6 +1841,7 @@
},
R"(class foo
+Size: 10 bytes
documentation
template class Foo {})",
@@ -1859,19 +1878,23 @@
},
{
[](HoverInfo ) {
-HI.Kind = index::SymbolKind::Variable;
-HI.LocalScope = "test::bar::";
+HI.Kind = index::SymbolKind::Field;
+HI.LocalScope = "test::Bar::";
HI.Value = "value";
HI.Name = "foo";
HI.Type = "type";
HI.Definition = "def";
+HI.Size = 4;
+HI.Offset = 12;
},
- R"(variable foo
+ R"(field foo
Type: type
Value = value
+Offset: 12 bytes
+Size: 4 bytes
-// In test::bar
+// In test::Bar
def)",
},
};
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -70,6 +70,10 @@
llvm::Optional> TemplateParameters;
/// Contains the evaluated value of the symbol if available.
llvm::Optional Value;
+ /// Contains the byte-size of fields and types where it's interesting.
+ llvm::Optional Size;
+ /// Contains the offset of fields within the enclosing class.
+ llvm::Optional Offset;
/// Produce a user-readable information.
markup::Document present() const;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -563,6 +563,28 @@
isFollowedByHardLineBreakIndicator(Str, LineBreakIndex);
}
+void addLayoutInfo(const NamedDecl , HoverInfo ) {
+ const auto = ND.getASTContext();
+
+ if (auto *RD = llvm::dyn_cast()) {
+if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl()))
+ HI.Size = Size->getQuantity();
+return;
+ }
+
+ if (const auto *FD = llvm::dyn_cast()) {
+const auto *Record = FD->getParent()->getDefinition();
+if (Record && !Record->isDependentType()) {
+ uint64_t OffsetBits = Ctx.getFieldOffset(FD);
+ if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
+HI.Size = Size->getQuantity();
+HI.Offset = OffsetBits / 8;
+ }
+}
+return;
+ }
+}
+
} //
[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.
kadircet added inline comments.
Comment at: clang-tools-extra/clangd/Hover.cpp:571
+if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl()))
+ HI.Size = Size->getQuantity();
+
QuantityType seems to be an alias for int64_t, not sure how likely it is that
someone will have a type that's bigger than 4GB, but still, should we make
typeof HoverInfo::Size optional instead of unsigned?
Comment at: clang-tools-extra/clangd/Hover.cpp:573
+
+ if (const auto *FD = llvm::dyn_cast()) {
+const auto *Record = FD->getParent()->getDefinition();
nit: either make this `else if` or put a return above ?
Comment at: clang-tools-extra/clangd/Hover.cpp:576
+if (Record && !Record->isDependentType()) {
+ unsigned OffsetBits = Ctx.getFieldOffset(FD);
+ if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
similar to size, offset is also of type uint64_t
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:69
struct Foo {
- int [[b^ar]];
+ char [[b^ar]];
};
any reason for changing these from int to char ?
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:1830
HI.Kind = index::SymbolKind::Class;
+HI.Size = 10;
HI.TemplateParameters = {
maybe also add offset?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77355/new/
https://reviews.llvm.org/D77355
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
This triggers only on the definition itself, not on references (probably too
noisy). Inspecting the definition seems like a decent hint for being interested
in layout.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D77355
Files:
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h
clang-tools-extra/clangd/unittests/HoverTests.cpp
Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -66,7 +66,7 @@
{R"cpp(
namespace ns1 { namespace ns2 {
struct Foo {
- int [[b^ar]];
+ char [[b^ar]];
};
}}
)cpp",
@@ -75,8 +75,10 @@
HI.LocalScope = "Foo::";
HI.Name = "bar";
HI.Kind = index::SymbolKind::Field;
- HI.Definition = "int bar";
- HI.Type = "int";
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
}},
// Local to class method.
{R"cpp(
@@ -100,7 +102,7 @@
{R"cpp(
namespace ns1 { namespace {
struct {
- int [[b^ar]];
+ char [[b^ar]];
} T;
}}
)cpp",
@@ -109,8 +111,21 @@
HI.LocalScope = "(anonymous struct)::";
HI.Name = "bar";
HI.Kind = index::SymbolKind::Field;
- HI.Definition = "int bar";
- HI.Type = "int";
+ HI.Definition = "char bar";
+ HI.Type = "char";
+ HI.Offset = 0;
+ HI.Size = 1;
+ }},
+ // Struct definition shows size.
+ {R"cpp(
+ struct [[^X]]{};
+ )cpp",
+ [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.Name = "X";
+ HI.Kind = index::SymbolKind::Struct;
+ HI.Definition = "struct X {}";
+ HI.Size = 1;
}},
// Variable with template type
{R"cpp(
@@ -646,6 +661,8 @@
EXPECT_EQ(H->TemplateParameters, Expected.TemplateParameters);
EXPECT_EQ(H->SymRange, Expected.SymRange);
EXPECT_EQ(H->Value, Expected.Value);
+EXPECT_EQ(H->Size, Expected.Size);
+EXPECT_EQ(H->Offset, Expected.Offset);
}
}
@@ -1810,6 +1827,7 @@
{
[](HoverInfo ) {
HI.Kind = index::SymbolKind::Class;
+HI.Size = 10;
HI.TemplateParameters = {
{std::string("typename"), std::string("T"), llvm::None},
{std::string("typename"), std::string("C"),
@@ -1823,6 +1841,7 @@
},
R"(class foo
+Size: 10 bytes
documentation
template class Foo {})",
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -70,6 +70,10 @@
llvm::Optional> TemplateParameters;
/// Contains the evaluated value of the symbol if available.
llvm::Optional Value;
+ /// Contains the byte-size of fields and types where it's interesting.
+ llvm::Optional Size;
+ /// Contains the offset of fields within the enclosing class.
+ llvm::Optional Offset;
/// Produce a user-readable information.
markup::Document present() const;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -563,6 +563,25 @@
isFollowedByHardLineBreakIndicator(Str, LineBreakIndex);
}
+void addLayoutInfo(const NamedDecl , HoverInfo ) {
+ const auto = ND.getASTContext();
+
+ if (auto *RD = llvm::dyn_cast())
+if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl()))
+ HI.Size = Size->getQuantity();
+
+ if (const auto *FD = llvm::dyn_cast()) {
+const auto *Record = FD->getParent()->getDefinition();
+if (Record && !Record->isDependentType()) {
+ unsigned OffsetBits = Ctx.getFieldOffset(FD);
+ if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
+HI.Size = Size->getQuantity();
+HI.Offset = OffsetBits / 8;
+ }
+}
+ }
+}
+
} // namespace
llvm::Optional getHover(ParsedAST , Position Pos,
@@ -619,6 +638,9 @@
auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias);
if (!Decls.empty()) {
HI = getHoverContents(Decls.front(), Index);
+// Layout info only shown when hovering on the field/class itself.
+if (Decls.front() == N->ASTNode.get())
+ addLayoutInfo(*Decls.front(), *HI);
// Look for a close enclosing expression to show the value of.
if
