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

[Andy Gibbs]

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.

When we've got to the end of this set of patches, I think I will
revisit the PrintProblem functions, and propose renaming them to
PrintSeenProblem and PrintExpectedProblem so that it is clearer
which function does what.  I would then make the SourceManager
pointer in PrintExpectedProblem a reference and this will then
drop the whole "if" clause out...

Andy