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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
