2013/1/16 John McCall <[email protected]> > On Dec 29, 2012, at 6:48 AM, Timur Iskhodzhanov <[email protected]> > wrote: > > 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. > > Sorry about the delay. > Please tell me if there's something actionable for me to improve my emails/patches to reduce delays/number of iterations. e.g. read these and these docs, read XYZ more carefully, do ABC rather than KLM, etc.
> Instead of repeating > Context.getTargetInfo().getCXXABI() != CXXABI_Microsoft > all over the place, please introduce an isItaniumABI() predicate. > Also, consider caching it as a field of VTableContext. > Unfortunately, I'm afraid havin isItaniumABI() is not very helpful as we may also see CXXABI_ARM. Am I right? Instead, I've added the IsMicrosoftABI field to VTableContext together with the isMicrosoftABI() method. By the way, thanks to the isMicrosoftABI() method it should be easier to find all the places in VTableBuilder.cpp where we have ABI-specific code when we decide to inject some interfaces and abstraction. Did I get your idea right? Or did you mean to add this predicate to ASTContext or TargetInfo ? + assert(!isMicrosoft && > + "Implicit virtual dtors are not yet supported in MS ABI"); > + > > This is not an appropriate use of assert. > I've switched back to llvm_unreachable. Please tell me if I'm doing it wrong too! > Otherwise this looks fine. Attached is an updated version of the patch, can you please review it? Thanks! > > John. >
bug_13231_5.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
