Add option to include multiple lines in snippets.

When a diagnostic includes a highlighted range spanning multiple lines, clang
now supports printing out multiple lines of context if necessary to show the
highlighted ranges. This is not yet exposed in the driver, but can be enabled
by "-Xclang -fcaret-diagnostics-max-lines -Xclang N".

This is experimental until we can find out whether it works well in practice,
and if so, what a good default for the maximum number of lines is.

llvm-svn: 303589
This commit is contained in:
Richard Smith 2017-05-22 23:51:40 +00:00
parent 81559593a3
commit 0c7d4d7e21
6 changed files with 401 additions and 85 deletions

View File

@ -87,6 +87,8 @@ VALUE_DIAGOPT(TemplateBacktraceLimit, 32, DefaultTemplateBacktraceLimit)
VALUE_DIAGOPT(ConstexprBacktraceLimit, 32, DefaultConstexprBacktraceLimit)
/// Limit number of times to perform spell checking.
VALUE_DIAGOPT(SpellCheckingLimit, 32, DefaultSpellCheckingLimit)
/// Limit number of lines shown in a snippet.
VALUE_DIAGOPT(SnippetLineLimit, 32, DefaultSnippetLineLimit)
VALUE_DIAGOPT(TabStop, 32, DefaultTabStop) /// The distance between tab stops.
/// Column limit for formatting message diagnostics, or 0 if unused.

View File

@ -63,11 +63,15 @@ public:
enum TextDiagnosticFormat { Clang, MSVC, Vi };
// Default values.
enum { DefaultTabStop = 8, MaxTabStop = 100,
enum {
DefaultTabStop = 8,
MaxTabStop = 100,
DefaultMacroBacktraceLimit = 6,
DefaultTemplateBacktraceLimit = 10,
DefaultConstexprBacktraceLimit = 10,
DefaultSpellCheckingLimit = 50 };
DefaultSpellCheckingLimit = 50,
DefaultSnippetLineLimit = 1,
};
// Define simple diagnostic options (with no accessors).
#define DIAGOPT(Name, Bits, Default) unsigned Name : Bits;

View File

@ -359,6 +359,9 @@ def fconstexpr_backtrace_limit : Separate<["-"], "fconstexpr-backtrace-limit">,
HelpText<"Set the maximum number of entries to print in a constexpr evaluation backtrace (0 = no limit).">;
def fspell_checking_limit : Separate<["-"], "fspell-checking-limit">, MetaVarName<"<N>">,
HelpText<"Set the maximum number of times to perform spell checking on unrecognized identifiers (0 = no limit).">;
def fcaret_diagnostics_max_lines :
Separate<["-"], "fcaret-diagnostics-max-lines">, MetaVarName<"<N>">,
HelpText<"Set the maximum number of source lines to show in a caret diagnostic">;
def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"<N>">,
HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">;
def verify : Flag<["-"], "verify">,

View File

