erichkeane marked an inline comment as done.
erichkeane added inline comments.
================
Comment at: lib/AST/ASTContext.cpp:2200
+ Layout.getBaseClassOffset(Base->getAsCXXRecordDecl());
+ CharUnits BaseSize = Context.getTypeSizeInChars(Base);
+ if (BaseOffset != CurOffset)
----------------
rsmith wrote:
> On reflection, I don't think this is right, and likewise I don't think the
> check for unique object representations of the base class above is quite
> right.
>
> A class can be standard-layout but not POD for the purposes of layout (eg, by
> declaring a special member function). If so, the derived class can reuse the
> base class's tail padding, and if it does, the derived class can have unique
> object representations even when the base class does not. Example:
>
> ```
> struct A { ~A(); int n; char a; }; struct B : A { char b, c, d; };
> ```
>
> Here, under the Itanium C++ ABI, `sizeof(B) == 8`, and `B` has unique object
> representations even though its base class `A` does not.
>
>
> This should be fairly easy to fix. One approach would be to change the
> recursive call to `hasUniqueObjectRepresentations` above to return the actual
> size occupied by the struct and its fields (rather than checking for tail
> padding), and add that size here instead of using the base size. Or you could
> query the DataSize in the record layout rather than getting the (complete
> object) type size (but you'd then still need to check that there's no bits of
> tail padding before the end of the dsize, since we still pad out trailing
> bit-fields in the dsize computation).
According to the standard, the above case isn't, because it is non-trivial,
right? 9.1 requires that "T" (B in your case) be trivially copyable, which it
isn't, right?
The predicate condition for a template specialization
has_unique_object_representations<T> shall be
satisfied if and only if:
(9.1) - T is trivially copyable, and
(9.2) - any two objects of type T with the same value have the same
object representation
https://reviews.llvm.org/D39347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
