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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
