Fix the bugs in the mapDiagnosticRanges (Still in progress)

[zhengkai]

Summary

Use a backtracing alogirthm to match all possible source locations to match the FileID of the Caret location.
This can map the ranges back as much as possible.
You can see some new ranges underlined in the diagnostic information In the test cases modified. This can prove that the new method is effective.
(Solved https://llvm.org/bugs/show_bug.cgi?id=24592)

Also change the standard to judge if the macro backtraces should be reduced.
Using raw encodings to specify if the ranges are all in one argument of the macro invocations.
But this part is still questionable due to some bugs inside the preprocessor system. (See: https://llvm.org/bugs/show_bug.cgi?id=24788)

[rtrieu]

Why are you leaking the raw encoding of locations from the SourceManager? This is an internal implementation detail and should not be relied on externally. SourceLocation is the typical way to use locations.

[zhengkai]

In D12379#245849, @rtrieu wrote:

Why are you leaking the raw encoding of locations from the SourceManager? This is an internal implementation detail and should not be relied on externally. SourceLocation is the typical way to use locations.

Because I want to check if all the ranges fit in the same argument of one macro invocation, they can be different Source Locations. In ExpansionInfo, these locations have the same ExpansionLocStart.

[rtrieu]

In D12379#245850, @zhengkai wrote:

In D12379#245849, @rtrieu wrote:

Why are you leaking the raw encoding of locations from the SourceManager? This is an internal implementation detail and should not be relied on externally. SourceLocation is the typical way to use locations.

Because I want to check if all the ranges fit in the same argument of one macro invocation, they can be different Source Locations. In ExpansionInfo, these locations have the same ExpansionLocStart.

If you call ExpansionInfo::getExpansionLocStart(), then the SourceLocation will be the same if the ExpansionLocStart is the same.

[zhengkai]

In D12379#245861, @rtrieu wrote:

In D12379#245850, @zhengkai wrote:

In D12379#245849, @rtrieu wrote:

Why are you leaking the raw encoding of locations from the SourceManager? This is an internal implementation detail and should not be relied on externally. SourceLocation is the typical way to use locations.

Because I want to check if all the ranges fit in the same argument of one macro invocation, they can be different Source Locations. In ExpansionInfo, these locations have the same ExpansionLocStart.

If you call ExpansionInfo::getExpansionLocStart(), then the SourceLocation will be the same if the ExpansionLocStart is the same.

But I still need to change the isMacroArg function to get the result of getExpansionLocStart(), and if I want to compare two SourceLocation, I still need to check the raw encoding. I don't know if it is needed not to leak the raw encoding.

[rtrieu]

In D12379#245868, @zhengkai wrote:

In D12379#245861, @rtrieu wrote:

In D12379#245850, @zhengkai wrote:

In D12379#245849, @rtrieu wrote:

Why are you leaking the raw encoding of locations from the SourceManager? This is an internal implementation detail and should not be relied on externally. SourceLocation is the typical way to use locations.

Because I want to check if all the ranges fit in the same argument of one macro invocation, they can be different Source Locations. In ExpansionInfo, these locations have the same ExpansionLocStart.

If you call ExpansionInfo::getExpansionLocStart(), then the SourceLocation will be the same if the ExpansionLocStart is the same.

But I still need to change the isMacroArg function to get the result of getExpansionLocStart(), and if I want to compare two SourceLocation, I still need to check the raw encoding. I don't know if it is needed not to leak the raw encoding.

I kind of see what you are trying to do here, and we should fix it a different way.

1. Get rid of changes to ExpansionInfo
2. Change the function SourceManager::isMacroArgExpansion to take two arguments instead of one. The new argument should be a SourceLocation pointer named MacroBegin.
3. Change the last line of SourceManager::isMacroArgExpansion from "return Expansion.isMacroArgExpansion();" to "if (!Expansion.isMacroArgExpansion()) return false;" After that, add the code to get the macro begin location.
4. Update DiagnosticRenderer to use SourceLocation instead of raw encodings.

[zhengkai]

Use the rtrieu's method to re write the implementation of SourceManager.

[zhengkai]

In D12379#245892, @rtrieu wrote:

In D12379#245868, @zhengkai wrote:

In D12379#245861, @rtrieu wrote:

In D12379#245850, @zhengkai wrote:

In D12379#245849, @rtrieu wrote:

Why are you leaking the raw encoding of locations from the SourceManager? This is an internal implementation detail and should not be relied on externally. SourceLocation is the typical way to use locations.

Because I want to check if all the ranges fit in the same argument of one macro invocation, they can be different Source Locations. In ExpansionInfo, these locations have the same ExpansionLocStart.

If you call ExpansionInfo::getExpansionLocStart(), then the SourceLocation will be the same if the ExpansionLocStart is the same.

But I still need to change the isMacroArg function to get the result of getExpansionLocStart(), and if I want to compare two SourceLocation, I still need to check the raw encoding. I don't know if it is needed not to leak the raw encoding.

I kind of see what you are trying to do here, and we should fix it a different way.

1. Get rid of changes to ExpansionInfo
2. Change the function SourceManager::isMacroArgExpansion to take two arguments instead of one. The new argument should be a SourceLocation pointer named MacroBegin.
3. Change the last line of SourceManager::isMacroArgExpansion from "return Expansion.isMacroArgExpansion();" to "if (!Expansion.isMacroArgExpansion()) return false;" After that, add the code to get the macro begin location.
4. Update DiagnosticRenderer to use SourceLocation instead of raw encodings.

Have changed the implementation.

[rtrieu]

include/clang/Basic/SourceManager.h

333 ↗

(On Diff #34826)

Remove unrelated whitespace change.

1155–1164 ↗

(On Diff #34826)

Merge the two isMacroArgExpansion functions by making the second argument a SourceLocation pointer with default argument of nullptr. See isAtStartOfImmediateMacroExpansion or isAtEndOfImmediateMacroExpansion functions for how this is done.

lib/Basic/SourceManager.cpp

1015–1024 ↗

(On Diff #34826)

Merge with other isMacroArgExpansion function

lib/Frontend/DiagnosticRenderer.cpp

311

No newline between comment and function. And what is the logic behind in this function that wasn't in the code it is replacing?

312

Why not return a SourceLocation instead of passing one in by reference?

313

Why is this passed by reference?

314–350

Use more vertical whitespace when coding.

323

In the other function, there is "End = SM->getImmediateExpansionRange(Backup).second;" Why is there not a "Begin = SM->getImmediateExpansionRange(Backup).first;" here?

331

There is a lot of duplicated code between these two functions. Can they merged together with an option to select whether to check the start or end locations?

352–367

Function not used; remove it.

417–418

Can these function be rewritten to return SourceLocation's? Then it would be:
Begin = retrieveBeginLocation(Begin, BeginFileID, CaretLocFileID, SM);
End = retrieveEndLocation(End, EndFileID, CaretLocFileID, SM);

if (Begin.invalid() || End.invalid()) continue;

466

What is this function doing that is not captured by the old isMacroArgExpansion function?

469

Drop the "= SourceLocation". Default initialization of SourceLocation does the same thing.

471

Use "PreLoc.isInvalid()" instead of "PreLoc == SourceLocation()".

504

Why does the range count matter?

512

Comment is out of date, no longer holding the raw encoding. Rename the variable to ArgumentLoc to convey what the SourceLocation is holding.

513

Drop the "= SourceLocation()". Default initialization for SourceLocation does the same thing.

test/Misc/caret-diags-macros.c

210

Why is Cstrlen changed to strlen_test?

[zhengkai]

lib/Frontend/DiagnosticRenderer.cpp

312

Because this function has two recursive calls inside.
So if return a SourceLocation here, I will need to use a temporal variable to store the result of the first call.
And this doesn't make it easier.

323

Begin = SM->getImmediateMacroCallerLoc(Begin);

Use getImmediateMacroCallerLoc function here.

331

I tried it but the code doesn't seem easier.

352–367

deleted. But I wonder this function is important for those who want to debug this code.

417–418

Replied before.

466

I don't understand what you mean here.

504

Because some ranges passed in are invalid.
And if the mapDiagnosticRanges function drops some ranges, this means this macro call doesn't contain all the information needed of all the ranges.

test/Misc/caret-diags-macros.c

210

The preprocessor can't get right when the argument itself is a function-like macro. This bug is not due to the high-level macro case but due to this problem.

[zhengkai]

lib/Frontend/DiagnosticRenderer.cpp

455

Because the function isMacroArgExpansion doesn't check if all locations are in the same argument location,

[rtrieu]

lib/Basic/SourceManager.cpp

1008 ↗

(On Diff #34943)

Fix alignment so function arguments line up.

1013 ↗

(On Diff #34943)

Add line break after this line.

lib/Frontend/DiagnosticRenderer.cpp

323

You did not add that here.

331

Using reference parameters to input/output data into functions is confusing and generally not done. SourceLocation is a small enough class that it is commonly used as return values and it has a built-in invalid value. Use the following function:

static SourceLocation retrieveMacroLocation(SourceLocation Loc,
                                            FileID MacroFileID,
                                            FileID CaretFileID,
                                            bool getBeginLoc,
                                            const SourceManager *SM) {
  if (MacroFileID == CaretFileID) return Loc;
  if (!Loc.isMacroID()) return SourceLocation();

  SourceLocation MacroLocation, MacroArgLocation;

  if (SM->isMacroArgExpansion(Loc)) {
    MacroLocation = SM->getImmediateMacroCallerLoc(Loc);
    MacroArgLocation = getBeginLoc ? SM->getImmediateExpansionRange(Loc).first
                                   : SM->getImmediateExpansionRange(Loc).second;
  } else {
    MacroLocation = getBeginLoc ? SM->getImmediateExpansionRange(Loc).first
                                : SM->getImmediateExpansionRange(Loc).second;
    MacroArgLocation = SM->getImmediateSpellingLoc(Loc);
  }

  MacroFileID = SM->getFileID(MacroLocation);
  MacroLocation = retrieveMacroLocation(MacroLocation, MacroFileID, CaretFileID,
                                        getBeginLoc, SM);
  if (MacroLocation.isValid()) return MacroLocation;

  MacroFileID = SM->getFileID(MacroArgLocation);
  return retrieveMacroLocation(MacroArgLocation, MacroFileID, CaretFileID,
                               getBeginLoc, SM);
}

352–367

It is more important to have good comments than debug functions since that saves the trouble of running a debugger. If someone is debugging this code, this function would be in the wrong place. Instead, the SourceManager would be a better place to put it, and it saves the trouble of passing in a SourceManager object. Besides that, there special considerations for debugging functions, like annotating it properly so it only shows up in debug builds and not increase the size of regular builds. If you still feel it is important, then the work should go into another patch.

406–407

Use:

Begin = retrieveMacroLocation(Begin, BeginFileID, CaretLocFileID,
                              true /*getBeginLoc*/, SM);
End = retrieveMacroLocation(End, BeginFileID, CaretLocFileID,
                            false /*getBeginLoc*/, SM);
if (Begin.isInvalid() || End.isInvalid()) continue;

with the fixed function above.

455

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.

460

Do the same for NewLoc

471

What is this SourceLocation? Why is it passed by reference?

502

Not done.

test/Misc/caret-diags-macros.c

210

Add a FIXME comment that it should be changed back to Cstrlen when the bug is resolved.

[zhengkai]

lib/Frontend/DiagnosticRenderer.cpp

323

The getImmediateMacroCallerLoc is implemented as following:

SourceLocation getImmediateMacroCallerLoc(SourceLocation Loc) const {
  if (!Loc.isMacroID()) return Loc;
  if (isMacroArgExpansion(Loc))
    return getImmediateSpellingLoc(Loc);

  return getImmediateExpansionRange(Loc).first;
}

471

Changed

[rtrieu]

lib/Frontend/DiagnosticRenderer.cpp

455

Still not answered: What is PreLoc? Why is it important to this function? What does PreLoc even mean?

471

What is Loc? Where is it pointing to? Why do you need it?

test/Misc/caret-diags-macros.c

210

Use:

// FIXME: Change test to use 'Cstrlen' instead of 'strlen_test' when macro printing is fixed.

[zhengkai]

lib/Frontend/DiagnosticRenderer.cpp

442

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.

454

See the comment ahead.

[rtrieu]

lib/Frontend/DiagnosticRenderer.cpp

442

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.

[zhengkai]

lib/Frontend/DiagnosticRenderer.cpp

442

Explained in the new comments in the updated diff.

lib/Frontend/DiagnosticRenderer.cpp

     const SourceManager *SM) {
   FileID CaretLocFileID = SM->getFileID(CaretLoc);
 
-  for (ArrayRef<CharSourceRange>::const_iterator I = Ranges.begin(),
-       E = Ranges.end();
-       I != E; ++I) {
+  for (auto I = Ranges.begin(), E = Ranges.end(); I != E; ++I) {
+    if (I->isInvalid()) continue;
+
     SourceLocation Begin = I->getBegin(), End = I->getEnd();
     bool IsTokenRange = I->isTokenRange();
 

     }
 
     while (Begin.isMacroID() && BeginFileID != CaretLocFileID) {
-      if (SM->isMacroArgExpansion(Begin)) {
+      if (SM->isMacroArgExpansion(Begin))
         Begin = SM->getImmediateSpellingLoc(Begin);
-        End = SM->getImmediateSpellingLoc(End);
-      } else {
-        Begin = SM->getImmediateExpansionRange(Begin).first;
-        End = SM->getImmediateExpansionRange(End).second;
-      }
+      else Begin = SM->getImmediateExpansionRange(Begin).first;
       BeginFileID = SM->getFileID(Begin);
-      if (BeginFileID != SM->getFileID(End)) {
-        // FIXME: Ugly hack to stop a crash; this code is making bad
-        // assumptions and it's too complicated for me to reason
-        // about.
-        Begin = End = SourceLocation();
-        break;
-      }
     }
+    
 
+    while (End.isMacroID() && EndFileID != CaretLocFileID) {
+      if (SM->isMacroArgExpansion(End))
+        End = SM->getImmediateSpellingLoc(End);
+      else End = SM->getImmediateExpansionRange(End).second;
+      EndFileID = SM->getFileID(End);
+    } 
+
+    if (BeginFileID != CaretLocFileID || EndFileID != CaretLocFileID) continue;
+
     // Return the spelling location of the beginning and end of the range.
     Begin = SM->getSpellingLoc(Begin);
     End = SM->getSpellingLoc(End);

                  SpellingRanges, None, &SM);
 }
 
+
 static bool checkRangeForMacroArgExpansion(CharSourceRange Range,
                                            const SourceManager &SM) {
   SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();

   return SM.isMacroArgExpansion(BegLoc);
 }
 
-/// A helper function to check if the current ranges are all inside
+/// A helper function to check if the current ranges are all inside 
 /// the macro expansions.
+
 static bool checkRangesForMacroArgExpansion(SourceLocation Loc,
                                             ArrayRef<CharSourceRange> Ranges,
                                             const SourceManager &SM) {

   SmallVector<CharSourceRange, 4> SpellingRanges;
   mapDiagnosticRanges(Loc, Ranges, SpellingRanges, &SM);
 
+  // The next snippet of code is supposed to be right but accually not
+  // working properly. I believe it is due to some other parts of the code.
+  // So I commented it.
+
+  /*
+  unsigned ValidCount = 0;
+  for (auto I:Ranges)
+    if (I.isValid()) ValidCount++;
+
+  if (SpellingRanges.size() < ValidCount)
+    return false;
+  */
+
+  if (SpellingRanges.size()>1) {
+    for (auto I = SpellingRanges.begin() + 1, E = SpellingRanges.end(), 
+         B = SpellingRanges.begin(); I != E; ++I)
+      if (I->getAsRange() != B->getAsRange()) return false;
+  }
+
+
   if (!SM.isMacroArgExpansion(Loc))
     return false;
 
-  for (auto I = SpellingRanges.begin(), E = SpellingRanges.end(); I != E; ++I)
+  for (auto I = SpellingRanges.begin(), E = SpellingRanges.end();
+       I != E; ++I) {
     if (!checkRangeForMacroArgExpansion(*I, SM))
       return false;
+  }
 
   return true;
 }

   unsigned IgnoredEnd = 0;
   while (Loc.isMacroID()) {
     LocationStack.push_back(Loc);
-    if (checkRangesForMacroArgExpansion(Loc, Ranges, SM))
+    if (checkRangesForMacroArgExpansion(Loc, Ranges, SM)) {
         IgnoredEnd = LocationStack.size();
-
+    }
     Loc = SM.getImmediateMacroCallerLoc(Loc);
     assert(!Loc.isInvalid() && "must have a valid source location here");
   }
 
-  LocationStack.erase(LocationStack.begin(),
+  LocationStack.erase(LocationStack.begin(), 
                       LocationStack.begin() + IgnoredEnd);
 
   unsigned MacroDepth = LocationStack.size();

test/Index/fix-its.m

 @end
 
 // CHECK: FIX-IT: Insert "@" at 18:22
-// CHECK: fix-its.m:9:28: note: expanded from macro '_rdar_12584554_C'
-// CHECK: Number FIX-ITs = 0
-// CHECK: fix-its.m:7:77: note: expanded from macro '_rdar_12584554_B'
-// CHECK: Number FIX-ITs = 0

test/Misc/caret-diags-macros.c

 void bar() {
   C(1);
   // CHECK: {{.*}}:17:5: warning: expression result unused
-  // CHECK: {{.*}}:15:16: note: expanded from macro 'C'
-  // CHECK: {{.*}}:14:16: note: expanded from macro 'B'
-  // CHECK: {{.*}}:13:14: note: expanded from macro 'A'
 }
 
 // rdar://7597492

 
 void test() {
   macro_args3(11);
-  // CHECK: {{.*}}:43:15: warning: expression result unused
+  // CHECK: {{.*}}:40:15: warning: expression result unused
   // Also check that the 'caret' printing agrees with the location here where
   // its easy to FileCheck.
   // CHECK-NEXT:      macro_args3(11);
   // CHECK-NEXT: {{^              \^~}}
-  // CHECK: {{.*}}:36:36: note: expanded from macro 'macro_args3'
-  // CHECK: {{.*}}:35:36: note: expanded from macro 'macro_args2'
-  // CHECK: {{.*}}:34:24: note: expanded from macro 'macro_args1'
 
   macro_many_args3(
     1,
     2,
     3);
-  // CHECK: {{.*}}:55:5: warning: expression result unused
-  // CHECK: {{.*}}:40:55: note: expanded from macro 'macro_many_args3'
-  // CHECK: {{.*}}:39:55: note: expanded from macro 'macro_many_args2'
-  // CHECK: {{.*}}:38:35: note: expanded from macro 'macro_many_args1'
+  // CHECK: {{.*}}:49:5: warning: expression result unused
+  // CHECK: {{.*}}:37:55: note: expanded from macro 'macro_many_args3'
+  // CHECK: {{.*}}:36:55: note: expanded from macro 'macro_many_args2'
+  // CHECK: {{.*}}:35:35: note: expanded from macro 'macro_many_args1'
 
   macro_many_args3(
     1,
     M2,
     3);
-  // CHECK: {{.*}}:64:5: warning: expression result unused
+  // CHECK: {{.*}}:58:5: warning: expression result unused
   // CHECK: {{.*}}:4:12: note: expanded from macro 'M2'
-  // CHECK: {{.*}}:40:55: note: expanded from macro 'macro_many_args3'
-  // CHECK: {{.*}}:39:55: note: expanded from macro 'macro_many_args2'
-  // CHECK: {{.*}}:38:35: note: expanded from macro 'macro_many_args1'
+  // CHECK: {{.*}}:37:55: note: expanded from macro 'macro_many_args3'
+  // CHECK: {{.*}}:36:55: note: expanded from macro 'macro_many_args2'
+  // CHECK: {{.*}}:35:35: note: expanded from macro 'macro_many_args1'
 
   macro_many_args3(
     1,
     macro_args2(22),
     3);
-  // CHECK: {{.*}}:74:17: warning: expression result unused
+  // CHECK: {{.*}}:68:17: warning: expression result unused
   // This caret location needs to be printed *inside* a different macro's
   // arguments.
   // CHECK-NEXT:        macro_args2(22),
   // CHECK-NEXT: {{^                \^~}}
-  // CHECK: {{.*}}:35:36: note: expanded from macro 'macro_args2'
-  // CHECK: {{.*}}:34:24: note: expanded from macro 'macro_args1'
-  // CHECK: {{.*}}:40:55: note: expanded from macro 'macro_many_args3'
-  // CHECK: {{.*}}:39:55: note: expanded from macro 'macro_many_args2'
-  // CHECK: {{.*}}:38:35: note: expanded from macro 'macro_many_args1'
+  // CHECK: {{.*}}:32:36: note: expanded from macro 'macro_args2'
+  // CHECK: {{.*}}:31:24: note: expanded from macro 'macro_args1'
+  // CHECK: {{.*}}:37:55: note: expanded from macro 'macro_many_args3'
+  // CHECK: {{.*}}:36:55: note: expanded from macro 'macro_many_args2'
+  // CHECK: {{.*}}:35:35: note: expanded from macro 'macro_many_args1'
 }
 
 #define variadic_args1(x, y, ...) y

 
 void test2() {
   variadic_args3(1, 22, 3, 4);
-  // CHECK: {{.*}}:93:21: warning: expression result unused
+  // CHECK: {{.*}}:87:21: warning: expression result unused
   // CHECK-NEXT:      variadic_args3(1, 22, 3, 4);
   // CHECK-NEXT: {{^                    \^~}}
-  // CHECK: {{.*}}:90:53: note: expanded from macro 'variadic_args3'
-  // CHECK: {{.*}}:89:50: note: expanded from macro 'variadic_args2'
-  // CHECK: {{.*}}:88:35: note: expanded from macro 'variadic_args1'
+  // CHECK: {{.*}}:84:53: note: expanded from macro 'variadic_args3'
+  // CHECK: {{.*}}:83:50: note: expanded from macro 'variadic_args2'
+  // CHECK: {{.*}}:82:35: note: expanded from macro 'variadic_args1'
 }
 
 #define variadic_pasting_args1(x, y, z) y

 
 void test3() {
   variadic_pasting_args3(1, 2, 3, 4);
-  // CHECK: {{.*}}:109:32: warning: expression result unused
-  // CHECK: {{.*}}:105:72: note: expanded from macro 'variadic_pasting_args3'
-  // CHECK: {{.*}}:103:68: note: expanded from macro 'variadic_pasting_args2'
-  // CHECK: {{.*}}:102:41: note: expanded from macro 'variadic_pasting_args1'
+  // CHECK: {{.*}}:103:32: warning: expression result unused
+  // CHECK: {{.*}}:99:72: note: expanded from macro 'variadic_pasting_args3'
+  // CHECK: {{.*}}:97:68: note: expanded from macro 'variadic_pasting_args2'
+  // CHECK: {{.*}}:96:41: note: expanded from macro 'variadic_pasting_args1'
 
   variadic_pasting_args3a(1, 2, 3, 4);
-  // CHECK:        {{.*}}:115:3: warning: expression result unused
+  // CHECK:        {{.*}}:109:3: warning: expression result unused
   // CHECK-NEXT:     variadic_pasting_args3a(1, 2, 3, 4);
   // CHECK-NEXT: {{  \^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~}}
-  // CHECK:        {{.*}}:106:44: note: expanded from macro 'variadic_pasting_args3a'
+  // CHECK:        {{.*}}:100:44: note: expanded from macro 'variadic_pasting_args3a'
   // CHECK-NEXT:   #define variadic_pasting_args3a(x, y, ...) variadic_pasting_args2a(x, y, __VA_ARGS__)
   // CHECK-NEXT: {{                                           \^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~}}
-  // CHECK:        {{.*}}:104:70: note: expanded from macro 'variadic_pasting_args2a'
+  // CHECK:        {{.*}}:98:70: note: expanded from macro 'variadic_pasting_args2a'
   // CHECK-NEXT:   #define variadic_pasting_args2a(x, y, ...) variadic_pasting_args1(x, y ## __VA_ARGS__)
   // CHECK-NEXT: {{                                                                     \^~~~~~~~~~~~~~~~}}
-  // CHECK:        {{.*}}:102:41: note: expanded from macro 'variadic_pasting_args1'
+  // CHECK:        {{.*}}:96:41: note: expanded from macro 'variadic_pasting_args1'
   // CHECK-NEXT:   #define variadic_pasting_args1(x, y, z) y
   // CHECK-NEXT: {{                                        \^}}
 }

 
 #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
 int test4 = BAD_CONDITIONAL_OPERATOR+BAD_CONDITIONAL_OPERATOR;
-// CHECK:         {{.*}}:130:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
+// CHECK:         {{.*}}:124:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
 // CHECK-NEXT:    #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
 // CHECK-NEXT: {{^                                      \^}}
-// CHECK:         {{.*}}:130:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
+// CHECK:         {{.*}}:124:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
 // CHECK-NEXT:    #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
 // CHECK-NEXT: {{^                                      \^}}
-// CHECK:         {{.*}}:130:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
+// CHECK:         {{.*}}:124:39: note: expanded from macro 'BAD_CONDITIONAL_OPERATOR'
 // CHECK-NEXT:    #define BAD_CONDITIONAL_OPERATOR (2<3)?2:3
 // CHECK-NEXT: {{^                                 ~~~~~\^~~~}}
 

 #define TWOL (2<
 #define X 1+TWOL 3) QMARK 4:5
 int x = X;
-// CHECK:         {{.*}}:145:9: note: place parentheses around the '+' expression to silence this warning
+// CHECK:         {{.*}}:139:9: note: place parentheses around the '+' expression to silence this warning
 // CHECK-NEXT:    int x = X;
 // CHECK-NEXT: {{^        \^}}
-// CHECK-NEXT:    {{.*}}:144:21: note: expanded from macro 'X'
+// CHECK-NEXT:    {{.*}}:138:21: note: expanded from macro 'X'
 // CHECK-NEXT:    #define X 1+TWOL 3) QMARK 4:5
 // CHECK-NEXT: {{^          ~~~~~~~~~ \^}}
-// CHECK-NEXT:    {{.*}}:142:15: note: expanded from macro 'QMARK'
+// CHECK-NEXT:    {{.*}}:136:15: note: expanded from macro 'QMARK'
 // CHECK-NEXT:    #define QMARK ?
 // CHECK-NEXT: {{^              \^}}
-// CHECK-NEXT:    {{.*}}:145:9: note: place parentheses around the '?:' expression to evaluate it first
+// CHECK-NEXT:    {{.*}}:139:9: note: place parentheses around the '?:' expression to evaluate it first
 // CHECK-NEXT:    int x = X;
 // CHECK-NEXT: {{^        \^}}
-// CHECK-NEXT:    {{.*}}:144:21: note: expanded from macro 'X'
+// CHECK-NEXT:    {{.*}}:138:21: note: expanded from macro 'X'
 // CHECK-NEXT:    #define X 1+TWOL 3) QMARK 4:5
 // CHECK-NEXT: {{^            ~~~~~~~~\^~~~~~~~~}}
 

 #define ONEPLUS 1+
 #define Y ONEPLUS (2<3) QMARK 4:5
 int y = Y;
-// CHECK:         {{.*}}:164:9: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first
+// CHECK:         {{.*}}:158:9: warning: operator '?:' has lower precedence than '+'; '+' will be evaluated first
 // CHECK-NEXT:    int y = Y;
 // CHECK-NEXT: {{^        \^}}
-// CHECK-NEXT:    {{.*}}:163:25: note: expanded from macro 'Y'
+// CHECK-NEXT:    {{.*}}:157:25: note: expanded from macro 'Y'
 // CHECK-NEXT:    #define Y ONEPLUS (2<3) QMARK 4:5
 // CHECK-NEXT: {{^          ~~~~~~~~~~~~~ \^}}
-// CHECK-NEXT:    {{.*}}:142:15: note: expanded from macro 'QMARK'
+// CHECK-NEXT:    {{.*}}:136:15: note: expanded from macro 'QMARK'
 // CHECK-NEXT:    #define QMARK ?
 // CHECK-NEXT: {{^              \^}}
 

 #define /* */ BARC(c, /* */b, a) (a + b ? c : c)
   iequals(__LINE__, BARC(123, (456 < 345), 789), 8);
 }
-// CHECK:         {{.*}}:180:21: warning: operator '?:' has lower precedence than '+'
+// CHECK:         {{.*}}:174:21: warning: operator '?:' has lower precedence than '+'
 // CHECK-NEXT:      iequals(__LINE__, BARC(123, (456 < 345), 789), 8);
 // CHECK-NEXT: {{^                    \^~~~~~~~~~~~~~~~~~~~~~~~~~~}}
-// CHECK-NEXT:    {{.*}}:179:41: note: expanded from macro 'BARC'
+// CHECK-NEXT:    {{.*}}:173:41: note: expanded from macro 'BARC'
 // CHECK-NEXT:    #define /* */ BARC(c, /* */b, a) (a + b ? c : c)
 // CHECK-NEXT: {{^                                  ~~~~~ \^}}
 

 #if UTARG_MAX_U
 #endif
 
-// CHECK:         {{.*}}:193:5: warning: left side of operator converted from negative value to unsigned: -1 to 18446744073709551615
+// CHECK:         {{.*}}:187:5: warning: left side of operator converted from negative value to unsigned: -1 to 18446744073709551615
 // CHECK-NEXT:    #if UTARG_MAX_U
 // CHECK-NEXT: {{^    \^~~~~~~~~~~}}
-// CHECK-NEXT:    {{.*}}:191:21: note: expanded from macro 'UTARG_MAX_U'
+// CHECK-NEXT:    {{.*}}:185:21: note: expanded from macro 'UTARG_MAX_U'
 // CHECK-NEXT:    #define UTARG_MAX_U APPEND (MAX_UINT, UL)
 // CHECK-NEXT: {{^                    \^~~~~~~~~~~~~~~~~~~~~}}
-// CHECK-NEXT:    {{.*}}:190:27: note: expanded from macro 'APPEND'
+// CHECK-NEXT:    {{.*}}:184:27: note: expanded from macro 'APPEND'
 // CHECK-NEXT:    #define APPEND(NUM, SUFF) APPEND2(NUM, SUFF)
 // CHECK-NEXT: {{^                          \^~~~~~~~~~~~~~~~~~}}
-// CHECK-NEXT:    {{.*}}:189:31: note: expanded from macro 'APPEND2'
+// CHECK-NEXT:    {{.*}}:183:31: note: expanded from macro 'APPEND2'
 // CHECK-NEXT:    #define APPEND2(NUM, SUFF) -1 != NUM ## SUFF
 // CHECK-NEXT: {{^                           ~~ \^  ~~~~~~~~~~~}}
 

 void f(char* pMsgBuf, char* pKeepBuf) {
 Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ", Cstrlen(pKeepBuf));
 }
-// CHECK:         {{.*}}:216:62: warning: format specifies type 'int' but the argument has type 'unsigned long'
+// CHECK:         {{.*}}:210:62: warning: format specifies type 'int' but the argument has type 'unsigned long'
 // CHECK-NEXT:    Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ", Cstrlen(pKeepBuf));
 // CHECK-NEXT: {{^                                                    ~~~      \^}}
 // CHECK-NEXT: {{^                                                    %1lu}}
-// CHECK-NEXT:    {{.*}}:213:21: note: expanded from macro 'Cstrlen'
+// CHECK-NEXT:    {{.*}}:207:21: note: expanded from macro 'Cstrlen'
 // CHECK-NEXT:    #define Cstrlen(a)  strlen_test(a)
 // CHECK-NEXT: {{^                    \^}}
-// CHECK-NEXT:    {{.*}}:212:56: note: expanded from macro 'sprintf2'
+// CHECK-NEXT:    {{.*}}:206:56: note: expanded from macro 'sprintf2'
 // CHECK-NEXT:      __builtin___sprintf_chk (str, 0, __darwin_obsz(str), __VA_ARGS__)
 // CHECK-NEXT: {{^                                                       \^}}

test/Misc/serialized-diags.c

 // CHECK: Range: {{.*[/\\]}}serialized-diags.c:22:3 {{.*[/\\]}}serialized-diags.c:22:6
 // CHECK: Range: {{.*[/\\]}}serialized-diags.c:22:13 {{.*[/\\]}}serialized-diags.c:22:18
 // CHECK: +-{{.*[/\\]}}serialized-diags.c:20:15: note: expanded from macro 'false' []
-// CHECK: +-Range: {{.*[/\\]}}serialized-diags.c:22:3 {{.*[/\\]}}serialized-diags.c:22:6
 // CHECK: +-Range: {{.*[/\\]}}serialized-diags.c:20:15 {{.*[/\\]}}serialized-diags.c:20:16
 // CHECK: +-{{.*[/\\]}}serialized-diags.c:19:1: note: 'taz' declared here []
 // CHECK: {{.*[/\\]}}serialized-diags.h:5:7: warning: incompatible integer to pointer conversion initializing 'char *' with an expression of type 'int' [-Wint-conversion]