[cfe-commits] [Patch 2 of 7] -verify fixes and enhancement
andyg1001 at hotmail.co.uk
Wed Jul 4 08:41:29 PDT 2012
On Wednesday, July 04, 2012 2:35 AM, Jordan Rose wrote:
> 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...
More information about the cfe-commits