Checkin Policy

[Mark Mitchell <mark at markmitchell dot com>]

I recently made some changes to the C++ front-end and ran the
regression tests.  Seeing some unexpected failures, I went to
investigate.  After a bit, I concluded that I was not responsible.  I
quickly fired off an email to Jason accusing him of high crimes, or at
least misdemeanors.  But, no dice; he was not responsible either.

Eventually, I deduced that this patch:

  Mon Nov 16 21:02:52 1998  J"orn Rennecke 

        * reload1.c (reload_reg_free_for_value_p): When considered reload
        has an output, matching inputs are not sufficient to avoid conflict.

fixes the problem.  (It came in the mail while I was investigating the
situation.)  It appears that Ulrich had found a problem in a previous
change while testing glibc; this patch was the solution.  Thanks to
J"orn for the quick fix!

I've noticed that changes to reload, especially, have more than once
resulted in seriously broken back-ends.  That's because working on
reload is notoriously hard.  And, hey, we've all broken GCC at one
time or another!

That said, I'd like to suggest, rather strongly this time, that we
require people to run the regression tests before submitting patches,
and certainly before comitting them.  In this case, the problem could
have been found simply by running the C++ tests, at least on
x86-linux-gnu.  (I have no idea whether this bug would have occurred
on other platforms or not.  I have no idea whether the tests were run
on some other platform; it's entirely possible that they were.  Thus,
I mean no criticism of the particular change in question here.)  I do
think that x86 is a particularly good index platform.  It is probably
the platform on which GCC is used the most, and several of us develop
on it.  Its many quirks make it likely to expose problems not evident
on other machines.  And I expect that many people, including everyone
at Cygnus, has access to such a machine.

Just as we require ChangeLog entries for submissions, I suggest that
we also require the submitter to run the full set of regression tests
for any back-end change, and the language-specific set for front-end
changes.  I further suggest that middle-end changes be tested on an
x86 target, except where inconvenient (no such machine is available)
or illogical (the change only affects RISC platforms, say).  I don't
think the submitter should have to *prove* that s/he ran the tests, or
even assert so explicitly, but it should still be required.  And, if
someone tends to forget to run the tests, the powers that be might
encourage that person to explicitly state that they have done so
before check-in.

Had this policy been in effect, Ulrich and I would not have had to
track down the bug, and that time could have gone to making GCC, and
the GNU C library, better.

-- 
Mark Mitchell 			mark@markmitchell.com
Mark Mitchell Consulting	http://www.markmitchell.com