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

[Jordan Rose]

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


On Jun 11, 2012, at 8:58 AM, Andy Gibbs wrote:

> Hi all,
> 
> The -verify option has the potential for uses outside just the clang 
> test-suite, for example, within a test-suite for a 3rd-party library. 
> However, it has a number of idiosyncrasies or missing features which the 
> attached patch seeks to address.  While none of the alterations are perhaps 
> critical to the development of clang itself (that remains to be seen!), they 
> do make the -verify feature immeasurably more flexible and useful.
> 
> In the first instance, the patch enables -verify to distinguish between 
> blocks of code which have been #if/#ifdef-ed out and those which have not. 
> The current implementation will not do this, so for example, this trivial 
> fragment will cause an "expected but not seen" failure:
> 
> #if 0
> #error some error // expected-error {{some error}}
> #endif
> 
> Secondly, in the current implementation, all diagnostics must be contained 
> on the actual line which generates the diagnostic.  The new implementation 
> allows a line number to be optionally specified as part of the diagnostic 
> declaration using the @ symbol, for example:
> 
> // expected-error@<line> {{some error}}
> 
> where <line> can be some absolute line number, for example: 10, or a 
> relative line number, for example: -1 or +5, from the current line.  Note 
> that where an absolute line number is given this is intentionally the actual 
> source line in the file in question, not one modified by the #line 
> directive.  Likewise, relative line numbers are also not affected by the 
> #line directive.
> 
> Thirdly, in the current implementation, it is possible to specify an exact 
> number of times a diagnostic will appear, or that it will appear "1 or more" 
> times.  This new implementation extends this to enable the test case to 
> specify a more precise range.  Therefore, the following are now possible:
> 
> (Those marked * are already possible in the current implementation)
> 
> // expected-error 2 {{some error}} -- must appear twice*
> // expected-error + {{some error}} -- must appear once at least*
> // expected-error 1+ {{some error}} -- same as the line above
> // expected-error 0+ {{some error}} -- may or may not appear
> // expected-error 2-5 {{some error}} -- must appear at least 2 but not more 
> than 5 times.
> 
> The patch also includes the relevant modifications necessary to the existing 
> clang test-suite.  These modifications were necessary for two reasons: since 
> #if-block processing is now included, various tests in the PCH folder were 
> now missing their relevant diagnostics.  Also #error/#warning directives 
> include all text up to the end-of-line in their message so tests containing 
> these have the diagnostics shifted onto a separate line now.
> 
> Coincidentally, this is a good thing in any case since (as was highlighted 
> in Preprocessor/line-directive.c) the following line will always 
> (erroneously!) pass in the current implementation:
> 
> #error AAA // expected-error {{BBB}}
> 
> A couple of tests have also been added to test the new features of -verify.
> 
> The attached patch is taken against the current trunk.  I hope it meets the 
> quality criteria to be considered for committal.  Please don't hesitate to 
> quiz me on any aspect of it!
> 
> Kind regards
> 
> Andy
> 
> 
> 
> <verify-enhancement.diff>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits