[cfe-commits] [Patch 7 of 7] -verify fixes and enhancement

[Jordan Rose]

On Jul 3, 2012, at 3:23 PM, Andy Gibbs wrote:

> On Tuesday, July 03, 2012 5:18 PM, Jordan Rose wrote:
> 
>> As I said before (without context) I don't like the idea of reparsing "unseen"
>> files at the end; I think our implementation of CommentHandler and
>> VerifyDiagnosticsHandler should just be good enough to handle these cases,
>> and/or that -verify doesn't support expectations in header files.
> 
> I totally agree (as I have mentioned elsewhere) and this is the reasoning behind
> the contentious part 6 patch -- it removes one source of "unseen" files. There
> are two others, of which I have pinned one down but I don't have a patch I am
> pleased with yet.  As I said earlier, I have to push this work out to later in
> July/August since I am about to become a father for the second time and this
> will call me into other duties for a short while.

Congratulations! :-)

And I suppose it's no worse than what we're doing now (i.e., completely ignoring the preprocessor and just groveling through every file we see for things that look like verify directives).


>> In general, though, I think this is a very useful cleanup/enhancement to
>> -verify, and although I think these policy issues should be resolved (and
>> one more person should probably review the patches) I'm hoping they can go
>> in soon.
> 
> Thank you: high praise makes it all worth while.  I, for my part, am very
> grateful that you (and Richard) have taken the time to diligently critique
> my patches.
> 
> I have just posted the first three patches with the latest round of comments
> implemented, and I trust are now commonly held to be acceptable.  Would it be
> possible to get these actually committed now?  I guess you'd like all the
> patches to be ready together, but we're still discussing patch 4 and 6, in
> particular, and I would like to be able to get the first three patches out of
> the way so that I don't have to keep rebasing them...  The changes they make
> are complete in themselves and cause no test regressions.  I hope it isn't too
> unreasonable a request?

I'm happy to commit 1-3 once my last few questions are resolved. :-) I'll wait on a few more comments (till Thursday at least, since tomorrow's a US holiday).


> Regarding the remaining patches, this is my suggestion:
> 
> Patch 4: I have an alternative proposal for this which approaches the problem
> from another angle.  I think it still may raise the same concerns, but I will
> try to post a patch to it shortly to see what you think.  If you think not,
> then we can leave this patch out and I will adjust the subsequent patches to
> fit.  I will have to retain this patch in the separate distribution I maintain
> since I have users requesting this fix.
> 
> Patch 5: This will get adjusted as regards the position with patch 4.
> 
> Patch 6: I am happy to leave this out altogether at present.  As mentioned
> above, ultimately I think we should put some solution in as this will move
> towards removing the "unseen" files problem.  I have in mind what may be a
> better solution... watch this space!
> 
> Patch 7: This again will need to be adjusted to fit the position regarding
> patches 4 and 6 since there is a test-case for each of these patches
> contained within.

Sounds good to me!
Jordan