What is a regression?

What is a regression?

[Jason Merrill]

I think that the release process for recent releases has given undue 
priority to bugs marked as regressions.  I agree that it's important for 
things that worked in the previous release to keep working in the new 
release.  But the regression tag is used for much more trivial things.

For instance, Bug 32252 is an ice-on-valid bug in a new C++ feature, 
variadic templates.  But since 4.2 gave a syntax error instead of an 
ICE, this gets marked as a regression.

This seems wrong to me.  We should only use the regression tag for 
things that worked properly in the previous release and fail in the new 
release.  A change from rejects-valid to ice-on-valid is an extremely 
low priority for me, and should not affect the release schedule.  I 
would like to remove the regression tag entirely from such bugs.

Similarly, bugs marked as 4.1/4.2/4.3 regression don't seem like a high 
priority to me.  If a bug wasn't a blocker for 4.2, it shouldn't be a 
blocker for 4.3.  It makes sense to give such a bug a higher priority 
than it would normally (say, one point higher), but it seems to me that 
only regressions relative to the previous release series should actually 
be considered for release timing.

Incidentally, how are priorities assigned to bugs?  I don't see any 
guidelines on the website.

Jason

Re: What is a regression?

[David Miller]

From: Jason Merrill <jason@redhat.com>
Date: Mon, 22 Oct 2007 15:42:50 -0400

> For instance, Bug 32252 is an ice-on-valid bug in a new C++ feature,
> variadic templates.  But since 4.2 gave a syntax error instead of an
> ICE, this gets marked as a regression.

I agree that the regression marker is questionable.

But wouldn't you agree that it's not all that great to ship a new
feature in GCC that users have already found ways to ICE?

The flip side of the coin is that the user has an equal chance
as before to deal with the situation, by not using the feature.

However, the difference that I see as important here is that what
was before a lack of functionality issue is now a quality issue.
And therefore we should really fix the ICE.

Re: What is a regression?

[skaller]

On Mon, 2007-10-22 at 15:42 -0400, Jason Merrill wrote:
> I think that the release process for recent releases has given undue 
> priority to bugs marked as regressions.  I agree that it's important for 
> things that worked in the previous release to keep working in the new 
> release.  But the regression tag is used for much more trivial things.
> 
> For instance, Bug 32252 is an ice-on-valid bug in a new C++ feature, 
> variadic templates.  But since 4.2 gave a syntax error instead of an 
> ICE, this gets marked as a regression.
> 
> This seems wrong to me.  We should only use the regression tag for 
> things that worked properly in the previous release and fail in the new 
> release. 

But Jason, the compiler worked properly in rejecting invalid syntax.
Now you're suggesting it fails to do so. This suggests a real regression
and a real bug: the new feature should have an enabling flag
that couldn't have been set before it was implemented, and without
that flag should create the same error in the current version.

Not arguing against your point in general but this particular
case appears to be mishandled and the regression genuine.

BTW: did WG21 already pass this proposal?

-- 
John Skaller <skaller at users dot sf dot net>
Felix, successor to C++: http://felix.sf.net

Re: What is a regression?

[Andrew MacLeod]

Jason Merrill wrote:
>
> Similarly, bugs marked as 4.1/4.2/4.3 regression don't seem like a 
> high priority to me.  If a bug wasn't a blocker for 4.2, it shouldn't 
> be a blocker for 4.3.  It makes sense to give such a bug a higher 
> priority than it would normally (say, one point higher), but it seems 
> to me that only regressions relative to the previous release series 
> should actually be considered for release timing.
>

I think this is a very important point.  If it didn't block a previous 
release, it shouldn't block the current release. It doesn't mean it 
shouldn't get looked at, but it also shouldn't be a blocker.  I think 
the high priority regressions should be ones that are new to 4.3 because 
they have clearly been either introduced or exposed by this release and 
need to be dealt with. 

Andrew

Re: What is a regression?

[Jason Merrill]

David Miller wrote:

> But wouldn't you agree that it's not all that great to ship a new
> feature in GCC that users have already found ways to ICE?

Oh, absolutely, it's just not a regression.

Jason

Re: What is a regression?

[Jason Merrill]

