[PATCH] D45174: non-zero-length bit-fields should make a class non-empty

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: majnemer.
rnk added a comment.

In https://reviews.llvm.org/D45174#1054820, @rsmith wrote:

> +rnk This might also affect the MS ABI, but it does not result in any test case failures at least (and MSVC's type trait matches our state after this patch rather than before).


I've convinced myself that this is NFC for MS record layout because this is the only place we use RD->isEmpty() that matters:

  if (!FoundBase) {
    if (MDCUsesEBO && BaseDecl->isEmpty() &&
        BaseLayout.getNonVirtualSize() == CharUnits::Zero()) {
      BaseOffset = CharUnits::Zero();
    } else {
      // Otherwise, lay the base out at the end of the MDC.
      BaseOffset = Size = Size.alignTo(Info.Alignment);
    }
  }

In particular, that `getNonVirtualSize()` check returns false when unnamed bitfields get involved. Here's a layout:

  struct __declspec(empty_bases) A { char : 3; };
  struct __declspec(empty_bases) B { char : 3; };
  struct UseEmpty : A, B {
    char space;
  } o[1];
  static_assert(sizeof(o) == 3, "incorrect layout");
  
  
  *** Dumping AST Record Layout
           0 | struct A (empty)
       0:0-2 |   char
             | [sizeof=1, align=1,
             |  nvsize=1, nvalign=1]
  
  *** Dumping AST Record Layout
           0 | struct B (empty)
       0:0-2 |   char
             | [sizeof=1, align=1,
             |  nvsize=1, nvalign=1]
  
  *** Dumping AST Record Layout
           0 | struct UseEmpty
           0 |   struct A (base) (empty)
       0:0-2 |     char
           1 |   struct B (base) (empty)
       1:0-2 |     char
           2 |   char space
             | [sizeof=3, align=1,
             |  nvsize=3, nvalign=1]

So even though these classes went from "empty" to "non-empty", we previously allocated space for them. I think @majnemer spent a while on that.


Repository:
  rC Clang

https://reviews.llvm.org/D45174