This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix darwin/25908, key functions vs non weak vtables
- From: Mark Mitchell <mark at codesourcery dot com>
- To: Mike Stump <mrs at apple dot com>
- Cc: Geoffrey Keating <geoffk at apple dot com>, Andrew Pinski <pinskia at physics dot uc dot edu>, "gcc-patches at gcc dot gnu dot org Patches" <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 01 Mar 2006 18:58:41 -0800
- Subject: Re: [PATCH] Fix darwin/25908, key functions vs non weak vtables
- References: <200601261532.k0QFWGBC015147@earth.phy.uc.edu> <20F66F6F-07FC-4937-951A-DD5612F92597@apple.com> <EE2DCF39-3DB1-4AB1-913B-6807FA860283@apple.com> <b189eb63e713c5b3218274b04d912fde@physics.uc.edu> <71862B07-18E9-41B6-B0CE-95F7BC80E749@apple.com> <4BB51C48-AE6C-41C4-9861-F37F8BA205D6@apple.com> <87A89141-28CA-42A5-B91F-876489BDAAFE@apple.com>
Mike Stump wrote:
> On Feb 21, 2006, at 12:48 PM, Mike Stump wrote:
>> PR darwin/25908
>> * decl2.c (import_export_decl): Fix ABI breakage on darwin.
>
> I checked this in as obvious.
That's a stretch. It's a patch with the potential to break the ABI on
all platforms changing a very delicate part of the code. And, you
wouldn't have submitted it for review if you thought it was obvious
originally.
I planned to review the patch at some point in future, but I've been
busy with other things. I'm not sure exactly what to do now, because if
I review the patch and accept it, I've only encouraged people to declare
things obvious to avoid review. On the other hand, if it's a good
patch, it's cutting off nose to spite face to not review it.
I decided to review the patch.
+ || (DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD
(class_type))
+ && targetm.cxx.key_method_may_be_inline ())
This doesn't make sense: if the key method is inline, then of course the
key method may be inline. The only setter of CLASSTYPE_KEY_METHOD
ensures that the function is not inline at that time, so the only way
CLASSTYPE_KEY_METHOD can end up inline is if it is declared inline
outside of the class, which implies that key_method_may_be_inline.
Please retest without the second line, or explain what I'm missing.
Please don't abuse the obviousness rule in future.
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713