skaller wrote:
> But Jason, the compiler worked properly in rejecting invalid syntax.
> Now you're suggesting it fails to do so.  This suggests a real regression
> and a real bug: the new feature should have an enabling flag
> that couldn't have been set before it was implemented, and without
> that flag should create the same error in the current version.

The current version gives an error "ISO C++ does not include variadic 
templates" unless you enable C++0x mode.  And then crashes.

But in any case, nobody has code that relies on getting an error from a 
previous version of the compiler that would be broken by moving to 4.3. 
  Only regressions on valid code seem serious enough to me to warrant 
blocking a release.

> BTW: did WG21 already pass this proposal?

Yes, a few meetings back.

Jason

Re: What is a regression?

[Andrew Pinski]

On 10/22/07, Jason Merrill <jason@redhat.com> wrote:
> David Miller wrote:
>
> > But wouldn't you agree that it's not all that great to ship a new
> > feature in GCC that users have already found ways to ICE?
>
> Oh, absolutely, it's just not a regression.

Except it is still ICEing.  There was a discussion before (I cannot
find it right now) that made the following considered a regression:
anything to ICE (even after an error, though not a blocking one)
works to wrong code
rejects invalid to accepts valid
accepts invalid to rejects valid
diagnostic regressions

And there was a previous discussion about setting the priority too
(the release manager is the only one who should be setting it too).

Thanks,
Andrew Pinski

Re: What is a regression?

[Andrew Pinski]

On 10/22/07, Andrew MacLeod <amacleod@redhat.com> wrote:
> I think this is a very important point.  If it didn't block a previous
> release, it shouldn't block the current release.

Yes it is but does a regression that is just found to be a regression
is considered a blocking one, it should block the current release.
Likewise for all new incomming regressions.  Just because we did not
know about a regression before, should not cause it not to be high
priority.

Thanks,
Andrew Pinski

Re: What is a regression?

[skaller]

On Tue, 2007-10-23 at 00:00 -0400, Jason Merrill wrote:
> skaller wrote:
> > But Jason, the compiler worked properly in rejecting invalid syntax.
> > Now you're suggesting it fails to do so.  This suggests a real regression
> > and a real bug: the new feature should have an enabling flag
> > that couldn't have been set before it was implemented, and without
> > that flag should create the same error in the current version.
> 
> The current version gives an error "ISO C++ does not include variadic 
> templates" unless you enable C++0x mode.  And then crashes.

I see. So this is unpleasant, but technically it has met the
ISO conformance requirements in issuing the diagnostic.
A quick fix would be to abort() the compiler to prevent the ICE
in released versions (but developers would test with the abort
commented out whilst trying to get the implementation working).

> But in any case, nobody has code that relies on getting an error from a 
> previous version of the compiler that would be broken by moving to 4.3. 

Well it seems you're right given that 4.3 DOES issue a diagnostic,
so I guess I'd agree with you now given that data.

>   Only regressions on valid code seem serious enough to me to warrant 
> blocking a release.

I still think that is too strong a position. A good fraction
of compiler time is spent bugging out user code.. one could
even say the job of a compiler is not generating machine code,
but telling programmers they're idiots :)

Still, if you were to use "ISO C++ conformance model" as the
criteria, you could probably weaken your position and still
reduce the number of blocking "regressions": an ICE in the
case of experimental code could be allowed PROVIDED there
is a diagnostic WHEN one is actually required.

In fact, that may be TOO weak: people do rely on QOI diagnostics
beyond those required by ISO.

> > BTW: did WG21 already pass this proposal?
> 
> Yes, a few meetings back.

Not my preferred model but better than nothing.

-- 
John Skaller <skaller at users dot sf dot net>
Felix, successor to C++: http://felix.sf.net

Re: What is a regression?

[Ian Lance Taylor]

Jason Merrill <jason@redhat.com> writes:

> I think that the release process for recent releases has given undue
> priority to bugs marked as regressions.  I agree that it's important
> for things that worked in the previous release to keep working in the
> new release.  But the regression tag is used for much more trivial
> things.

We had a discussion of these sorts of issues at the GCC mini-summit
back in April.  We didn't come to any conclusions.  But there are some
notes here:

http://gcc.gnu.org/ml/gcc/2007-04/msg00676.html


