diff --git a/clang/include/clang/Basic/DiagnosticOptions.def b/clang/include/clang/Basic/DiagnosticOptions.def index 0ab6724ed9ef..2467b24fd909 100644 --- a/clang/include/clang/Basic/DiagnosticOptions.def +++ b/clang/include/clang/Basic/DiagnosticOptions.def @@ -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. diff --git a/clang/include/clang/Basic/DiagnosticOptions.h b/clang/include/clang/Basic/DiagnosticOptions.h index c9b0c5def992..c195003de5c4 100644 --- a/clang/include/clang/Basic/DiagnosticOptions.h +++ b/clang/include/clang/Basic/DiagnosticOptions.h @@ -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; diff --git a/clang/include/clang/Driver/CC1Options.td b/clang/include/clang/Driver/CC1Options.td index bd2062d967b4..33b0632e5d67 100644 --- a/clang/include/clang/Driver/CC1Options.td +++ b/clang/include/clang/Driver/CC1Options.td @@ -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<"">, 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<"">, + HelpText<"Set the maximum number of source lines to show in a caret diagnostic">; def fmessage_length : Separate<["-"], "fmessage-length">, MetaVarName<"">, HelpText<"Format message diagnostics so that they fit within N columns or fewer, when possible.">; def verify : Flag<["-"], "verify">, diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 51147b6f9499..29cdcd7df1d2 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -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) { diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index a4937386b93f..a18b8c888c26 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -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> +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 maybeAddRange(std::pair A, + std::pair 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 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 Lines = {CaretLineNo, CaretLineNo}; for (SmallVectorImpl::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()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::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. diff --git a/clang/test/Misc/caret-diags-multiline.cpp b/clang/test/Misc/caret-diags-multiline.cpp new file mode 100644 index 000000000000..4826d9beaa3f --- /dev/null +++ b/clang/test/Misc/caret-diags-multiline.cpp @@ -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 +decltype(T() + + 1 + + 2 + + 3) +void g(); + +// CHECK: note: candidate template ignored: substitution failure +// CHECK-NEXT: {{^}}void g(); +// CHECK-NEXT: {{^}} ^{{$}} +template +decltype(T() + + 1 + + 2 + + 3 + + 4) +void g(); + +void h() { g(); } + + +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 + ); +}