[PATCH] Add MicrosoftVFTableContext to AST

[John McCall]

On Jul 22, 2013, at 4:10 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> 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.

Interesting.  Yeah, that deserves to have its own linkage specifier in LLVM IR.

>> +  // 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?

I think putting a comment on the recursive method stating what you expect
to be in MethodInfoMap after each iteration would make a lot of sense.

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