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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
