[PATCH] Add MicrosoftVFTableContext to AST

[Timur Iskhodzhanov]

I've updated the patch, it's a bit shorter now :)

2013/7/20 John McCall <rjmccall at apple.com>:
> On Jul 19, 2013, at 8:33 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>
> 2013/7/19 John McCall <rjmccall at apple.com>:
>
> +  void LayoutVFTable() {
>
> +    // FIXME: unclear what to do with RTTI in MS ABI as emitting it
> anywhere
> +    // breaks the vftable layout. Just skip RTTI for now, can't mangle
> anyway.
> +
>
> Can you explain how it breaks the vftable layout?  My understanding is that
> disabling RTTI actually changes the ABI by causing the RTTI pointer to
> be skipped in the layout, thus changing all the offsets; is it more than
> that?
>
>
> Please note this comment is copied from
> VTableBuilder::LayoutPrimaryAndSecondaryVTables.
>
> As far as I understand, if we emit RTTI, it should go to vftable[-1].
> Otherwise there's even no slot allocated for it.
> That means the vftable symbol should point in the middle of a section
> (if RTTI is emitted), which is not yet supported by LLVM IIRC (see the
> "Symbol in the middle of a global" thread).
>
>
> Ah, gotcha.  Yes, having LLVM and linker support for this would be great.
>
> I think we should also use weak/strong-odr linkage for no-RTTI/RTTI
> cases so the more complete vftables are chosen at link-time.
>
>
> I thought vf-tables never had strong linkage?

Sorry, I've used a slightly wrong term.
The vftables are COMDAT sections with the "select the largest
definition if different sizes are available" flag.

> +  // See if this class expands a vftable of one of its bases.
> +  const CXXRecordDecl *NextBase = 0, *NextLastVBase = LastVBase;
>
> Why isn't this always just the primary base?  In what situations do
> virtual function entries ever get added to a vf-table from a non-primary
> base?
>
>
> consider
>  struct A;
>  struct B;
>  struct C: B;
>  struct MDC: A, C;
> if C adds new methods to B, they are put into the B's vftable.
> Basically, each class in the hierarchy can add methods to its own
> primary base vftable.
>
>
> Right, but from the perspective of C, they’re going in its primary vf-table
> (which is the same as B-in-C’s primary vf-table).
>
> This is why I was saying (1) identify tables by the most derived base
> subobject for which they’re the primary vf-table and then (2) recurse
> down the primary base chain from there (adding methods as you
> move back towards the most derived base subobject).

Ah I see.
The thing is that I've been clearing the BasePath in case there's only
one vftable in a class.
I've removed this code now, deferring the decision to the mangler.

> You have a lot of very hard-to-follow path logic in this code; it's very
> difficult to review this without understanding the invariants better.
>
>
> I had the same complaint while reading VTableBuilder/VTableContext :)
> That said, I'll be happy to put explaining comments into the code
> where necessary, but it's already hard for me to decide what needs
> more comments...
>
> It seems like MethodInfoMap is basically trying to track the current
> class's notion of the final overriders of all the methods in its primary
> vf-table?
>
>
> Not in the primary vftable but rather in the vftable it's building
> (e.g. first vbase's votable).
>
>
> By "current class”, I meant the class for which you are currently adding
> methods.
>
> Other than that - yes.
> I think my MethodInfoMap is similar to MethodInfoMap from
> VTableBuilder::AddMethods.
> Please note that using FinalOverriders alone would not suffice due to
> a different "this parameter" logic in this ABI.
>
>
> Right, but all that is is a mapping of methods to methods from a
> different base subobject.

Can you please clarify if I should add any comments for this into the code?

> John.