Ping? (the patch is still the same, as none of the affected files have been changed since Dec 29) Timur Iskhodzhanov, Google Russia
2012/12/29 Timur Iskhodzhanov <[email protected]>: > Please review the new patch attached. > > I've put an extra FIXME for the RTTI handling > and added a new check to the constructor CodeGen test. > > Timur Iskhodzhanov, > Google Russia > > > > 2012/12/29 Timur Iskhodzhanov <[email protected]>: >> 2012/12/4 Timur Iskhodzhanov <[email protected]> >>> > Okay, but I'm curious about something here. It looks like you're not even >>> > allocating space in the v-table for the RTTI pointer if RTTI is disabled? >>> Yes - but it turned out this is not necessary. >>> When I was working on the patch initially the COFF writer formatted >>> vftables wrong, so I thought this was a problem. >>> Now that COFF is fixed, turns out I can put RTTI back - thanks for noticing! >> I take my words back (again). >> The vftable must have AddressPoint=0 in the single-inheritance case; >> I've found this while working on the virtual dtor patch. >> >> If the AddressPoint=4 and the class looks like this: >> struct C { >> virtual void foo(); >> virtual ~C(); >> } >> the call to foo() jumps into ~C() instead if the constructor is >> compiled by Clang (works OK if compiled by VC). >> >> I'll prepare a newer vtable layout patch to fix that. >> >>> > That means that every translation unit using the class needs to agree >>> > about whether RTTI is enabled, or else they might miscompile. If that's >>> > what MSVC does, fine, but if not and what you're really trying to do is >>> > just punt on RTTI for now, why not just always emit RTTI pointers as null? >>> > >>> >> b) I had to remove "AddressPoint != 0" assertion since AddressPoint >>> >> should be 0 for some classes in -cxx-abi microsoft, >>> > >>> > You could weaken the assertion instead of removing it. >>> it's uint64_t, so weakening the "x != 0" assertion is just assert('x >>> is any'), right? :) >>> However, now that I've put RTTI pointer back, I can put the assertion >>> back as well. >> I'll have to remove the assertion again :( _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
