[PATCH] [ms-cxxabi] Correctly compute the size of member pointers

[John McCall]

On Mar 22, 2013, at 4:24 PM, Reid Kleckner <rnk at google.com> wrote:
> On Fri, Mar 22, 2013 at 4:44 PM, John McCall <rjmccall at apple.com> wrote:
> On Mar 22, 2013, at 11:41 AM, Reid Kleckner <rnk at google.com> wrote:
> >    - Update the test case to pass under x86_64.
> >
> > Hi rjmccall,
> >
> > http://llvm-reviews.chandlerc.com/D568
> 
> +    // FIXME: Handling x86_64 will require changing this method to return sizes
> +    // in bytes instead of pointers.  Some member pointer sizes are not
> +    // multiples of a pointer size.
> 
> So, change the method to return sizes in bytes instead of pointers instead
> of reporting a diagnostic in a core AST routine.
> 
> OK, I'll change the method.  I assumed there'd be more than one caller.  :)
> 
> What should I do for non-x86 architectures then?  I could assert, since presumably we'll have errorred out much earlier if someone asks the frontend for mips code.

I'd formalize it as a difference between 32-bit and 64-bit, but yes, you don't need to go completely hog-wild supporting the MS ABI on non-MS-supported targets.

> There's prior art for issuing diagnostics in AST from RecordLayoutBuilder, but maybe that's different.

Record layout is only done at most once.  It's not *evil*, since this is an "unsupported" diagnostic, but yeah, there's no reason to not just get it right here.

> +// getNumBases() seems to only give us the number of direct bases, and not the
> +// total.  This function tells us if we inherit from anybody that uses MI.
> +static bool recordHasMultipleInheritance(const CXXRecordDecl *RD) {
> +  while (RD->getNumBases() > 0) {
> +    if (RD->getNumBases() > 1)
> +      return true;
> +    assert(RD->getNumBases() == 1);
> +    RD = RD->bases_begin()->getType()->getAsCXXRecordDecl();
> +  }
> +  return false;
> +}
> 
> Hmm.  I bet that non-primary base class forces the "multiple inheritance" case:
>   struct A { int x; void bar(); };
>   struct B : A { virtual void foo(); };
> What's sizeof(void (B::*)())?
> 
> Nice catch, 2 * sizeof(void*), so it's the "multiple" case.  Added a test case.
> 
> What do I need to look for to identify this case?  A class that is polymorphic, but whose base is not?

That would work.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130322/d95998f6/attachment.html>