Re: patch rereview requested for PRs 6860, 10467 and 11741

[Roger Sayle <roger at eyesopen dot com>]

On 20 Sep 2003, Geoff Keating wrote:
> I think it's unreasonable to ask a reviewer to prove your patch is
> incorrect.  Instead, you should explain why you think it's correct.

I agree whole heartedly.  However I also believe is unreasonable for
a reviewer to reject a patch without giving an adequate explanation.
Alternatively, a reviewer is also free to suggest an alternative
solution or approach to tackling the problem.  Reviewer comments of
the form "I haven't bothered" are less than ideal.


> Arguments like "before, the compiler would have ICEd" are not valid.
> You might be turning an ICE into silent bad code generation.

Indeed, the full motivation for why the patch is believed to be
correct have been presented several times.  The "before, we'd ICE"
was a counter argument against the silly claims the patch would
introduce new regressions.

Whilst I agree that we could potentially be turning an ICE into
silent bad code generation or just moving the ICE elsewhere in the
compiler, for the known examples in the PR, we now do the correct
thing.

I'm sure that we both agree that a patch that is an improvement,
fixes several known high priority PRs and causes no testsuite
regressions, should not be held up due to the fear of a potential
latent bug elsewhere.  Any latent bugs, if the exist, deserve their
own PR.  The rules are that latent bugs exposed by a patch, are the
responsibility of the original patch submitter.

Patches must be assumed innocent until proven guilty!


[Geoff, you too are guilty of exactly the same paranoia with David
Anglin's solution to PR middle-end/11414, where again a patch that
fixes a critical regression, in that case a bootstrap failure, is
entangled in the review processes because the reviewer suspects it
might break something, but without suggestions for a correct fix
or justification for the reviewer's beliefs:
http://gcc.gnu.org/ml/gcc-patches/2003-07/msg00247.html]


If reviewers aren't prepared to accept patches with some risk, that
have shown due diligence, the quality of patches that make it into
GCC will be limited to the quality of the reviewers, irrespective
of the ability of the patch submitters.


Not that I'm complaining, I was just politely asking RTH to consider
rereview the patch.

Roger
--