[cfe-commits] Patch for review: enhancements/fixes to -verify

Hi, Andy. CCing to cfe-dev since this is a change worth discussing.

Personally, I think this is great. Being able to make expected-* be preprocessor-dependent means it's a lot easier to write conditional tests, which previously you'd need to use different FileCheck invocations to handle.

I'm less excited about the non-deterministic number of warnings, since I don't think we should encourage tests that don't know exactly how many warnings will be emitted. '+' is kind of a special case for notes that we trigger more than once but don't really care about. But since this is fully backwards-compatible, this isn't a strong objection.

Good catch on the comments-in-the-line problem. I just hit that with a FileCheck test recently.

I didn't see anything obviously out-of-place here, but the -verify code isn't my area of expertise, so please wait for someone else's review.

Thanks for coming up with this!
Jordan

Personally, I think this is great. Being able to make expected-* be
preprocessor-dependent means it's a lot easier to write conditional
tests, which previously you'd need to use different FileCheck
invocations to handle.

Thanks! Yes, conditional tests was the main impetus behind making the
patch.

I'm less excited about the non-deterministic number of warnings,
since I don't think we should encourage tests that don't know exactly
how many warnings will be emitted. '+' is kind of a special case for
notes that we trigger more than once but don't really care about. But
since this is fully backwards-compatible, this isn't a strong objection.

This is one of those things that fell out while thinking about use-cases.
Making the feature possible doesn't need to mean that those writing test
cases should be advised to make use of them: this comes down to having some
sort of test-case-writing guidance for those developing on clang. Where I
started from, was looking for a way of swallowing up uninteresting
diagnostics in a set of test-cases for a separate project, while still
catching the important ones. In the particular case in question, the exact
number of the uninteresting diagnostics could not be easily determined since
the test-cases depended on include file containing quite a number of
platform-specific #if blocks. I had seen the "+" token for "1 or more" but
I was looking for a "0 or more". I considered implementing a "*" token
(again the regex-style grammar), but then thought that a clearer
implementation would be the 0+ route and the full set, including a min/max
range, followed from that for completeness' sake.

Good catch on the comments-in-the-line problem. I just hit that with
a FileCheck test recently.

Yes, easy to miss! The problem here of course is that #error takes the
whole line for its output, but since there was a separate parse operation on
the source file for checking the diagnostics, the comment was getting
extracted even though in the normal compile operation, the comment is not a
comment...

I didn't see anything obviously out-of-place here, but the -verify code
isn't my area of expertise, so please wait for someone else's review.

Thanks for coming up with this!
Jordan

No problem.

Cheers,
Andy