Hi Andy, On Wed, Jun 13, 2012 at 8:17 AM, Andy Gibbs <[email protected]> wrote: > 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.
Thanks, the test cases look great. > 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. OK, I think you've made a good case for all these features. However, you are much more likely to get them reviewed if you can provide separate patches for each feature. > 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". OK. > 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. Nice catch. > 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 didn't follow this. It seems to me that we should either (a) only consider expected-* comments we see when preprocessing the main source file, or (b) consider all expected-* comments we see in any file (and the latter makes more sense to me). When does the need for re-parsing arise? Thanks! Richard _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
