rsmith added inline comments.

================
Comment at: lib/AST/ASTContext.cpp:2238-2240
+  // All other pointers are unique.
+  if (Ty->isPointerType() || Ty->isMemberPointerType())
+    return true;
----------------
erichkeane wrote:
> rsmith wrote:
> > This is not correct: member pointer representations can have padding bits. 
> > In the MS ABI, a pointer to member function is a pointer followed by 0-3 
> > ints, and if there's an odd number of ints, I expect there'll be 4 bytes of 
> > padding at the end of the representation on 64-bit targets.
> > 
> > I think you'll need to either add a `clang::CXXABI` entry point for 
> > determining whether a `MemberPointerType` has padding, or perhaps extend 
> > the existing `getMemberPointerWidthAndAlign` to also provide this 
> > information.
> Based on looking through the two, I think I can just do this as Width%Align 
> == 0, right?  I've got an incoming patch that does that, so hopefully that is 
> sufficient.
I don't think that will work; the MS implementation *sometimes* rounds the size 
up to the alignment. (... which sounds pretty dodgy to me; shouldn't the 
returned size always be a multiple of the returned 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

Reply via email to