[cfe-dev] Patch for review: enhancements/fixes to -verify (resubmission)

[Andy Gibbs]

Hi,

Attached is a resubmission of my -verify enhancement patch.

There are two main differences between this version and the previous one at 
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120611/058799.html:

1. Following the patches for bugs in the preprocessor posted at:
     http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-June/022037.html
     http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-June/022044.html
   the needed changes to pre-existing test-cases has been brought to
   a minimum.  ***NOTE*** this patch won't apply without these first
   being applied!!!

2. I have added an extensive test-case to (hopefully) demonstrate that
   there should be no erroneous swallowing of expected-* checks, as
   was Richard Smith's concern.


Since this is a resubmission, and there has been some discussion on
what the functionality should be, I will quickly outline what the
main changes for the new -verify implementation are:

1. Diagnostic checks can now be filtered out along with the lines
   that generate the diagnostics, when they are encapsulated inside
   a #if block that is skipped.  This enables conditional checks.

2. It is also now possible to place expected-* checks on separate
   lines to that which creates the diagnostic.  A line number can
   be specified in the check, and can be either absolute (i.e. to
   the actual line in the source file) or relative to the current
   line.

3. There is additional flexibility in the "number of occurrences"
   feature.  In addition to being able to specify a fixed number
   or "one or more", it is now possible to specify "n or more"
   and "between n and m".  As mentioned in one of the earlier
   posts, this is not necessarily a feature that may find much
   use for writing clang test-cases, but is a useful feature for
   anyone wishing to use -verify to test their own code.


The following are features discussed but not implemented:

1. Richard Smith asked for a @file:line feature, but this will
   cause multiple issues in pre-existing test-cases, since if I
   were to implement this properly, the verifier should check
   that every diagnostic is from the same file that the check
   specifies (and the default would be the current file).  This
   means many test-cases will fail since currently they make
   use (misuse?!) of the fact that there isn't this check.

2. Jonathan Sauer suggested a way of specifying a way of
   overriding the conditional checks, but as Jordan Rose noted
   there are better ways of handling the corner cases that may
   need such a feature, notably FileCheck could be used.

3. There was also talk about different "modes" for -verify, but
   this has been avoided intentionally.


The following changes were necessary in pre-existing test-cases:

1. Where PCHs are involved, the expected-* checks needed to be
   moved into the actual test-case rather than the "header".

2. Where expected-* checks followed #error/#warning directives,
   the checks have been moved to a separate line.  This enables
   better checking of these directives (given the way they are
   implemented themselves currently -- see below).


The following are bugs in the current implementation of "-verify"
that are fixed in my implementation:

1. It is possible to have a test-case with the following:

     #error XYZ // expected-error{{ABC}}

   In the current implementation, this will always pass.  In the
   new implementation the expected-error check *must* go on a
   separate line, otherwise clang will say that there is a missing
   diagnostic check.  This is because in the current implementation
   of #error/#warning, the whole line is considered as the text for
   the diagnostic (comments are included!).  With the expected-*
   check on a separate line, a correct matching with the diagnostic
   is possible.

2. If a fatal error occurs in a test-case, then in the current
   implementation there will be no diagnostics from the -verify
   function either.  This is fixed in the new implementation.

3. It could happen that the current implementation would not gather
   all the expected-* checks from included header files since it
   only considered the main file and one other FileID record.  The
   new implementation considers all FileID records, marking them
   "unseen" if a diagnostic occurs with them, and "seen" if a
   expected-* check is found within them.  At the end, if there
   exists any files that are "unseen" and not "seen", then these
   are parsed again for expected-* checks.  This should limit the
   likelihood of missed expected-* checks, and did reveal an error
   in test/Modules/on-demand-build.m test-case.


I hope that has covered in summary everything important!  Please
still quiz me if there are outstanding concerns not properly
addressed...

Thanks to all who have hung on in there with me on this ;o)

Cheers
Andy



-------------- next part --------------
A non-text attachment was scrubbed...
Name: verify-enhancement.diff
Type: application/octet-stream
Size: 45887 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20120613/0ed37c38/attachment.obj>