[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:455
@@ -419,1 +454,3 @@
 
+static bool checkLocForMacroArgExpansion(SourceLocation Loc,
+                                         const SourceManager &SM,
----------------
rtrieu wrote:
> zhengkai wrote:
> > Because the function isMacroArgExpansion doesn't check if all locations are in the same argument location,
> I still don't know what you are doing here.  You have two SourceLocation's, Loc and PreLoc.  Those are not good names for telling them apart or what they do.  You pass the second SourceLocation in by reference, both as an input value, and later as something to put an output value in.  This is a bad idea.  If you need a SourceLocation to be passed back to the caller, make the function return a SourceLocation, or use a SourceLocation pointer as an argument that is only for output.  The SourceLocation class is tiny so it is okay to copy around.
Still not answered: What is PreLoc?  Why is it important to this function?  What does PreLoc even mean?

================
Comment at: lib/Frontend/DiagnosticRenderer.cpp:471
@@ -422,1 +470,3 @@
+                                           const SourceManager &SM,
+                                           SourceLocation &Loc) {
   SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
----------------
zhengkai wrote:
> rtrieu wrote:
> > What is this SourceLocation?  Why is it passed by reference?
> Changed
What is Loc?  Where is it pointing to?  Why do you need it?

================
Comment at: test/Misc/caret-diags-macros.c:210
@@ -215,3 +209,3 @@
 void f(char* pMsgBuf, char* pKeepBuf) {
-Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ", Cstrlen(pKeepBuf));
+Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ", strlen_test(pKeepBuf));
 }
----------------
rtrieu wrote:
> Add a FIXME comment that it should be changed back to Cstrlen when the bug is resolved.
Use:
```
// FIXME: Change test to use 'Cstrlen' instead of 'strlen_test' when macro printing is fixed.
```


http://reviews.llvm.org/D12379