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 :(
bug_13231_4.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
