[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Did ore looking into the bitfield shennangins.  Will update this patch tomorrow.  @rsmith if you get a chance, do you perhaps have an opinion on 'bool' fields?  It seems to me that a bool could either be considered an 8 bit value, or a 1 bit value with 7 bits of padding.  So:

  bool b;
  struct S { bool b; };
   // Patch currently lists these as unique.  Should they be: 
  static_assert(!__has_unique_object_representations(b));
  static_assert(!__has_unique_object_representations(S));



================
Comment at: lib/AST/ASTContext.cpp:2213
+        Field->isBitField()
+            ? Field->getBitWidthValue(Context)
+            : Context.toBits(Context.getTypeSizeInChars(Field->getType()));
----------------
erichkeane wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > This isn't quite right; you want min(bitfield length, bit size of type) here. Eg, a `bool b : 8` or `int n : 1000` bitfield has padding bits.
> > I guess I'm missing something with this comment... why would a bitfield with bool b: 8 have padding?
> > 
> > Additionally, isn't int n : 1000 illegal, since 1000 > 32 (# bits in an int)?
> > Both clang and GCC reject the 2nd case.  Curiously, GCC rejects bool b: 9, but Clang seems to allow 'bool' to have ANY size.  Is that an error itself?
> > 
> > Finally: Should we consider ANY bool to be not a unique object representation?  It has 1 bit of data, but 8 bits of storage.  GCC's implementation of this trait accepts them, but I'm second guessing that at the moment...
> I did a bit more looking into this... In the 'TYPE b: X' case, 'X' is always the size of the field, so :
> 
>     struct Bitfield {
>       bool b: 16;
>     };
>     static_assert(sizeof(Bitfield) == 2);
> 
> The rest of my padding detection works correctly.
Umm... so holy crap.  We actually reserve the object spece for the whole thing, but ONLY the first sizeof(field) fields are actually stored to!  Everything else is truncated!

I think the correct solution is to do:
   // too large of a bitfield size causes truncation, and thus 'padding' bits.
   if (FieldSizeInBits > Context.getTypeSizeInBits(Field->getType()) return false;


https://reviews.llvm.org/D39347