The goal is presumably: how can we produce the highest quality
release?  I agree that our single-minded focus on regressions is
misleading.  For example, it might be better to produce a more nuanced
list of bugs which must be fixed, which should be fixed, and which we
would like to fix.  Then we can set numeric targets for each level, to
be acheived before the release.  However, this would require a lot
more effort.  It would be a lot of work for one person--and who would
that person be?  If many people did it, then how would we keep
consistency?

Ian

Re: What is a regression?

[Paolo Bonzini]

> I think this is a very important point.  If it didn't block a previous 
> release, it shouldn't block the current release. It doesn't mean it 
> shouldn't get looked at, but it also shouldn't be a blocker.  I think 
> the high priority regressions should be ones that are new to 4.3 because 
> they have clearly been either introduced or exposed by this release and 
> need to be dealt with.

It might happen that a bug is triggered more easily in 4.3 than it is in 
4.1 or 4.2, but an artificial test case can be constructed that fails on 
all three releases.  See for example PR32004, where comment#16 has a 
testcase failing on 4.1/4.2 too.

Paolo

Re: What is a regression?

[Paolo Bonzini]

> I think this is a very important point.  If it didn't block a previous 
> release, it shouldn't block the current release. It doesn't mean it 
> shouldn't get looked at, but it also shouldn't be a blocker.  I think 
> the high priority regressions should be ones that are new to 4.3 because 
> they have clearly been either introduced or exposed by this release and 
> need to be dealt with.

It might happen that a bug is triggered more easily in 4.3 than it is in 
4.1 or 4.2, but an artificial test case can be constructed that fails on 
all three releases.  See for example PR32004, where comment#16 has a 
testcase failing on 4.1/4.2 too.

Paolo

Re: What is a regression?

[David Fang]

> I still think that is too strong a position. A good fraction
> of compiler time is spent bugging out user code.. one could
> even say the job of a compiler is not generating machine code,
> but telling programmers they're idiots :)

Every compiler version I've tried has been telling me this for years. 
When can we expect some *positive* feedback from compilers? 
"Congratulations, your code is less of a spaghetti-mess than it was last 
revision, keep up the good work."  I smell a request for enhancement...

Fang

Re: What is a regression?

[skaller]

On Tue, 2007-10-23 at 03:05 -0400, David Fang wrote:
> > I still think that is too strong a position. A good fraction
> > of compiler time is spent bugging out user code.. one could
> > even say the job of a compiler is not generating machine code,
> > but telling programmers they're idiots :)
> 
> Every compiler version I've tried has been telling me this for years. 
> When can we expect some *positive* feedback from compilers? 
> "Congratulations, your code is less of a spaghetti-mess than it was last 
> revision, keep up the good work."  I smell a request for enhancement...

So you and your compiler are off to a co-dependency workshop? <g>

-- 
John Skaller <skaller at users dot sf dot net>
Felix, successor to C++: http://felix.sf.net

Re: What is a regression?

[Gabriel Dos Reis]

Jason Merrill <jason@redhat.com> writes:

| But in any case, nobody has code that relies on getting an error from
| a previous version of the compiler that would be broken by moving to
| 4.3. Only regressions on valid code seem serious enough to me to
| warrant blocking a release.

I strongly agree.

-- Gaby

Re: What is a regression?

[Mark Mitchell]

Ian Lance Taylor wrote:
> Jason Merrill <jason@redhat.com> writes:
> 
>> I think that the release process for recent releases has given undue
>> priority to bugs marked as regressions.  I agree that it's important
>> for things that worked in the previous release to keep working in the
>> new release.  But the regression tag is used for much more trivial
>> things.
> 
> We had a discussion of these sorts of issues at the GCC mini-summit
> back in April.  We didn't come to any conclusions.

There's a misconception in this thread that I need to address as the RM:
the notion of "release-blocker".  We don't really have release blockers,
because we don't have a way of making sure things get fixed.  If I
declare a bug a release blocker, we may just never have another release.
 Companies can declare something a release-blocker because they can then
direct resources to go work on the issue.  We don't have that luxury.
When I look at the bug list immediately before making a release, I scan
the P1s and decide if I can live with myself after I push the button.

