erichkeane added inline comments.
================
Comment at: lib/AST/ASTContext.cpp:2238-2240
+ // All other pointers are unique.
+ if (Ty->isPointerType() || Ty->isMemberPointerType())
+ return true;
----------------
rsmith wrote:
> 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?)
Ah, right... I missed this part of the MicrosoftCXXABI:
if (Target.getTriple().isArch64Bit())
Width = llvm::alignTo(Width, Align);
So likely I can just do the same math as I implemented here, except before the
width is reset in the alignTo here.
Thanks for getting back so quickly! I'll get back on this once I get a patch
for the array align error.
https://reviews.llvm.org/D39347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
