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

[Andy Gibbs]

On Wednesday, July 04, 2012 5:41 PM, Andy Gibbs wrote:
> On Wednesday, July 04, 2012 2:35 AM, Jordan Rose wrote:
>> [...snip...]
>> Oops, I see that's already been taken care of. Wait, so why do
>> we need the additional check at the end? Referring to this part
>> of the patch:
>> 
>>> -    if (D.Location.isInvalid() || !SourceMgr)
>>> +    if (D.DiagnosticLoc.isInvalid() || !SourceMgr)
>> 
>> I see that it's not /your/ isInvalid() originally, but I don't
>> think anything should break if you take it away. ...probably.
>
> Well it passed the simple check which was to stick an assert in
> and see if anything broke!  I also cannot conceive of a reason
> why DiagnosticLoc could be invalid at this point.
>
> However, I would be tempted to leave it in since it was in before.
> I can only think that the originator had some reason for making
> the check rather than asserting, meaning that he may have foreseen
> a situation where DiagnosticLoc may be invalid.  I will check
> through the commit logs to see if anything comes up.

I have checked through the commits and I believe that the isInvalid
check comes from r102516 when the PrintProblem function was
duplicated.  I believe it is safe therefore to remove the check in
this duplicated version of PrintProblem since it is only used in
the situation where DiagnosticLoc must be valid.

Attached then is a patch which tidies up the PrintProblem functions.
It renames them PrintUnexpected and PrintExpected according to their
application.  The SourceManager pointer in PrintExpected has been
changed to a reference and the "if" clause has subsequently dropped
out.

A similar change can not be made to PrintUnexpected since it may be
used in a situation either where there is not a SourceManager or
where the diagnostic location is invalid.

This patch is in addition to patches 1, 2 and 3 posted recently.
It should be possible to apply it directly after patch 2, but I
have only tested it applied after patch 3.

Cheers,
Andy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: verify-part2a.diff
Type: application/octet-stream
Size: 6232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120705/32b7e674/attachment.obj>