@ -1087,6 +1087,9 @@ bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
Opts.SpellCheckingLimit = getLastArgIntValue(
Args, OPT_fspell_checking_limit,
DiagnosticOptions::DefaultSpellCheckingLimit, Diags);
Opts.SnippetLineLimit = getLastArgIntValue(
Args, OPT_fcaret_diagnostics_max_lines,
DiagnosticOptions::DefaultSnippetLineLimit, Diags);
Opts.TabStop = getLastArgIntValue(Args, OPT_ftabstop,
DiagnosticOptions::DefaultTabStop, Diags);
if (Opts.TabStop == 0 || Opts.TabStop > DiagnosticOptions::MaxTabStop) {

View File

@ -928,6 +928,56 @@ void TextDiagnostic::emitBuildingModuleLocation(SourceLocation Loc,
OS << "While building module '" << ModuleName << "':\n";
}
/// \brief Find the suitable set of lines to show to include a set of ranges.
static llvm::Optional<std::pair<unsigned, unsigned>>
findLinesForRange(const CharSourceRange &R, FileID FID,
const SourceManager &SM) {
if (!R.isValid()) return None;
SourceLocation Begin = R.getBegin();
SourceLocation End = R.getEnd();
if (SM.getFileID(Begin) != FID || SM.getFileID(End) != FID)
return None;
return std::make_pair(SM.getExpansionLineNumber(Begin),
SM.getExpansionLineNumber(End));
}
/// Add as much of range B into range A as possible without exceeding a maximum
/// size of MaxRange. Ranges are inclusive.
std::pair<unsigned, unsigned> maybeAddRange(std::pair<unsigned, unsigned> A,
std::pair<unsigned, unsigned> B,
unsigned MaxRange) {
// If A is already the maximum size, we're done.
unsigned Slack = MaxRange - (A.second - A.first + 1);
if (Slack == 0)
return A;
// Easy case: merge succeeds within MaxRange.
unsigned Min = std::min(A.first, B.first);
unsigned Max = std::max(A.second, B.second);
if (Max - Min + 1 <= MaxRange)
return {Min, Max};
// If we can't reach B from A within MaxRange, there's nothing to do.
// Don't add lines to the range that contain nothing interesting.
if ((B.first > A.first && B.first - A.first + 1 > MaxRange) ||
(B.second < A.second && A.second - B.second + 1 > MaxRange))
return A;
// Otherwise, expand A towards B to produce a range of size MaxRange. We
// attempt to expand by the same amount in both directions if B strictly
// contains A.
// Expand downwards by up to half the available amount, then upwards as
// much as possible, then downwards as much as possible.
A.second = std::min(A.second + (Slack + 1) / 2, Max);
Slack = MaxRange - (A.second - A.first + 1);
A.first = std::max(Min + Slack, A.first) - Slack;
A.second = std::min(A.first + MaxRange - 1, Max);
return A;
}
/// \brief Highlight a SourceRange (with ~'s) for any characters on LineNo.
static void highlightRange(const CharSourceRange &R,
unsigned LineNo, FileID FID,
@ -990,9 +1040,12 @@ static void highlightRange(const CharSourceRange &R,
EndColNo = map.startOfPreviousColumn(EndColNo);
// If the start/end passed each other, then we are trying to highlight a
// range that just exists in whitespace, which must be some sort of other
// bug.
assert(StartColNo <= EndColNo && "Trying to highlight whitespace??");
// range that just exists in whitespace. That most likely means we have
// a multi-line highlighting range that covers a blank line.
if (StartColNo > EndColNo) {
assert(StartLineNo != EndLineNo && "trying to highlight whitespace");
StartColNo = EndColNo;
}
}
assert(StartColNo <= map.getSourceLine().size() && "Invalid range!");
@ -1103,7 +1156,7 @@ void TextDiagnostic::emitSnippetAndCaret(
// Decompose the location into a FID/Offset pair.
std::pair<FileID, unsigned> LocInfo = SM.getDecomposedLoc(Loc);
FileID FID = LocInfo.first;
unsigned FileOffset = LocInfo.second;
unsigned CaretFileOffset = LocInfo.second;
// Get information about the buffer it points into.
bool Invalid = false;
@ -1111,101 +1164,118 @@ void TextDiagnostic::emitSnippetAndCaret(
if (Invalid)
return;
const char *BufStart = BufData.data();
const char *BufEnd = BufStart + BufData.size();
unsigned CaretLineNo = SM.getLineNumber(FID, CaretFileOffset);
unsigned CaretColNo = SM.getColumnNumber(FID, CaretFileOffset);
unsigned LineNo = SM.getLineNumber(FID, FileOffset);
unsigned ColNo = SM.getColumnNumber(FID, FileOffset);
// Arbitrarily stop showing snippets when the line is too long.
static const size_t MaxLineLengthToPrint = 4096;
if (ColNo > MaxLineLengthToPrint)
if (CaretColNo > MaxLineLengthToPrint)
return;
// Rewind from the current position to the start of the line.
const char *TokPtr = BufStart+FileOffset;
const char *LineStart = TokPtr-ColNo+1; // Column # is 1-based.
// Compute the line end. Scan forward from the error position to the end of
// the line.
const char *LineEnd = TokPtr;
while (*LineEnd != '\n' && *LineEnd != '\r' && LineEnd != BufEnd)
++LineEnd;
// Arbitrarily stop showing snippets when the line is too long.
if (size_t(LineEnd - LineStart) > MaxLineLengthToPrint)
return;
// Trim trailing null-bytes.
StringRef Line(LineStart, LineEnd - LineStart);
while (Line.size() > ColNo && Line.back() == '\0')
Line = Line.drop_back();
// Copy the line of code into an std::string for ease of manipulation.
std::string SourceLine(Line.begin(), Line.end());
// Build the byte to column map.
const SourceColumnMap sourceColMap(SourceLine, DiagOpts->TabStop);
// Create a line for the caret that is filled with spaces that is the same
// number of columns as the line of source code.
std::string CaretLine(sourceColMap.columns(), ' ');
// Highlight all of the characters covered by Ranges with ~ characters.
// Find the set of lines to include.
const unsigned MaxLines = DiagOpts->SnippetLineLimit;
std::pair<unsigned, unsigned> Lines = {CaretLineNo, CaretLineNo};
for (SmallVectorImpl<CharSourceRange>::iterator I = Ranges.begin(),
E = Ranges.end();
I != E; ++I)
highlightRange(*I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
if (auto OptionalRange = findLinesForRange(*I, FID, SM))
Lines = maybeAddRange(Lines, *OptionalRange, MaxLines);
// Next, insert the caret itself.
ColNo = sourceColMap.byteToContainingColumn(ColNo-1);
if (CaretLine.size()<ColNo+1)
CaretLine.resize(ColNo+1, ' ');
CaretLine[ColNo] = '^';
for (unsigned LineNo = Lines.first; LineNo != Lines.second + 1; ++LineNo) {
const char *BufStart = BufData.data();
const char *BufEnd = BufStart + BufData.size();
std::string FixItInsertionLine = buildFixItInsertionLine(LineNo,
sourceColMap,
Hints, SM,
DiagOpts.get());
// Rewind from the current position to the start of the line.
const char *LineStart =
BufStart +
SM.getDecomposedLoc(SM.translateLineCol(FID, LineNo, 1)).second;
if (LineStart == BufEnd)
break;
// If the source line is too long for our terminal, select only the
// "interesting" source region within that line.
unsigned Columns = DiagOpts->MessageLength;
if (Columns)
selectInterestingSourceRegion(SourceLine, CaretLine, FixItInsertionLine,
Columns, sourceColMap);
// Compute the line end.
const char *LineEnd = LineStart;
while (*LineEnd != '\n' && *LineEnd != '\r' && LineEnd != BufEnd)
++LineEnd;
// 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 the user is intended to see.
if (DiagOpts->ShowSourceRanges) {
SourceLine = ' ' + SourceLine;
CaretLine = ' ' + CaretLine;
}
// Arbitrarily stop showing snippets when the line is too long.
// FIXME: Don't print any lines in this case.
if (size_t(LineEnd - LineStart) > MaxLineLengthToPrint)
return;
// Finally, remove any blank spaces from the end of CaretLine.
while (CaretLine[CaretLine.size()-1] == ' ')
CaretLine.erase(CaretLine.end()-1);
// Trim trailing null-bytes.
StringRef Line(LineStart, LineEnd - LineStart);
while (!Line.empty() && Line.back() == '\0' &&
(LineNo != CaretLineNo || Line.size() > CaretColNo))
Line = Line.drop_back();
// Emit what we have computed.
emitSnippet(SourceLine);
// Copy the line of code into an std::string for ease of manipulation.
std::string SourceLine(Line.begin(), Line.end());
if (DiagOpts->ShowColors)
OS.changeColor(caretColor, true);
OS << CaretLine << '\n';
if (DiagOpts->ShowColors)
OS.resetColor();
// Build the byte to column map.
const SourceColumnMap sourceColMap(SourceLine, DiagOpts->TabStop);
if (!FixItInsertionLine.empty()) {
if (DiagOpts->ShowColors)
// Print fixit line in color
OS.changeColor(fixitColor, false);
if (DiagOpts->ShowSourceRanges)
OS << ' ';
OS << FixItInsertionLine << '\n';
if (DiagOpts->ShowColors)
OS.resetColor();
// Create a line for the caret that is filled with spaces that is the same
// number of columns as the line of source code.
std::string CaretLine(sourceColMap.columns(), ' ');
// Highlight all of the characters covered by Ranges with ~ characters.
for (SmallVectorImpl<CharSourceRange>::iterator I = Ranges.begin(),
E = Ranges.end();
I != E; ++I)
highlightRange(*I, LineNo, FID, sourceColMap, CaretLine, SM, LangOpts);
// Next, insert the caret itself.
if (CaretLineNo == LineNo) {
CaretColNo = sourceColMap.byteToContainingColumn(CaretColNo - 1);
if (CaretLine.size() < CaretColNo + 1)
CaretLine.resize(CaretColNo + 1, ' ');
CaretLine[CaretColNo] = '^';
}
std::string FixItInsertionLine = buildFixItInsertionLine(
LineNo, sourceColMap, Hints, SM, DiagOpts.get());
// If the source line is too long for our terminal, select only the
// "interesting" source region within that line.
unsigned Columns = DiagOpts->MessageLength;
if (Columns)
selectInterestingSourceRegion(SourceLine, CaretLine, FixItInsertionLine,
Columns, sourceColMap);
// 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 the user is intended to see.
if (DiagOpts->ShowSourceRanges) {
SourceLine = ' ' + SourceLine;
CaretLine = ' ' + CaretLine;
}
// 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.
emitSnippet(SourceLine);
if (!CaretLine.empty()) {
if (DiagOpts->ShowColors)
OS.changeColor(caretColor, true);
OS << CaretLine << '\n';
if (DiagOpts->ShowColors)
OS.resetColor();
}
if (!FixItInsertionLine.empty()) {
if (DiagOpts->ShowColors)
// Print fixit line in color
OS.changeColor(fixitColor, false);
if (DiagOpts->ShowSourceRanges)
OS << ' ';
OS << FixItInsertionLine << '\n';
if (DiagOpts->ShowColors)
OS.resetColor();
}
}
// Print out any parseable fixit information requested by the options.

View File

@ -0,0 +1,234 @@
// RUN: not %clang_cc1 -std=c++11 -fcaret-diagnostics-max-lines 5 -Wsometimes-uninitialized %s 2>&1 | FileCheck %s --strict-whitespace
void line(int);
// Check we expand the range as much as possible within the limit.
// CHECK: warning: variable 'a' is used uninitialized whenever 'if' condition is true
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~{{$}}
// CHECK-NEXT: note: uninitialized use occurs here
// CHECK-NEXT: {{^}} return a;
// CHECK-NEXT: {{^}} ^
// CHECK-NEXT: note: remove the 'if' if its condition is always false
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(1);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} } else {
// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
// CHECK-NEXT: note: initialize the variable
int f1(int cond) {
int a;
if (cond) {
line(1);
} else {
a = 3;
}
return a;
}
// CHECK: warning: variable 'a' is used uninitialized whenever 'if' condition is true
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~{{$}}
// CHECK-NEXT: note: uninitialized use occurs here
// CHECK-NEXT: {{^}} return a;
// CHECK-NEXT: {{^}} ^
// CHECK-NEXT: note: remove the 'if' if its condition is always false
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(1);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(2);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} } else {
// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
// CHECK-NEXT: note: initialize the variable
int f2(int cond) {
int a;
if (cond) {
line(1);
line(2);
} else {
a = 3;
}
return a;
}
// CHECK: warning: variable 'a' is used uninitialized whenever 'if' condition is true
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~{{$}}
// CHECK-NEXT: note: uninitialized use occurs here
// CHECK-NEXT: {{^}} return a;
// CHECK-NEXT: {{^}} ^
// CHECK-NEXT: note: remove the 'if' if its condition is always false
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(1);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(2);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(3);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} } else {
// CHECK-NEXT: {{^}}~~~~~~~~~{{$}}
// CHECK-NEXT: note: initialize the variable
int f3(int cond) {
int a;
if (cond) {
line(1);
line(2);
line(3);
} else {
a = 3;
}
return a;
}
// CHECK: warning: variable 'a' is used uninitialized whenever 'if' condition is true
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~{{$}}
// CHECK-NEXT: note: uninitialized use occurs here
// CHECK-NEXT: {{^}} return a;
// CHECK-NEXT: {{^}} ^
// CHECK-NEXT: note: remove the 'if' if its condition is always false
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(1);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(2);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(3);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(4);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: note: initialize the variable
int f4(int cond) {
int a;
if (cond) {
line(1);
line(2);
line(3);
line(4);
} else {
a = 3;
}
return a;
}
// CHECK: warning: variable 'a' is used uninitialized whenever 'if' condition is true
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~{{$}}
// CHECK-NEXT: note: uninitialized use occurs here
// CHECK-NEXT: {{^}} return a;
// CHECK-NEXT: {{^}} ^
// CHECK-NEXT: note: remove the 'if' if its condition is always false
// CHECK-NEXT: {{^}} if (cond) {
// CHECK-NEXT: {{^}} ^~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(1);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(2);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(3);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: {{^}} line(4);
// CHECK-NEXT: {{^}}~~~~~~~~~~~~{{$}}
// CHECK-NEXT: note: initialize the variable
int f5(int cond) {
int a;
if (cond) {
line(1);
line(2);
line(3);
line(4);
line(5);
} else {
a = 3;
}
return a;
}
// Check that we don't include lines with no interesting code if we can't reach
// the interesting part within the line limit.
// CHECK: error: no matching function for call to 'g
// CHECK: note: candidate template ignored: substitution failure
// CHECK-NEXT: {{^}}decltype(T()
// CHECK-NEXT: {{^}} ~{{$}}
// CHECK-NEXT: {{^}} + 1
// CHECK-NEXT: {{^}} + 2
// CHECK-NEXT: {{^}} + 3
// CHECK-NEXT: {{^}}void g();
// CHECK-NEXT: {{^}} ^{{$}}
template<typename T>
decltype(T()
+ 1
+ 2
+ 3)
void g();
// CHECK: note: candidate template ignored: substitution failure
// CHECK-NEXT: {{^}}void g();
// CHECK-NEXT: {{^}} ^{{$}}
template<typename T>
decltype(T()
+ 1
+ 2
+ 3
+ 4)
void g();
void h() { g<int()>(); }
void multiple_ranges(int a, int b) {
// CHECK: error: invalid operands
// CHECK-NEXT: &(a)
// CHECK-NEXT: ~~~~
// CHECK-NEXT: +
// CHECK-NEXT: ^
// CHECK-NEXT: &(b)
// CHECK-NEXT: ~~~~
&(a)
+
&(b);
// CHECK-NEXT: error: invalid operands
// CHECK-NEXT: &(
// CHECK-NEXT: ~~
// CHECK-NEXT: a
// CHECK-NEXT: ~
// CHECK-NEXT: )
// CHECK-NEXT: ~
// CHECK-NEXT: +
// CHECK-NEXT: ^
// CHECK-NEXT: &(
// CHECK-NEXT: ~~
&(
a
)
+
&(
b
);
// CHECK-NEXT: error: invalid operands
// CHECK-NEXT: &(a
// CHECK-NEXT: ~~
// CHECK-NEXT: )
// CHECK-NEXT: ~
// CHECK-NEXT: +
// CHECK-NEXT: ^
// CHECK-NEXT: &(
// CHECK-NEXT: ~~
// CHECK-NEXT: b
// CHECK-NEXT: ~
&(a
)
+
&(
b
);
}