rsmith added inline comments.
================ Comment at: lib/AST/MicrosoftCXXABI.cpp:253 + + MPI.HasPadding = MPI.Width % MPI.Align == 0; ---------------- erichkeane wrote: > rsmith wrote: > > rsmith wrote: > > > erichkeane wrote: > > > > 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? > > > I think the idea is that a struct with one pointer and an odd number of > > > ints, on 32-bit Windows, will have no padding per se, but its size will > > > simply not be a multiple of its alignment. (So struct layout can put > > > things in the four bytes after it, but will insert padding before it to > > > place it at an 8 byte boundary.) > > See example here: https://godbolt.org/g/Nr8C2L > Well... oh dear. > > That would mean that: > auto p = &A::N; > static_assert(!has_unique_object_representations_v<decltype(p)>); // not > unique, since it has padding, right? > > struct S { > decltype(p) s; > }; > static_assert(!has_unique_object_representations_v<S>); // also not unique, > since 's' has padding. > > struct S2 { > decltype(p) s; > int a; > }; > static_assert(has_unique_object_representations_v<S2>); // Actually unique > again, since the padding is filled. > > > Do I have this right? The first `static_assert` looks wrong. Should be: ``` static_assert(has_unique_object_representations_v<decltype(p)>); // unique, has no padding ``` Note that `sizeof(decltype(p))` is the size without any padding. The type does not have padding itself, but might induce lead padding (due to alignment) and tail padding (due to size not being divisible by alignment) in objects it's placed within. But I think your current approach will get this all right, so long as `MPI.HasPadding` is only set to `true` in the cases where there actually is padding in the `MPI.Width` bytes of the representation (which only happens when `Width` is rounded up for alignment). https://reviews.llvm.org/D39347 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
