The effects of exact diagnostic checking

Hi,

I just implemented and committed exact diagnostic checking by -verify.
This doesn't mean that the text has to match exactly, but that the count
has to match exactly. This means: one expected-diag directive, one
actual diagnostic. The old situation was that one expected-diag matched
any number of actual diags, which masked some bugs.

If there are multiple diagnostics on one line for legitimate reasons
(usually expected-note {{previous definition here}} or the like) you can
now tell the checker how many there should be:
// expected-note 3 {{previous definition here}}
expected the diagnostic to appear exactly three times.

I've updated all test cases to this new scheme were there were no bugs,
but some are left failing. These are:

== Analysis/CFDateGC.m ==
I believe this is a bug. The analyzer warns about a leak twice for a line.
If it's actually two leaks, just adjust the comment.

== Driver/env-include-paths.c ==
I don't understand this test case. I don't think it does what it's
supposed to do, but I don't even know what it is supposed to do.

== Sema/tentative-decls.c ==
One line emits a redefinition error twice, with different previous
definitions. I think this is a bug.

== Sema/block-misc.c ==
Block pointer to block pointer to non-function emits the error for every
pointer level. I think it shouldn't.

== SemaCXX/default2.cpp ==
Invalid default arguments to members functions of inner classes are
diagnosed twice.

== SemaCXX/dcl_init_aggr.cpp ==
I've adjusted this test case to not fail, but I believe the situation
could be better. When declaring an object array and no appropriate
constructor is found, you get an error for every single element.

== SemaCXX/nested-name-spec.cpp ==
Some ambiguity errors are diagnosed multiple times.

== SemaCXX/using-directive.cpp ==
Same. This is why we wanted this functionality change in the first place.

== SemaCXX/member-name-lookup.cpp ==
Probably the same.

Sebastian

:frowning: This is kinda annoying. If 100 people did this with 10 test cases apiece, at any given time, there would be about 1000 failing test cases, and the poor developers would then have to puzzle out which failures are theirs, which failures mark real bugs in their work that they now can't catch and so on. Imagine we have a cron job that reverts all work that doesn't result in a clean test suite run.

I now have 13 failing test cases, you only listed nine, and before I updated, I had 0. :frowning: That leave me wondering what happened with 4 of them.

Mike Stump wrote:

:frowning: This is kinda annoying. If 100 people did this with 10 test
cases apiece, at any given time, there would be about 1000 failing
test cases, and the poor developers would then have to puzzle out
which failures are theirs, which failures mark real bugs in their work
that they now can't catch and so on. Imagine we have a cron job that
reverts all work that doesn't result in a clean test suite run.

Then my change would be reverted :slight_smile:
The problem is that there is no good way of doing such a thing without
temporarily breaking the tree. If I just published my patch to the
mailing list and asked people to apply it and see what test failures
they have and if they can fix them, would it happen? The people have the
same work as before - puzzle out which failures are there, and which are
real bugs - plus they have to download and manually apply the patch,
instead of having SVN do it for them.

I now have 13 failing test cases, you only listed nine, and before I
updated, I had 0. :frowning: That leave me wondering what happened with 4
of them.

Me too. Can you post some more info?

I can revert the change for the time being, but that's no solution. The
bugs are there - they just weren't visible before. I've already filtered
all non-bugs I found.

Anyway, if you have test cases that fail that didn't fail for me, that
just means there are test cases which aren't portable between our
systems, which is also a real problem.

Sebastian

Hi Sebastian,

We really do appreciate this work, but I do think giving people heads up about this *before* the patch was applied would have been better. People could have then played around with the patch, see how it broke things, and then have fixes ready soon after the patch went it. Even if the only way to do this was to temporarily break the tests in the tree, it probably would be better to do it during the week instead of on the weekend (where tests could linger, broken, for several days).

Ted

Oh, another way, would be to just XFAIL or disable the testcases and let others re-enable them as they want. Simple easy do to, no testing hit, and a reasonable way to ensure the issues are addressed.

Mike Stump wrote:

Oh, another way, would be to just XFAIL or disable the testcases and
let others re-enable them as they want. Simple easy do to, no testing
hit, and a reasonable way to ensure the issues are addressed.

I can still do that. Should I?

Sebastian

Yes please, for the tests that fail.

FWIW, I don't understand what this is all about either. Daniel, should this test just be removed?

-Chris

Ted Kremenek wrote:

Mike Stump wrote:

Oh, another way, would be to just XFAIL or disable the testcases and
let others re-enable them as they want. Simple easy do to, no testing
hit, and a reasonable way to ensure the issues are addressed.

I can still do that. Should I?

Sebastian

Yes please, for the tests that fail.

Done, for those that fail on my computer. (And I fixed two.) Mike, you
said there are additional tests that fail for you. Can you tell me which
those are? I'd like to investigate them.

Sebastian

We're back at 100% nominal. :slight_smile: