On Jan 16, 2013, at 6:11 AM, Timur Iskhodzhanov <[email protected]> wrote: > 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.
It's really just that I've been very busy recently, as you can probably tell from my commit history. :) I don't have an immediate solution. > 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? The ARM ABI is a variant of the Itanium ABI; my intent was that isItaniumABI would return true for it. But it's not that important; caching IsMicrosoftABI is fine for now. > + 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! Neither of these is appropriate! The compiler should never crash just because something is unsupported. Consider adding a method to emit an "unsupported" diagnostic here (you can follow IRGen's lead). John.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
