[cfe-commits] r170049 - in /cfe/trunk: lib/Frontend/DiagnosticRenderer.cpp test/Misc/caret-diags-macros.c
Eli Friedman
eli.friedman at gmail.com
Wed Dec 12 16:15:00 PST 2012
Author: efriedma
Date: Wed Dec 12 18:14:59 2012
New Revision: 170049
URL: http://llvm.org/viewvc/llvm-project?rev=170049&view=rev
Log:
More hacking on mapDiagnosticRanges to make it handle more cases.
This still isn't quite right, but it fixes a crash.
I factored out findCommonParent because we need it on the result of
getImmediateExpansionRange: for a function macro, the beginning
and end of an expansion range can come out of different
macros/macro arguments, which means the resulting range is a complete
mess to handle consistently.
I also made some changes to how findCommonParent works; it works somewhat
better in some cases, and somewhat worse in others, but I think overall
it's a better balance. I'm coming to the conclusion that mapDiagnosticRanges
isn't using the right algorithm, though: chasing the caret is fundamentally
more complicated than any algorithm which only considers one FileID for the
caret can handle because each SourceLocation doesn't really have a single parent.
We need to follow the same path of choosing expansion locations and spelling
locations which the caret used to come up with the correct range
in the general case.
Fixes <rdar://problem/12847524>.
Modified:
cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
cfe/trunk/test/Misc/caret-diags-macros.c
Modified: cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp?rev=170049&r1=170048&r2=170049&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp (original)
+++ cfe/trunk/lib/Frontend/DiagnosticRenderer.cpp Wed Dec 12 18:14:59 2012
@@ -306,6 +306,56 @@
}
}
+// Find the common parent for the beginning and end of the range.
+static void findCommonParent(SourceLocation &Begin, SourceLocation &End,
+ const SourceManager *SM) {
+ if (Begin.isInvalid() || End.isInvalid()) {
+ Begin = End = SourceLocation();
+ return;
+ }
+
+ FileID BeginFileID = SM->getFileID(Begin);
+ FileID EndFileID = SM->getFileID(End);
+
+ // First, crawl the expansion chain for the beginning of the range.
+ llvm::SmallDenseMap<FileID, SourceLocation> BeginLocsMap;
+ BeginLocsMap[BeginFileID] = Begin;
+ while (Begin.isMacroID() && BeginFileID != EndFileID) {
+ if (SM->isMacroArgExpansion(Begin)) {
+ Begin = SM->getImmediateSpellingLoc(Begin);
+ if (Begin.isMacroID())
+ continue;
+ Begin = SourceLocation();
+ BeginFileID = FileID();
+ break;
+ }
+ Begin = SM->getImmediateExpansionRange(Begin).first;
+ BeginFileID = SM->getFileID(Begin);
+ BeginLocsMap[BeginFileID] = Begin;
+ }
+
+ // Then, crawl the expansion chain for the end of the range.
+ if (BeginFileID != EndFileID) {
+ while (End.isMacroID() && !BeginLocsMap.count(EndFileID)) {
+ if (SM->isMacroArgExpansion(End)) {
+ End = SM->getImmediateSpellingLoc(End);
+ if (End.isMacroID())
+ continue;
+ End = SourceLocation();
+ EndFileID = FileID();
+ break;
+ }
+ End = SM->getImmediateExpansionRange(End).second;
+ EndFileID = SM->getFileID(End);
+ }
+ if (End.isMacroID()) {
+ Begin = BeginLocsMap[EndFileID];
+ BeginFileID = EndFileID;
+ }
+ }
+ assert(Begin.isValid() == End.isValid());
+}
+
// Helper function to fix up source ranges. It takes in an array of ranges,
// and outputs an array of ranges where we want to draw the range highlighting
// around the location specified by CaretLoc.
@@ -329,40 +379,38 @@
SourceLocation Begin = I->getBegin(), End = I->getEnd();
bool IsTokenRange = I->isTokenRange();
- FileID BeginFileID = SM->getFileID(Begin);
- FileID EndFileID = SM->getFileID(End);
-
- // Find the common parent for the beginning and end of the range.
+ // Compute the common parent; we can't highlight a range where
+ // the begin and end have different FileIDs.
+ findCommonParent(Begin, End, SM);
- // First, crawl the expansion chain for the beginning of the range.
- llvm::SmallDenseMap<FileID, SourceLocation> BeginLocsMap;
- while (Begin.isMacroID() && BeginFileID != EndFileID) {
- BeginLocsMap[BeginFileID] = Begin;
- Begin = SM->getImmediateExpansionRange(Begin).first;
- BeginFileID = SM->getFileID(Begin);
- }
-
- // Then, crawl the expansion chain for the end of the range.
- if (BeginFileID != EndFileID) {
- while (End.isMacroID() && !BeginLocsMap.count(EndFileID)) {
- End = SM->getImmediateExpansionRange(End).second;
- EndFileID = SM->getFileID(End);
- }
- if (End.isMacroID()) {
- Begin = BeginLocsMap[EndFileID];
- BeginFileID = EndFileID;
- }
- }
+ FileID BeginFileID = SM->getFileID(Begin);
while (Begin.isMacroID() && BeginFileID != CaretLocFileID) {
if (SM->isMacroArgExpansion(Begin)) {
+ // We have a macro argument; take the spelling loc, which is
+ // a step closer to where the argument was written.
Begin = SM->getImmediateSpellingLoc(Begin);
End = SM->getImmediateSpellingLoc(End);
+ BeginFileID = SM->getFileID(Begin);
+ assert(BeginFileID == SM->getFileID(End));
} else {
+ // Take the next expansion in the expansion chain.
Begin = SM->getImmediateExpansionRange(Begin).first;
End = SM->getImmediateExpansionRange(End).second;
+
+ // Compute the common parent again; the beginning and end might
+ // come out of different macro expansions.
+ findCommonParent(Begin, End, SM);
+ BeginFileID = SM->getFileID(Begin);
}
- BeginFileID = SM->getFileID(Begin);
+ }
+
+ // If this is the expansion of a macro argument, point the range at the
+ // use of the argument in the definition of the macro, not the expansion.
+ if (SM->isMacroArgExpansion(Begin)) {
+ assert(SM->isMacroArgExpansion(End));
+ Begin = End = SM->getImmediateExpansionRange(Begin).first;
+ IsTokenRange = true;
}
// Return the spelling location of the beginning and end of the range.
Modified: cfe/trunk/test/Misc/caret-diags-macros.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/caret-diags-macros.c?rev=170049&r1=170048&r2=170049&view=diff
==============================================================================
--- cfe/trunk/test/Misc/caret-diags-macros.c (original)
+++ cfe/trunk/test/Misc/caret-diags-macros.c Wed Dec 12 18:14:59 2012
@@ -181,10 +181,10 @@
}
// CHECK: {{.*}}:180:21: warning: expression result unused
// CHECK-NEXT: iequals(__LINE__, BARC(4,3,2,6,8), 8);
-// CHECK-NEXT: {{^ \^~~~~~~~~~~~~~~}}
+// CHECK-NEXT: {{^ \^ ~}}
// CHECK-NEXT: {{.*}}:179:51: note: expanded from macro 'BARC'
// CHECK-NEXT: #define /* */ BARC(c, /* */b, a, ...) (a+b+/* */c + __VA_ARGS__ +0)
-// CHECK-NEXT: {{^ ~~~~~~~~~~ \^}}
+// CHECK-NEXT: {{^ \^}}
#define APPEND2(NUM, SUFF) -1 != NUM ## SUFF
#define APPEND(NUM, SUFF) APPEND2(NUM, SUFF)
@@ -205,3 +205,24 @@
// CHECK-NEXT: {{.*}}:189:31: note: expanded from macro 'APPEND2'
// CHECK-NEXT: #define APPEND2(NUM, SUFF) -1 != NUM ## SUFF
// CHECK-NEXT: {{^ ~~ \^ ~~~~~~~~~~~}}
+
+
+unsigned long strlen_test(const char *s);
+#define __darwin_obsz(object) __builtin_object_size (object, 1)
+#define sprintf2(str, ...) \
+ __builtin___sprintf_chk (str, 0, __darwin_obsz(str), __VA_ARGS__)
+#define Cstrlen(a) strlen_test(a)
+#define Csprintf sprintf2
+void f(char* pMsgBuf, char* pKeepBuf) {
+Csprintf(pMsgBuf,"\nEnter minimum anagram length (2-%1d): ", Cstrlen(pKeepBuf));
+}
+// CHECK: {{.*}}:217: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: {{^ %1ld}}
+// CHECK-NEXT: {{.*}}:214:21: note: expanded from macro 'Cstrlen'
+// CHECK-NEXT: #define Cstrlen(a) strlen_test(a)
+// CHECK-NEXT: {{^ \^}}
+// CHECK-NEXT: {{.*}}:213:56: note: expanded from macro 'sprintf2'
+// CHECK-NEXT: __builtin___sprintf_chk (str, 0, __darwin_obsz(str), __VA_ARGS__)
+// CHECK-NEXT: {{^ \^}}
More information about the cfe-commits
mailing list