[PATCH] D77355: [clangd] show layout info when hovering on a class/field definition.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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<int64_t> instead of unsigned?


================
Comment at: clang-tools-extra/clangd/Hover.cpp:573
+
+  if (const auto *FD = llvm::dyn_cast<FieldDecl>(&ND)) {
+    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