[PATCH] D12379: Fix the bugs in the mapDiagnosticRanges (Still in progress)

[Richard Trieu via cfe-commits]

rtrieu added inline comments.

================
Comment at: lib/Frontend/DiagnosticRenderer.cpp:442
@@ -419,1 +441,3 @@
 
+static bool checkLocForMacroArgExpansion(SourceLocation Loc,
+                                         const SourceManager &SM,
----------------
zhengkai wrote:
> I have stated that I need to check if all the locations checked fit in the same location in the expansionInfo. So I need the preLoc to store the previous checked location to see if they are the same.
That explains nothing.  You have a function 'checkLocForMacroArgExpansion' that has three parameters, SourceLocation Loc, SourceManager SM, and SourceLocation PreLoc.  From the name of the function, it appears that it checks Loc to see if is a macro argument expansion.  The SourceManger is needed for checking information about SourceLocation's.  That leaves PreLoc, which does not have a descriptive name ('pre' could be short for lots of different words) nor is it commented.

You mentioned it is the previously checked location.  Does this mean PreLoc is one location behind Loc?  If so, why not just use the offset to get one behind?  Or does it refer to different locations on the macro stack?

Since both 'checkLocForMacroArgExpansion' and 'checkRangeForMacroArgExpansion' now have a third parameter that is not obvious from the function name what they do, add comments to the functions to explain the purpose of the third parameter.


http://reviews.llvm.org/D12379