When I mark a PR as "P1", that means "This is a regression, and I think
it's embarrassing for us, as a community, to have this bug in a
release."  Unfortunately, every release goes out with P1 bugs open, so
we can't really call them "release blockers".  My judgment isn't always
great, and it's certainly not final: I'm willing for people to suggest
that P1s be downgraded.  I've suggested that people do that by putting a
note in the PR, and CC'ing me.

I agree that PR32252 (the example given by Jason) should not be P1.
I've downgraded it to P2.  (P2 means "regression users will notice on a
major platform, but not P1".  P3 means "not yet looked at".  Things like
ICE-after-valid-error are marked P4.  Things utterly outside the release
criteria are P5.)

One of the consistent problems with GCC has been that we disregard the
experience of users that aren't power users.  However, I disagree that
these things should be extremely low priority.  Compiler ICEs send
message that what we're producing is not of high quality so I think we
should take them seriously.

I do also think we should start weeding out some of the P2s that have
been around a long time.  If GCC 2.95 didn't ICE, but GCC 3.0+ have all
ICE'd, then, at this point, it's not much of a regression any more; it's
just an ICE that's "always" been there.

I do think that we should take regressions, in general, very seriously
-- especially cases where we generate wrong code.  One of the consistent
complaints I have heard is that people fear GCC upgrades and the
perception is that this is worse than with other compilers.  Certainly,
I've talked to other compiler vendors who claim that they consider any
wrong-code generation bug a release blocker.

Now, all that said, of course I think that other bugs are important too,
and I'm all for fixing them.  But, in terms of looking at a release and
deciding how ready-to-go it is, I think regressions are as reasonable a
measure as any.

Thanks,

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

Re: What is a regression?

[Jason Merrill]

Mark Mitchell wrote:
> When I mark a PR as "P1", that means "This is a regression, and I think
> it's embarrassing for us, as a community, to have this bug in a
> release."  Unfortunately, every release goes out with P1 bugs open, so
> we can't really call them "release blockers".  My judgment isn't always
> great, and it's certainly not final: I'm willing for people to suggest
> that P1s be downgraded.  I've suggested that people do that by putting a
> note in the PR, and CC'ing me.

OK, thanks for clarifying that.  Are the P1s the only thing you consider 
  when deciding whether or not to make a release?

> I agree that PR32252 (the example given by Jason) should not be P1.
> I've downgraded it to P2.  (P2 means "regression users will notice on a
> major platform, but not P1".  P3 means "not yet looked at".  Things like
> ICE-after-valid-error are marked P4.  Things utterly outside the release
> criteria are P5.)

How do non-regressions fit into this scheme?

> One of the consistent problems with GCC has been that we disregard the
> experience of users that aren't power users.  However, I disagree that
> these things should be extremely low priority.  Compiler ICEs send
> message that what we're producing is not of high quality so I think we
> should take them seriously.

Absolutely.  But not as seriously as wrong-code or 
ice-on-valid/rejects-valid bugs, IMO.

> I do also think we should start weeding out some of the P2s that have
> been around a long time.  If GCC 2.95 didn't ICE, but GCC 3.0+ have all
> ICE'd, then, at this point, it's not much of a regression any more; it's
> just an ICE that's "always" been there.

Yes.

> I do think that we should take regressions, in general, very seriously
> -- especially cases where we generate wrong code.  One of the consistent
> complaints I have heard is that people fear GCC upgrades and the
> perception is that this is worse than with other compilers.  Certainly,
> I've talked to other compiler vendors who claim that they consider any
> wrong-code generation bug a release blocker.

I absolutely agree for regressions on valid code.  But regressions on 
invalid code, or on valid code that we've never accepted, shouldn't 
affect people upgrading.  That's the distinction I want to make.

> Now, all that said, of course I think that other bugs are important too,
> and I'm all for fixing them.  But, in terms of looking at a release and
> deciding how ready-to-go it is, I think regressions are as reasonable a
> measure as any.

I think the term "critical regressions" you've used in past status 
updates (wrong-code, ice-on-valid-code, rejects-valid regressions) are a 
good measure, but not other regressions; non-critical regressions should 
not have higher priority than serious non-regression bugs.  We have had 
a bunch of C++ wrong-code bugs hanging around for a long time, and I 
don't think ice-on-invalid-code regressions should take priority over those.

