[PATCH] Fix darwin/25908, key functions vs non weak vtables

[Mark Mitchell]

Mike Stump wrote:
> On Mar 1, 2006, at 6:58 PM, Mark Mitchell wrote:
>> +                 || (DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD
>> (class_type))
>> +                     && targetm.cxx.key_method_may_be_inline ())
>>
>> This doesn't make sense:
> 
> I can shorten that part of it to just:
> 
>    DECL_DECLARED_INLINE_P (CLASSTYPE_KEY_METHOD (class_type))
> 
> if you prefer.

Yes, please.  There's no virtue in having redundant code.  If you're
concerned that the two things get out of synch, then the right solution
is an assertion, not a redundant check.

> I didn't do that as I don't know that it safe to do, I'm
> happy to accept your assertion it is. 

This is further evidence that the patch was not obvious: you did not
fully understand the way in which the various predicates were used.
Instead of taking the time to analyze that, or ask for help, you just
committed your patch.

> What I do know, is that if it is
> unsafe, darwin _can't_ catch it, in fact, very few ports will be able to
> catch it, only some arm ports can, and then, only if just the right
> testcase is in the testsuite, which, I don't know if it is,
> historically, there just aren't any testcases in the testsuite that test
> this sort of thing.

Then write one and run it in a simulator.  Or, get a reviewer to sign
off on the change.

>> Please retest without the second line, or explain what I'm missing.
> 
> Darwin can't catch anything unsafe about that change.

But it can catch typos, build problems, etc.  Please retest, and check
in without the redundant line.

>> Please don't abuse the obviousness rule in future.
> 
> I'm sorry you consider it an abuse.  I consider it obvious, though, it
> is possible that we have different notions of obvious.  My notion of the
> obvious rule was formed long ago, pre-dating egcs by a wide margin.  I
> thought the intent of the egcs rule was to carry forward the tradition
> of the original obvious rule?

The obvious rule is intended to capture cases that are patently obvious,
i.e., changes where no reasonable reviewer would object to the code.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713