[cfe-commits] r70597 - in /cfe/trunk: include/clang/Frontend/TextDiagnosticPrinter.h lib/Frontend/TextDiagnosticPrinter.cpp test/Misc/message-length.c
Douglas Gregor
dgregor at apple.com
Fri May 1 16:32:58 PDT 2009
Author: dgregor
Date: Fri May 1 18:32:58 2009
New Revision: 70597
URL: http://llvm.org/viewvc/llvm-project?rev=70597&view=rev
Log:
When printing a source line as part of a diagnostic, the source line
might be wider than we're supposed to print. In this case, we try to
select the "important" subregion of the source line, which contains
everything that we want to show (e.g., with underlining and the caret
itself) and tries to also contain some of the context.
>From the fantastically long line in the test case, we get an error
message that slices down to this:
message-length.c:18:120: warning: comparison of distinct pointer types
('int *' and 'float *')
a_func_to_call(ip == FloatPointer, ip[ALongIndexName],
~~ ^ ~~~~~~~~~~~~
There are a bunch of gee-it-sounds-good heuristics in here, which seem
to do well on the various simple tests I've thrown at it. However,
we're going to need to look at a bunch more diagnostics to tweak these
heuristics.
This is the second part of <rdar://problem/6711348>. Almost there!
Modified:
cfe/trunk/include/clang/Frontend/TextDiagnosticPrinter.h
cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
cfe/trunk/test/Misc/message-length.c
Modified: cfe/trunk/include/clang/Frontend/TextDiagnosticPrinter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/TextDiagnosticPrinter.h?rev=70597&r1=70596&r2=70597&view=diff
==============================================================================
--- cfe/trunk/include/clang/Frontend/TextDiagnosticPrinter.h (original)
+++ cfe/trunk/include/clang/Frontend/TextDiagnosticPrinter.h Fri May 1 18:32:58 2009
@@ -74,7 +74,8 @@
SourceManager &SM,
const CodeModificationHint *Hints,
unsigned NumHints,
- unsigned AvoidColumn);
+ unsigned AvoidColumn,
+ unsigned Columns);
virtual void HandleDiagnostic(Diagnostic::Level DiagLevel,
const DiagnosticInfo &Info);
Modified: cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp?rev=70597&r1=70596&r2=70597&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp (original)
+++ cfe/trunk/lib/Frontend/TextDiagnosticPrinter.cpp Fri May 1 18:32:58 2009
@@ -108,20 +108,158 @@
CaretLine[i] = '~';
}
+/// \brief Whether this is a closing delimiter such as ')' or ']'.
+static inline bool isClosingDelimiter(char c) {
+ return c == ')' || c == ']' || c == '}';
+}
+
+/// \brief When the source code line we want to print is too long for
+/// the terminal, select the "interesting" region.
+static void SelectInterestingSourceRegion(std::string &SourceLine,
+ std::string &CaretLine,
+ std::string &FixItInsertionLine,
+ unsigned Columns) {
+ if (CaretLine.size() > SourceLine.size())
+ SourceLine.resize(CaretLine.size(), ' ');
+
+ // Find the slice that we need to display the full caret line
+ // correctly.
+ unsigned CaretStart = 0, CaretEnd = CaretLine.size();
+ for (; CaretStart != CaretEnd; ++CaretStart)
+ if (!isspace(CaretLine[CaretStart]))
+ break;
+
+ for (; CaretEnd != CaretStart; --CaretEnd)
+ if (!isspace(CaretLine[CaretEnd - 1]))
+ break;
+
+ // CaretLine[CaretStart, CaretEnd) contains all of the interesting
+ // parts of the caret line. While this slice is smaller than the
+ // number of columns we have, try to grow the slice to encompass
+ // more context.
+
+ // If the end of the interesting region comes before we run out of
+ // space in the terminal, start at the beginning of the line.
+ if (CaretEnd < Columns)
+ CaretStart = 0;
+
+ unsigned TargetColumns = Columns - 4; // Give us a little extra room.
+ unsigned SourceLength = SourceLine.size();
+ bool StartIsFixed = false;
+ while (CaretEnd - CaretStart < TargetColumns) {
+ bool ExpandedRegion = false;
+ // Move the start of the interesting region left until we've
+ // pulled in something else interesting.
+ if (CaretStart && !StartIsFixed &&
+ CaretEnd - CaretStart < TargetColumns) {
+ unsigned NewStart = CaretStart;
+
+ bool BadStart = false;
+ do {
+ // Skip over any whitespace we see here; we're looking for
+ // another bit of interesting text.
+ if (NewStart)
+ --NewStart;
+ while (NewStart && isspace(SourceLine[NewStart]))
+ --NewStart;
+
+ // Skip over this bit of "interesting" text.
+ while (NewStart && !isspace(SourceLine[NewStart])) {
+ if (isClosingDelimiter(SourceLine[NewStart]))
+ StartIsFixed = true;
+ --NewStart;
+ }
+
+ // Move up to the non-whitespace character we just saw.
+ if (!StartIsFixed &&
+ isspace(SourceLine[NewStart]) &&
+ !isspace(SourceLine[NewStart + 1]))
+ ++NewStart;
+
+ // Never go back past closing delimeters, because
+ // they're unlikely to be important (and they result in
+ // weird slices). Instead, move forward to the next
+ // non-whitespace character.
+ BadStart = false;
+ if (StartIsFixed) {
+ ++NewStart;
+ while (NewStart != CaretEnd && isspace(SourceLine[NewStart]))
+ ++NewStart;
+ } else if (NewStart) {
+ // There are some characters that always signal that we've
+ // found a bad stopping place, because they always occur in
+ // the middle of or at the end of an expression. In these
+ // cases, we either keep bringing in more "interesting" text
+ // to try to get to a somewhat-complete slice of the code.
+ BadStart = ispunct(SourceLine[NewStart]);
+ }
+ } while (BadStart);
+
+ // If we're still within our limit, update the starting
+ // position within the source/caret line.
+ if (CaretEnd - NewStart <= TargetColumns && !StartIsFixed) {
+ CaretStart = NewStart;
+ ExpandedRegion = true;
+ }
+ }
+
+ // Move the end of the interesting region right until we've
+ // pulled in something else interesting.
+ if (CaretEnd != SourceLength &&
+ CaretEnd - CaretStart < TargetColumns) {
+ unsigned NewEnd = CaretEnd;
+
+ // Skip over any whitespace we see here; we're looking for
+ // another bit of interesting text.
+ while (CaretEnd != SourceLength && isspace(SourceLine[NewEnd - 1]))
+ ++NewEnd;
+
+ // Skip over this bit of "interesting" text.
+ while (CaretEnd != SourceLength && !isspace(SourceLine[NewEnd - 1]))
+ ++NewEnd;
+
+ if (NewEnd - CaretStart <= TargetColumns) {
+ CaretEnd = NewEnd;
+ ExpandedRegion = true;
+ }
+
+ if (!ExpandedRegion)
+ break;
+ }
+ }
+
+ // [CaretStart, CaretEnd) is the slice we want. Update the various
+ // output lines to show only this slice, with two-space padding
+ // before the lines so that it looks nicer.
+ SourceLine.erase(CaretEnd, std::string::npos);
+ CaretLine.erase(CaretEnd, std::string::npos);
+ if (FixItInsertionLine.size() > CaretEnd)
+ FixItInsertionLine.erase(CaretEnd, std::string::npos);
+
+ if (CaretStart > 2) {
+ SourceLine.replace(0, CaretStart, " ");
+ CaretLine.replace(0, CaretStart, " ");
+ if (FixItInsertionLine.size() >= CaretStart)
+ FixItInsertionLine.replace(0, CaretStart, " ");
+ }
+}
+
void TextDiagnosticPrinter::EmitCaretDiagnostic(SourceLocation Loc,
SourceRange *Ranges,
unsigned NumRanges,
SourceManager &SM,
const CodeModificationHint *Hints,
unsigned NumHints,
- unsigned AvoidColumn) {
+ unsigned AvoidColumn,
+ unsigned Columns) {
assert(!Loc.isInvalid() && "must have a valid source location here");
// We always emit diagnostics about the instantiation points, not the spelling
// points. This more closely correlates to what the user writes.
if (!Loc.isFileID()) {
SourceLocation OneLevelUp = SM.getImmediateInstantiationRange(Loc).first;
- EmitCaretDiagnostic(OneLevelUp, Ranges, NumRanges, SM, 0, 0, AvoidColumn);
+ EmitCaretDiagnostic(OneLevelUp, Ranges, NumRanges, SM, 0, 0, AvoidColumn,
+ Columns);
// Map the location through the macro.
Loc = SM.getInstantiationLoc(SM.getImmediateSpellingLoc(Loc));
@@ -210,10 +348,6 @@
CaretLine.insert(i+1, NumSpaces, CaretLine[i] == '~' ? '~' : ' ');
}
- // Finally, remove any blank spaces from the end of CaretLine.
- while (CaretLine[CaretLine.size()-1] == ' ')
- CaretLine.erase(CaretLine.end()-1);
-
// If we are in -fdiagnostics-print-source-range-info mode, we are trying to
// produce easily machine parsable output. Add a space before the source line
// and the caret to make it trivial to tell the main diagnostic line from what
@@ -222,27 +356,9 @@
SourceLine = ' ' + SourceLine;
CaretLine = ' ' + CaretLine;
}
-
- // AvoidColumn tells us which column we should avoid when printing
- // the source line. If the source line would start at or near that
- // column, add another line of whitespace before printing the source
- // line. Otherwise, the source line and the diagnostic text can get
- // jumbled together.
- unsigned StartCol = 0;
- for (unsigned N = SourceLine.size(); StartCol != N; ++StartCol)
- if (!isspace(SourceLine[StartCol]))
- break;
-
- if (StartCol != SourceLine.size() &&
- abs((int)StartCol - (int)AvoidColumn) <= 2)
- OS << '\n';
-
- // Emit what we have computed.
- OS << SourceLine << '\n';
- OS << CaretLine << '\n';
-
+
+ std::string FixItInsertionLine;
if (NumHints && PrintFixItInfo) {
- std::string InsertionLine;
for (const CodeModificationHint *Hint = Hints, *LastHint = Hints + NumHints;
Hint != LastHint; ++Hint) {
if (Hint->InsertionLoc.isValid()) {
@@ -258,19 +374,47 @@
= SM.getColumnNumber(HintLocInfo.first, HintLocInfo.second);
unsigned LastColumnModified
= HintColNo - 1 + Hint->CodeToInsert.size();
- if (LastColumnModified > InsertionLine.size())
- InsertionLine.resize(LastColumnModified, ' ');
+ if (LastColumnModified > FixItInsertionLine.size())
+ FixItInsertionLine.resize(LastColumnModified, ' ');
std::copy(Hint->CodeToInsert.begin(), Hint->CodeToInsert.end(),
- InsertionLine.begin() + HintColNo - 1);
+ FixItInsertionLine.begin() + HintColNo - 1);
}
}
}
+ }
- if (!InsertionLine.empty()) {
- if (PrintRangeInfo)
- OS << ' ';
- OS << InsertionLine << '\n';
- }
+ // If the source line is too long for our terminal, select only the
+ // "interesting" source region within that line.
+ if (Columns && SourceLine.size() > Columns)
+ SelectInterestingSourceRegion(SourceLine, CaretLine, FixItInsertionLine,
+ Columns);
+
+ // AvoidColumn tells us which column we should avoid when printing
+ // the source line. If the source line would start at or near that
+ // column, add another line of whitespace before printing the source
+ // line. Otherwise, the source line and the diagnostic text can get
+ // jumbled together.
+ unsigned StartCol = 0;
+ for (unsigned N = SourceLine.size(); StartCol != N; ++StartCol)
+ if (!isspace(SourceLine[StartCol]))
+ break;
+
+ if (StartCol != SourceLine.size() &&
+ abs((int)StartCol - (int)AvoidColumn) <= 2)
+ OS << '\n';
+
+ // Finally, remove any blank spaces from the end of CaretLine.
+ while (CaretLine[CaretLine.size()-1] == ' ')
+ CaretLine.erase(CaretLine.end()-1);
+
+ // Emit what we have computed.
+ OS << SourceLine << '\n';
+ OS << CaretLine << '\n';
+
+ if (!FixItInsertionLine.empty()) {
+ if (PrintRangeInfo)
+ OS << ' ';
+ OS << FixItInsertionLine << '\n';
}
}
@@ -572,7 +716,8 @@
EmitCaretDiagnostic(LastLoc, Ranges, NumRanges, LastLoc.getManager(),
Info.getCodeModificationHints(),
Info.getNumCodeModificationHints(),
- WordWrapped? WordWrapIndentation : 0);
+ WordWrapped? WordWrapIndentation : 0,
+ MessageLength);
}
OS.flush();
Modified: cfe/trunk/test/Misc/message-length.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/message-length.c?rev=70597&r1=70596&r2=70597&view=diff
==============================================================================
--- cfe/trunk/test/Misc/message-length.c (original)
+++ cfe/trunk/test/Misc/message-length.c Fri May 1 18:32:58 2009
@@ -11,3 +11,12 @@
int (*fp2)(int, float, short, float) = f;
}
+
+void a_func_to_call(int, int, int);
+
+void a_very_long_line(int *ip, float *FloatPointer) {
+ for (int ALongIndexName = 0; ALongIndexName < 100; ALongIndexName++) if (ip[ALongIndexName] == 17) a_func_to_call(ip == FloatPointer, ip[ALongIndexName], FloatPointer[ALongIndexName]);
+
+
+ int array0[] = { [3] 3, 5, 7, 4, 2, 7, 6, 3, 4, 5, 6, 7, 8, 9, 12, 345, 14, 345, 789, 234, 678, 345, 123, 765, 234 };
+}
More information about the cfe-commits
mailing list