I was thinking of a default priority scheme like

new/uncategorized bugs are P0.
critical regressions from the previous release series are P1.
other critical regressions and wrong-code bugs are P2.
non-critical regressions and ice-on-valid-code/rejects-valid bugs are P3.
most other bugs are P4.
enhancements are P5.

adjusting priorities up or down based on how often the bug is 
encountered, or how much work it is to fix.

Jason

Re: What is a regression?

[Joe Buck]

On Tue, Oct 23, 2007 at 02:20:24PM -0400, Jason Merrill wrote:
> Mark Mitchell wrote:
> >When I mark a PR as "P1", that means "This is a regression, and I think
> >it's embarrassing for us, as a community, to have this bug in a
> >release."  Unfortunately, every release goes out with P1 bugs open, so
> >we can't really call them "release blockers".  My judgment isn't always
> >great, and it's certainly not final: I'm willing for people to suggest
> >that P1s be downgraded.  I've suggested that people do that by putting a
> >note in the PR, and CC'ing me.
> 
> OK, thanks for clarifying that.  Are the P1s the only thing you consider 
>  when deciding whether or not to make a release?
> 
> >I agree that PR32252 (the example given by Jason) should not be P1.
> >I've downgraded it to P2.  (P2 means "regression users will notice on a
> >major platform, but not P1".  P3 means "not yet looked at".  Things like
> >ICE-after-valid-error are marked P4.  Things utterly outside the release
> >criteria are P5.)
> 
> How do non-regressions fit into this scheme?

I would like to see more attention paid to wrong-code bugs that aren't
marked as "regression".  Doesn't mean they should necessarily be P1, but
we tend to ignore them.

Re: What is a regression?

[Mark Mitchell]

Jason Merrill wrote:
> Mark Mitchell wrote:
>> When I mark a PR as "P1", that means "This is a regression, and I think
>> it's embarrassing for us, as a community, to have this bug in a
>> release."  Unfortunately, every release goes out with P1 bugs open, so
>> we can't really call them "release blockers".

> OK, thanks for clarifying that.  Are the P1s the only thing you consider
>  when deciding whether or not to make a release?

Roughly speaking, yes.  Of course, if someone were to raise an issue
with me in some other way, then I would consider that too.  But,
generally, I look at the open P1s to determine whether or not quality is
at an acceptable level.

>> I agree that PR32252 (the example given by Jason) should not be P1.
>> I've downgraded it to P2.  (P2 means "regression users will notice on a
>> major platform, but not P1".  P3 means "not yet looked at".  Things like
>> ICE-after-valid-error are marked P4.  Things utterly outside the release
>> criteria are P5.)
> 
> How do non-regressions fit into this scheme?

They don't.  Historically (well, as long as I can remember), anything
that was a non-regression was automatically P5.

That's not to say that there are no important non-regression bugs.  But,
my feeling has been that if the bug was not a regression, then we've had
a demonstrably useful compiler up until now, so it wasn't something that
I needed to worry about for the next release.

>> these things should be extremely low priority.  Compiler ICEs send
>> message that what we're producing is not of high quality so I think we
>> should take them seriously.
> 
> Absolutely.  But not as seriously as wrong-code or
> ice-on-valid/rejects-valid bugs, IMO.

Totally agreed.

In theory, the "cost" of a bug is some product-like function of its
prevalence (i.e., the number of people who run into it) and its severity
(i.e., the impact it has on those that encounter it).  I do try to make
that judgment in some cases: if I see a bug that looks unlikely to
affect anyone in the real world, then I might not mark it P1, even
though it's a wrong-code regression.  But, prevalence is hard to
measure, so, in general, I mark wrong-code regressions as P1.

> I absolutely agree for regressions on valid code.  But regressions on
> invalid code, or on valid code that we've never accepted, shouldn't
> affect people upgrading.  That's the distinction I want to make.

That's a fair point and that's why I downgraded the issue you mentioned
from P1 to P2.

> I think the term "critical regressions" you've used in past status
> updates (wrong-code, ice-on-valid-code, rejects-valid regressions) are a
> good measure, but not other regressions; non-critical regressions should
> not have higher priority than serious non-regression bugs.  We have had
> a bunch of C++ wrong-code bugs hanging around for a long time, and I
> don't think ice-on-invalid-code regressions should take priority over
> those.

