zhengkai created this revision.
zhengkai added a reviewer: rtrieu.
zhengkai added a subscriber: cfe-commits.

Use a new algorithm to find map back the ranges to its spelling locations.

But the function is not working properly either, and it breaks a test.

In the last case in the test file 
/llvm/tools/clang/test/Misc/caret-diags-macros.c,
it underlined the wrong range. This is reported as a bug in 
https://llvm.org/bugs/show_bug.cgi?id=24592.

The methods which are used to map back the source locations in the macro 
expansions are questionable in the SourceManager Class.

http://reviews.llvm.org/D12379

Files:
  lib/Frontend/DiagnosticRenderer.cpp
  test/Index/fix-its.m
  test/Misc/caret-diags-macros.c
  test/Misc/serialized-diags.c

Index: test/Misc/serialized-diags.c
===================================================================
--- test/Misc/serialized-diags.c
+++ test/Misc/serialized-diags.c
@@ -55,7 +55,6 @@
 // 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]
Index: test/Misc/caret-diags-macros.c
===================================================================
--- test/Misc/caret-diags-macros.c
+++ test/Misc/caret-diags-macros.c
@@ -16,9 +16,6 @@
 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
@@ -41,48 +38,45 @@
 
 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
@@ -91,12 +85,12 @@
 
 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
@@ -107,22 +101,22 @@
 
 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: {{                                        \^}}
 }
@@ -129,13 +123,13 @@
 
 #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: {{^                                 ~~~~~\^~~~}}
 
@@ -143,19 +137,19 @@
 #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: {{^            ~~~~~~~~\^~~~~~~~~}}
 
@@ -162,13 +156,13 @@
 #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: {{^              \^}}
 
@@ -179,10 +173,10 @@
 #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: {{^                                  ~~~~~ \^}}
 
@@ -193,16 +187,16 @@
 #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: {{^                           ~~ \^  ~~~~~~~~~~~}}
 
@@ -215,13 +209,13 @@
 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: {{^                                                       \^}}
Index: test/Index/fix-its.m
===================================================================
--- test/Index/fix-its.m
+++ test/Index/fix-its.m
@@ -20,7 +20,3 @@
 @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
Index: lib/Frontend/DiagnosticRenderer.cpp
===================================================================
--- lib/Frontend/DiagnosticRenderer.cpp
+++ lib/Frontend/DiagnosticRenderer.cpp
@@ -323,9 +323,9 @@
     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();
 
@@ -355,23 +355,22 @@
     }
 
     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);
@@ -423,6 +422,7 @@
                  SpellingRanges, None, &SM);
 }
 
+
 static bool checkRangeForMacroArgExpansion(CharSourceRange Range,
                                            const SourceManager &SM) {
   SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
@@ -435,8 +435,9 @@
   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) {
@@ -445,12 +446,34 @@
   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;
 }
@@ -478,14 +501,14 @@
   unsigned IgnoredEnd = 0;
   while (Loc.isMacroID()) {
     LocationStack.push_back(Loc);
-    if (checkRangesForMacroArgExpansion(Loc, Ranges, SM))
-      IgnoredEnd = LocationStack.size();
-
+    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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to