Hi,Attached is an cumulative update to part 2 of the patch previously posted, updating three more tests.
Are we good to commit yet? Cheers Andy On Thursday, October 11, 2012 7:35 PM, Andy Gibbs wrote:
Hi, Thanks for the feedback. Attached then is the patch split three ways: Part 1 is an additional patch that fixes a "bug" whereby VerifyDiagnosticConsumer would parse this as a valid directive: "junkexpected-error {{...}}" which I think most people would not expect!The fix ensures that "expected-..." is the beginning of the word, or follows immediately from the "//" or "/*" of a comment. In doing the fix, a coupleof test-cases had to be adjusted too. Part 2 is all the test-case changes incorporating the new "expected-no-diagnostics" directive. Part 3 is the change to VerifyDiagnosticConsumer and the remaining test-cases. I've tried to make the patches as close to current as possible, but new commits keep bringing in new test-cases that need the new directive, so it is possible it is already behind by the time you read this! Responses to your comments are inline below. Many thanks Andy On Wednesday, October 10, 2012 7:17 PM, David Blaikie wrote:looks fairly reasonable, though I'd expect Doug or someone to sign off on a new feature(tte) like this Some nits/thoughts: * VerifyDiagnosticConsumer.cpp: braced one line else (usually we drop the braces from one line blocks) around "Status = ...HasExpectedNoDiagnostics"Done.* verify3.c: * using -D to separate multiple tests within a single file can be a bit hard to follow. Would this be possible/easier to read if they were just separate files?IMHO its better to keep what is effectively variations on the one test in one place, but if you really want it split up I can do this, of course!* Also the right-alignment of CHECK prefixes isn't common, though not inherently problematic. (just seems like it'd make for some maintenance difficulty in the future should any check prefix become longer than the longest one already in use)Again, IMHO I think it is easier to read right-aligned: but they're not allright aligned with each other, just those in the current block.* It might be nice to commit the expected-no-diagnostics fixes first, then commit the improvement just to separate the mechanical from the interesting work.Done!* verify.m: is there a reason you didn't use expected-no-diagnostics there?Yes, its to check that the changes work through the ARCMT "re-direction" layer.On Wed, Oct 10, 2012 at 1:21 AM, Andy Gibbs> wrote:** bump ** :o)I've had to resync and attach the patch again here; a test-case needed tobe updated to reflect changes in trunk. The patch now is taken against r165609. Cheers Andy On Saturday, October 06, 2012 10:59 PM, Andy Gibbs wrote:Hi, Was there any interest in this patch? Please let me know if there are any changes that should be made, or whether it is good to be committed? (or even whether I was barking up the wrong tree...?!) (If any were unable to receive the patch -- I know that sometimes Apple users have problems with my attachments! -- then it can be viewed directly at [...snip...]) Cheers Andy On Tuesday, October 02, 2012 5:32 PM, Andy Gibbs wrote:All, Attached is a patch that adds an 'expected-no-diagnostics' directive to -verify. This can then be used, as discussed, to flag files that are not expected to produce any diagnostics at all. -verify has then been changed to generate an error if no expected-* directives are found (for example, if the RUN line doesn't specify the source file!). I've also made it so that the user cannot put 'expected-no-diagnostics' in with other expected-* directives (to avoid mistakes!). The complete patch is rather large, but I've grouped the changes to VerifyDiagnosticConsumer to the top of the file along with the additional test-case. The rest of the patch makes the necessary changes to fix 580 test-cases which now (in almost all cases) need to have the 'expected-no-diagnostics' directive. Comments / questions welcome! Andy
verify-update.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
