RFC: Monitoring old PRs, new dg directives

Richard Sandiford richard.sandiford@arm.com
Wed Jul 29 08:40:35 GMT 2020


Thanks for doing this.  +1 for the best fix being to add XFAILing tests
to the main testsute, enabled by default.  I don't see any other realistic
way of ensuring that fixes are matched with PRs at the time that the fix
is made (rather than some time after the fact).

Marek Polacek via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> […]
> My thinking is that for:
>
> * rejects-valid: use the existing dg-xfail-if
> * accepts-valid: use the new dg-accepts-invalid
> * ICEs: use the new dg-ice
>
> dg-ice can be used like this:
>
> // { dg-ice "build_over_call" { target c++11 } }
>
> and it means that if the test still ICEs, you'll get a quiet XFAIL.  If the
> ICE is fixed, you'll get an XPASS; if the ICE is gone but there are errors,
> you'll get an XPASS + FAIL.  Then you can close the old PR.

This is long overdue IMO, thanks for adding it.

> Similarly, dg-accepts-invalid:
>
> // { dg-accepts-invalid "PR86500" }
>
> means that if the test still compiles without errors, you'll get a quiet XFAIL.
> If we start giving errors, you'll get an XPASS.
>
> If the bug is fixed, simply remove the directive.
>
> The patch implementing these new directives is appended.  Once/if this is
> accepted, I can start adding the old tests we have in our Bugzilla.  (I'm
> only concerned about the c++ component, if that wasn't already clear.)
>
> The question is what makes the bug "old": is it one year without it being
> assigned?  6 months?  3 months?  Note: I *don't* propose to add every test for
> every new PR, just the reasonably old ones that are useful/important.  Such
> additions should be done in batches, so that we don't have dozens of commits,
> each of them merely adding a single test.

IMO it should be OK to add a testcase for any open PR, if someone think
it's useful, regardless of age and without being forced to batch the
commits.  I.e. I think it should come under the “obvious” rule and
people should just use their judgement about when it's appropriate.
Adding XFAILing tests shouldn't disturb anyone else very much.

I guess there's a possibility that some tests happen to pass already
on some targets.  That's more likely with middle-end and backend bugs
rather than frontend stuff though.  Perhaps for those it would make
sense to have a convention in which the failing testcase is restricted
(at the whole-test level) to the targets that the person committing the
testcase has actually tried.  Maybe with a comment on the dg-ice etc.
to remind people to reconsider the main target selector when un-XFAILing
the test.

Richard


More information about the Gcc-patches mailing list