Re: UCNs-in-IDs patch

[Mark Mitchell <mark at codesourcery dot com>]

Neil Booth wrote:

> > I object to this abuse of CVS write access to commit patches where there 
> > is general disagreement about whether the patch should go in at all.  
> > Please revert the patch immediately.
>
>
> Hear hear.  Geoff even ignored my pointing out we don't use
> abort() in cpplib and still put one in.

This change was not submitted as a possible project for inclusion in GCC 
4.1.  While that certainly doesn't make it any way inappropriate to 
include it in GCC 4.1, it's disappointing that a major new feature, 
clearly in the works, was not submitted.

Joseph, I do think that you should consider the tradeoffs between 
insisting on an absolutely complete testsuite and on having an effective 
implementation of the feature.  Your quote from the coding standards:

   Every language or library feature, whether standard or a GNU
   extension, and every warning GCC can give, should have testcases
   thoroughly covering both its specification and its implementation.
   Every bug fixed should have a testcase to detect if the bug recurs.

should not be interpreted too literally.  Our C and C++ compilers are 
quite good, but our testsuite is nowhere near as comprehensive as some 
of the available (proprietary) testsuites.  In fact, those testsuites 
are valuable because they have allowed a few experts to specialize in 
validation while multiple parties developed compilers.  I think that if 
we'd had to develop our own testsuite, of this quality, as we went, we'd 
have a worse compiler today than we do.

I'm all for people building a very high-quality free testsuite, but I 
don't think that the level of testsuite development to which you have 
held yourself when doing development (such as your tests for various 
things in the new C parser) is an appropriate standard to impose on 
everyone else.

The above comments notwithstanding, I am not arguing that Geoff's 
testing is sufficiently complete.  I have no opinion on the technical 
merits of the patch; I've not reviewed it, nor am I an expert in UCNs.

However, I do think that the fact that you, as a C front end maintainer, 
and Zack, as a cpplib maintainer, objected to the patch certainly should 
have prevented it from going in until some kind of broader consensus was 
reached.

Thankfully, we have not often seen this kind of situation arise, so we 
have no well-established procedures.

Our development plan gives us one option.  If two people with write 
privileges think the best course of action is to revert the patch, then 
we may revert the patch after 48 hours.  Neil and Joseph have alread 
indicating they think this is best; I do, as well.  But, this policy 
only applies to patches which introduce a regression.  I am not aware of 
any regressions introduced by the patch, so until someone finds one we 
cannot start that 48-hour clock.  I expect that we never contemplated a 
commit occuring in the face of such clear objections from appropriate 
maintainers.

Geoff, I would strongly advise you to immediately revert the patch.  In 
the meantime, I will raise this issue with the SC.

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304