rsmith added inline comments.
================
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.
Sounds like you've found more GCC bugs. In C++ (unlike in C), there is no
restriction on the maximum length of a bit-field. Specifically, [class.bit]p1
says: "The value of the integral constant expression may be larger than the
number of bits in the object
representation (6.7) of the bit-field’s type; in such cases the extra bits are
padding bits (6.7)."
For non-`bool` bit-fields, or `bool` bit-fields with more than 8 bits, the
extra bits are padding, and the type does not have unique object
representations. You'll need to check for this.
For `bool` bit-fields of length 1, we obviously have unique object
representations for that one bit.
The interesting case is `bool` bit-fields with length between 2 and 8
inclusive. It would seem reasonable to assume they have unique representations,
as we do for plain `bool` values, but this is really an ABI question (as,
actually, is the plain `bool` case). The x86-64 psABI is not very clear on the
bit-field question, but it does say "Booleans, when stored in a memory object,
are stored as single byte objects the value of which is always 0 (false) or 1
(true).", so for at least x86-64, `bool`s of up to 8 bits should be considered
to have unique object representations. The PPC64 psABI appears to say the same
thing, but is also somewhat unclear about the bit-field case.
================
Comment at: lib/AST/ASTContext.cpp:2277
+
+ return false;
+}
----------------
More cases to handle here:
* vectors (careful about, eg, vector of 3 foo)
* `_Complex int` and friends
* `_Atomic T`
* Obj-C block pointers
* Obj-C object pointers
* and perhaps OpenCL's various builtin types (pipe, sampler_t, event_t,
clk_event_t, queue_t, reserve_id_t)
It's fine to leave these for another change, but we shouldn't forget about
them. (There're also Obj-C class types and the Obj-C selector type, but I think
it makes sense for those to return `false` here.)
https://reviews.llvm.org/D39347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
