[PATCH] Add MicrosoftVFTableContext to AST

[John McCall]

On Jul 29, 2013, at 11:17 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
> 2013/7/29 John McCall <rjmccall at gmail.com>:
>> On Jul 29, 2013, at 3:43 AM, Timur Iskhodzhanov <timurrrr at google.com> wrote:
>> 
>> 2013/7/26 John McCall <rjmccall at gmail.com>:
>> 
>> 
>> In both ABIs, the algorithm for performing a virtual function call is to
>> adjust
>> the base pointer to a subobject which contains the method in its primary
>> v-table, load a function pointer from the v-table, and call it.
>> 
>> In your example, the difference is just that, under MSVC, B doesn’t have
>> an entry for f() in its vf-table.  (In fact, it doesn’t have a vf-table.)
>> So the
>> caller has to adjust to something that actually does have an entry for f(),
>> namely A.
>> 
>> +  // See if this class expands a vftable of the base we look at, which is
>> either
>> +  // the one defined by the vfptr base path or the primary base.
>> 
>> Still not sure why you’re not just starting from the right base and then
>> walking up the primary base chain.
>> 
>> 
>> That's not enough even in the simple case of "B: A".
>> 
>> The vfptr is in the A layout, so the "right base" is A.
>> If we don't go to the most derived class (B) from "the right base"
>> (A), we forget to add the more derived class's own new methods (and
>> probably return-adjusting thunks).
>> 
>> 
>> My point is that you should just start recursing from B instead of this
>> weird
>> combination of walking the path and then falling back on climbing the
>> primary base chain.
>> 
>> Finding B (the most-derived subobject that A is in the primary-base chain
>> of)
>> should be really easy — it’s just a depth-first search through the complete
>> object’s layout, stopping at the first thing with the same offset as A.
>> 
>> 
>> Hm...
>> 
>> Let's consider
>> --------
>> struct A {
>>  virtual TYPE* f();
>> };
>> struct B {
>>  virtual TYPE* g();
>> };
>> struct C: A, B {  <something>  }
>> --------
>> We'll have two vfptrs: for A at offset 0 and for B at offset 4
>> (assuming 32-bit arch).
>> 
>> Currently, for the B's vftable we'll do this:
>> enter AddMethods(C)
>>  enter AddMethods(B)
>>  allocate a vftable slot for B::g
>>  leave AddMethods(B)
>> update the B::g slot with this/return adjustment if C overrides it
>> leave AddMethods(C)
>> [this somewhat reflects how Itanium's VTableContext works]
>> 
>> Basically what you want is
>> enter AddMethods(B)
>> allocate a vftable slot for B::g,
>>  if we have a (final) overrider for g() in C,
>>    calculate this/return adjustment right here*.
>> leave AddMethods(B)
>> 
>> Ok, so at the (*) stage we can probably find the FinalOverrider to
>> just write the adjustments immediately...
>> This implies rewriting ComputeThisOffset to just take the final
>> overrider and return the this offset in a complete object.
>> There were a few minor complexities in rewriting it (e.g. API being
>> not convenient) that have overloaded my brain on Friday evening
>> though.
>> 
>> Do you think this is important for the first version? If so - I'll
>> continue trying to do this next week.
>> 
>> 
>> I think it’s worth it, thanks.
>> 
>> I’ll wait to review that unless there’s something intermediate about
>> the current patch you’d like me to check out.
>> 
>> 
>> Nope, it didn't work well.
>> It turns out we should actually append slots to the vftable of a
>> non-primary subobject if we have a covariant-return override. As a
>> result, we have to visit the more-derived classes after we enumerate
>> all the methods of the primary sub objects.
>> 
>> 
>> That’s really interesting!  Also I feel like that cannot possibly work in
>> the
>> presence of virtual inheritance.  What does MSVC do if you have:
>> 
>> struct C0 { int x; };
>> struct C1 : C0 { virtual void f(); int y; }; // converting C0* to C1* is
>> non-trivial
>> 
>> struct A {
>>  virtual C0 *foo();
>>  virtual C0 *bar();
>> };
>> 
>> struct B : virtual A {
>>  virtual C1 *foo(); // added to the A subobject’s vf-table?
>> };
>> struct C : virtual A {
>>  virtual C1 *bar(); // added to the A subobject’s vf-table?
>> };
>> 
>> strruct D : B, C {} // What’s at the end of the A subobject’s vf-table?
> 
> Hm, that's an interesting case.
> 
> CL doesn't allow us to build it, saying
> vftable.cpp(17) : error C2250: 'D' : ambiguous inheritance of 'C1 *A::foo(void)'
> 
> If I add final overriders to D, the error is still there.
> 
> Does that make sense?

Do they only complain if there’s a bar present which collides?

I’ll look at the patch.

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