I think we need to agree on what the priority scheme is for.  In a
commercial setting, we might have bug-killing droids who came in each
morning, picked the highest priority bug off the list, and zapped it.
But, we don't -- we have people who pick whatever is of interest to
them.  So, I've used the priority scheme as a way to tell me how well
we're doing relative to the previous release.

In other words, I'm not trying to suggest that if you want to maximize
goodness of the compiler you pick the highest priority bug and fix it.
Then again, if you fix a P1, you're probably doing a good thing.  When
we run out of P1s, come see me. :-)  We can fight over whether to work
on the P2s, or something else.

Thanks,

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

Re: What is a regression?

[Gerald Pfeifer]

On Tue, 23 Oct 2007, Mark Mitchell wrote:
> [...]

I realized that the documentation we currently have up at
  http://gcc.gnu.org/bugs/management.html
was partly incorrect (only listing P1 to P2) and certainly
quite incomplete, so I tried to cast my understanding and
what I found in this thread into a documentation update.

Thoughts on the patch below (which I have not committed yet)?

Gerald

Index: bugs/management.html
===================================================================
RCS file: /cvs/gcc/wwwdocs/htdocs/bugs/management.html,v
retrieving revision 1.25
diff -u -3 -p -r1.25 management.html
--- bugs/management.html	14 Jan 2007 11:38:36 -0000	1.25
+++ bugs/management.html	1 Jan 2008 21:46:11 -0000
@@ -168,13 +168,37 @@ problem where an easy workaround exists 
 
 <h3 align="center" id="priority">Priority</h3>
 This field describes the importance and order in which a bug should be
-fixed.  It is utilized by the programmers/engineers to
-prioritize their work to be done.  The available priorities are:
+fixed.  The available priorities are:
 
 <table cellspacing="3">
-<tr><th>P1</th><td>Most important</td></tr>
-<tr><th>P2</th><td></td></tr>
-<tr><th>P3</th><td>Least important</td></tr>
+<tr>
+  <th valign="top">P1</th>
+  <td>Most important.  This generally labels a regression which the
+    release manager feels should be addressed for the next release
+    including wrong-code regressions.<br />
+    For practical reasons, releases tend to go out with P1 bugs open,
+    but we try to minimize those.  If you want to downgrade a P1 bug,
+    CC the release manager on the PR and add a note.
+  </td>
+</tr><tr>
+  <th valign="top">P2</th>
+  <td>This generally indicates a regression users will notice on a
+    major platform, which is not P1 though.
+  </td>
+</tr><tr>
+  <th valign="top">P3</th>
+  <td>The default priority for new PRs which have not been prioritized
+    yet.</td>
+</tr><tr>
+  <th valign="top">P4</th>
+  <td>Usually not a regression, but still of some priority.
+    ICE-after-valid-error bugs fall in this category, for example, as
+    do regressions that have persisted for several major releases.
+  </td>
+</tr><tr>
+  <th valign="top">P5</th>
+  <td>Least important.  Not on the radar of release management.</td>
+</tr>
 </table>
 
 </td></tr>

Re: What is a regression?

[Mark Mitchell]

Gerald Pfeifer wrote:
> On Tue, 23 Oct 2007, Mark Mitchell wrote:
>> [...]
> 
> I realized that the documentation we currently have up at
>   http://gcc.gnu.org/bugs/management.html
> was partly incorrect (only listing P1 to P2) and certainly
> quite incomplete, so I tried to cast my understanding and
> what I found in this thread into a documentation update.
> 
> Thoughts on the patch below (which I have not committed yet)?

I think that's an appropriate patch, for now.  I've had some offline
discussions about other changes we might want to make, but let's capture
the current state.

Thanks,

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

Re: What is a regression?

[Gerald Pfeifer]

On Wed, 2 Jan 2008, Mark Mitchell wrote:
> I think that's an appropriate patch, for now.  I've had some offline 
> discussions about other changes we might want to make, but let's capture 
> the current state.

Exactly.  That was my thinking. :-)  If we want to make any updates to
our processes, I'll be happy to help update the docs accordingly then.

Gerald