erichkeane added inline comments.
================
Comment at: lib/AST/ASTContext.cpp:2183-2184
+ for (const auto *Field : RD->fields()) {
+ if (!Context.hasUniqueObjectRepresentations(Field->getType()))
+ return false;
+ int64_t FieldSizeInBits =
----------------
rsmith wrote:
> erichkeane wrote:
> > rsmith wrote:
> > > What should happen for fields of reference type?
> > References are not trivially copyable, so they will prevent the struct from
> > having a unique object representation.
> That sounds like the wrong behavior to me. If two structs have references
> that bind to the same object, then they have the same object representation,
> so the struct does have unique object representations.
I didn't think of it that way... I WILL note that GCC rejects references in
their implementation, but that could be a bug on their part.
================
Comment at: lib/AST/ASTContext.cpp:2213
+ Field->isBitField()
+ ? Field->getBitWidthValue(Context)
+ : Context.toBits(Context.getTypeSizeInChars(Field->getType()));
----------------
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...
================
Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+ MPI.HasPadding = MPI.Width % MPI.Align == 0;
----------------
rsmith wrote:
> This seems to be backwards?
>
> Also, I'm not sure whether this is correct. In the strange case where `Width`
> is not a multiple of `Align` (because we don't round up the width), there is
> no padding. We should only set `HasPadding` to `true` in the `alignTo` case
> below.
I think you're right about it being backwards.
However, doesn't the struct with a single Ptr and either 1 or 3 Ints have tail
padding as well? I do believe you're right about the alignTo situation below,
but only if Width changes, right?
https://reviews.llvm.org/D39347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
