[cfe-commits] [Patch 2 of 4] -verify fixes (stage 2)

[Andy Gibbs]

On Wednesday, July 25, 2012 4:49 PM, Argyrios Kyrtzidis wrote:
> Correct me if I'm wrong but, from what I understand, the original issue
> that motivated Andy's patch is that the passed in DiagnosticConsumer
> (which can be a VerifyDiagnosticConsumer) is not "initialized" properly
> by calling BeginSourceFile on it before parsing starts (thus the
> VerifyDiagnosticConsumer cannot hook in the Preprocessor and receive the
> comments).
> 
> Why not just adding a BeginSourceFile in CaptureDiagnosticConsumer which
> will just call BeginSourceFile on the DiagnosticConsumer that was passed
> in to arcmt::checkForManualIssues ?
>
> That way the "contract" of the DiagnosticConsumer (call BeginSourceFile
> with the preprocessor before parsing is initiated) will be respected and
> VerifyDiagnosticConsumer should be able to get the comments without
> arcmt::checkForManualIssues needing to worry about it.

You know, that was just too blindingly obvious for me to think of!!  No,
seriously, of course this is the way to go and attached is a patch that
does just that.  Thanks for pulling me out of that dead end!

A note regarding the implementation:

It is not possible to simply forward BeginSourceFile/EndSourceFile calls
as they occur, since this will cause VerifyDiagnosticConsumer to try to
verify diagnostics without diagnostics being forwarded.  This means that
the first BeginSourceFile call is forwarded (this is all that is required
to set up the preprocessor hooks) and the matching EndSourceFile is then
called after the diagnostics list has been processed.  This is done by
an explicit call to a new method "FinishCapture" (it cannot be done
automatically in the destructor of CaptureDiagnosticConsumer since by
then the Preprocessor instance has become invalid).

This also necessitated a change to VerifyDiagnosticConsumer so that it
only processes the directives list once the final EndSourceFile call is
reached (otherwise, again, it will process them too early).

Much better now, yes?  :o)

Many thanks again,
Andy

-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch2.diff
Type: application/octet-stream
Size: 6780 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120726/c07a87cc/attachment.obj>