[clangd] Show padding following a field on field hover.

[sammccall]

Summary

This displays as: Size: 4 bytes (+4 padding)

Also stop showing (byte) offset/size for bitfields. They're not
meaningful and using them to calculate padding is dangerous!

[njames93]

clang-tools-extra/clangd/unittests/HoverTests.cpp

105

Does it make any sense to include the padding for union fields?

[sammccall]

clang-tools-extra/clangd/unittests/HoverTests.cpp

105

I think so - it tells you how much room you have to grow/are wasting in particular branches of the union. True, it's easy enough to infer by looking at the union size and the field size. But on the other hand, the *purpose* of a union is to get a compact memory layout, so memory layout information is more likely to be useful.

Maybe we should omit Offset though...

[sammccall]

Stop reporting offset for fields in a union, too

[kbobyrev]

LGTM, thanks!

clang-tools-extra/clangd/Hover.h

struct HoverInfo {
   /// Set for all templates(function, class, variable).
   llvm::Optional<std::vector<Param>> TemplateParameters;
   /// Contains the evaluated value of the symbol if available.
   llvm::Optional<std::string> Value;
   /// Contains the byte-size of fields and types where it's interesting.
   llvm::Optional<uint64_t> Size;
   /// Contains the offset of fields within the enclosing class.
   llvm::Optional<uint64_t> Offset;
+  /// Contains the padding following a field within the enclosing class.
+  llvm::Optional<uint64_t> Padding;
   // Set when symbol is inside function call. Contains information extracted
   // from the callee definition about the argument this is passed as.
   llvm::Optional<Param> CalleeArgInfo;
   struct PassType {
     // How the variable is passed to callee.
     enum PassMode { Ref, ConstRef, Value };
     PassMode PassBy = Ref;
     // True if type conversion happened. This includes calls to implicit

clang-tools-extra/clangd/Hover.cpp

 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/PrettyPrinter.h"
+#include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
if (auto Size = Ctx.getTypeSizeInCharsIfKnown(RD->getTypeForDecl()))
       HI.Size = Size->getQuantity();
     return;
   }
 
   if (const auto *FD = llvm::dyn_cast<FieldDecl>(&ND)) {
     const auto *Record = FD->getParent();
     if (Record)
       Record = Record->getDefinition();
-    if (Record && !Record->isInvalidDecl() && !Record->isDependentType()) {
-      HI.Offset = Ctx.getFieldOffset(FD) / 8;
-      if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType()))
-        HI.Size = Size->getQuantity();
+    if (Record && !Record->isInvalidDecl() && !Record->isDependentType() &&
+        !FD->isBitField()) {
+      const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(Record);
+      HI.Offset = Layout.getFieldOffset(FD->getFieldIndex()) / 8;
+      if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
+        HI.Size = FD->isZeroSize(Ctx) ? 0 : Size->getQuantity();
+        unsigned EndOfField = *HI.Offset + *HI.Size;
+
+        // Calculate padding following the field.
+        if (!Record->isUnion() &&
+            FD->getFieldIndex() + 1 < Layout.getFieldCount()) {
+          // Measure padding up to the next class field.
+          unsigned NextOffset =
+              Layout.getFieldOffset(FD->getFieldIndex() + 1) / 8;
+          if (NextOffset >= EndOfField) // next field could be a bitfield!
+            HI.Padding = NextOffset - EndOfField;
+        } else {
+          // Measure padding up to the end of the object.
+          HI.Padding = Layout.getSize().getQuantity() - EndOfField;
+        }
+      }
+      // Offset in a union is always zero, so not really useful to report.
+      if (Record->isUnion())
+        HI.Offset.reset();
     }
     return;
   }
 }
 
 // If N is passed as argument to a function, fill HI.CalleeArgInfo with
 // information about that argument.
 void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
if (Value) {
     P.appendText("Value = ");
     P.appendCode(*Value);
   }
 
   if (Offset)
     Output.addParagraph().appendText(
         llvm::formatv("Offset: {0} byte{1}", *Offset, *Offset == 1 ? "" : "s")
             .str());
-  if (Size)
-    Output.addParagraph().appendText(
+  if (Size) {
+    auto &P = Output.addParagraph().appendText(
         llvm::formatv("Size: {0} byte{1}", *Size, *Size == 1 ? "" : "s").str());
+    if (Padding && *Padding != 0)
+      P.appendText(llvm::formatv(" (+{0} padding)", *Padding).str());
+  }
 
   if (CalleeArgInfo) {
     assert(CallPassType);
     std::string Buffer;
     llvm::raw_string_ostream OS(Buffer);
     OS << "Passed ";
     if (CallPassType->PassBy != HoverInfo::PassType::Value) {
       OS << "by ";

clang-tools-extra/clangd/unittests/HoverTests.cpp

const std::function<void(HoverInfo &)> ExpectedBuilder;
          HI.Definition = "void foo()";
          HI.ReturnType = "void";
          HI.Type = "void ()";
          HI.Parameters.emplace();
        }},
       // Field
       {R"cpp(
           namespace ns1 { namespace ns2 {
-            struct Foo {
+            class Foo {
               char [[b^ar]];
+              double y[2];
             };
           }}
           )cpp",
        [](HoverInfo &HI) {
          HI.NamespaceScope = "ns1::ns2::";
          HI.LocalScope = "Foo::";
          HI.Name = "bar";
          HI.Kind = index::SymbolKind::Field;
          HI.Definition = "char bar";
          HI.Type = "char";
          HI.Offset = 0;
          HI.Size = 1;
+         HI.Padding = 7;
+         HI.AccessSpecifier = "private";
+       }},
+      // Union field
+      {R"cpp(
+            union Foo {
+              char [[b^ar]];
+              double y[2];
+            };
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.LocalScope = "Foo::";
+         HI.Name = "bar";
+         HI.Kind = index::SymbolKind::Field;
+         HI.Definition = "char bar";
+         HI.Type = "char";
+         HI.Size = 1;
+         HI.Padding = 15;
+         HI.AccessSpecifier = "public";
+       }},
+      // Bitfield
+      {R"cpp(
+            struct Foo {
+              int [[^x]] : 1;
+              int y : 1;
+            };
+          )cpp",
+       [](HoverInfo &HI) {
+         HI.NamespaceScope = "";
+         HI.LocalScope = "Foo::";
+         HI.Name = "x";
+         HI.Kind = index::SymbolKind::Field;
+         HI.Definition = "int x : 1";
+         HI.Type = "int";
          HI.AccessSpecifier = "public";
        }},
       // Local to class method.
       {R"cpp(
           namespace ns1 { namespace ns2 {
             struct Foo {
               void foo() {
                 int [[b^ar]];
template <typename T, typename C = bool> class Foo {})",
             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;
+            HI.Padding = 4;
           },
           R"(field foo
 
 Type: type
 Value = value
 Offset: 12 bytes
-Size: 4 bytes
+Size: 4 bytes (+4 padding)
 
 // In test::Bar
 def)",
       },
       {
           [](HoverInfo &HI) {
             HI.Kind = index::SymbolKind::Field;
             HI.AccessSpecifier = "public";