[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Please add tests for the member pointer cases across various different targets.



================
Comment at: lib/AST/ASTContext.cpp:2183-2184
+  for (const auto *Field : RD->fields()) {
+    if (!Context.hasUniqueObjectRepresentations(Field->getType()))
+      return false;
+    int64_t FieldSizeInBits =
----------------
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.


================
Comment at: lib/AST/ASTContext.cpp:2213
+        Field->isBitField()
+            ? Field->getBitWidthValue(Context)
+            : Context.toBits(Context.getTypeSizeInChars(Field->getType()));
----------------
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.


================
Comment at: lib/AST/MicrosoftCXXABI.cpp:253
+
+  MPI.HasPadding = MPI.Width % MPI.Align == 0;
 
----------------
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.


https://reviews.llvm.org/D39347