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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to