-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[clang] Adjust TextDiagnostic style ranges for interesting source region #164941
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesNeed to merge #164935 first. After: Full diff: https://github.com/llvm/llvm-project/pull/164941.diff 4 Files Affected:
diff --git a/clang/include/clang/Frontend/TextDiagnostic.h b/clang/include/clang/Frontend/TextDiagnostic.h
index e2e88d4d648a2..5ea55641e0720 100644
--- a/clang/include/clang/Frontend/TextDiagnostic.h
+++ b/clang/include/clang/Frontend/TextDiagnostic.h
@@ -16,10 +16,12 @@
#define LLVM_CLANG_FRONTEND_TEXTDIAGNOSTIC_H
#include "clang/Frontend/DiagnosticRenderer.h"
-#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/FormattedStream.h"
namespace clang {
+using formatted_raw_ostream = llvm::formatted_raw_ostream;
+
/// Class to encapsulate the logic for formatting and printing a textual
/// diagnostic message.
///
@@ -33,11 +35,11 @@ namespace clang {
/// DiagnosticClient is implemented through this class as is diagnostic
/// printing coming out of libclang.
class TextDiagnostic : public DiagnosticRenderer {
- raw_ostream &OS;
+ formatted_raw_ostream &OS;
const Preprocessor *PP;
public:
- TextDiagnostic(raw_ostream &OS, const LangOptions &LangOpts,
+ TextDiagnostic(formatted_raw_ostream &OS, const LangOptions &LangOpts,
DiagnosticOptions &DiagOpts, const Preprocessor *PP = nullptr);
~TextDiagnostic() override;
@@ -46,22 +48,23 @@ class TextDiagnostic : public DiagnosticRenderer {
unsigned Start;
unsigned End;
enum llvm::raw_ostream::Colors Color;
- StyleRange(unsigned S, unsigned E, enum llvm::raw_ostream::Colors C)
- : Start(S), End(E), Color(C){};
+ StyleRange(unsigned S, unsigned E,
+ enum llvm::formatted_raw_ostream::Colors C)
+ : Start(S), End(E), Color(C) {};
};
- /// Print the diagonstic level to a raw_ostream.
+ /// Print the diagonstic level to a formatted_raw_ostream.
///
/// This is a static helper that handles colorizing the level and formatting
/// it into an arbitrary output stream. This is used internally by the
/// TextDiagnostic emission code, but it can also be used directly by
/// consumers that don't have a source manager or other state that the full
/// TextDiagnostic logic requires.
- static void printDiagnosticLevel(raw_ostream &OS,
+ static void printDiagnosticLevel(formatted_raw_ostream &OS,
DiagnosticsEngine::Level Level,
bool ShowColors);
- /// Pretty-print a diagnostic message to a raw_ostream.
+ /// Pretty-print a diagnostic message to a formatted_raw_ostream.
///
/// This is a static helper to handle the line wrapping, colorizing, and
/// rendering of a diagnostic message to a particular ostream. It is
@@ -77,9 +80,10 @@ class TextDiagnostic : public DiagnosticRenderer {
/// \param Columns The number of columns to use in line-wrapping, 0 disables
/// all line-wrapping.
/// \param ShowColors Enable colorizing of the message.
- static void printDiagnosticMessage(raw_ostream &OS, bool IsSupplemental,
- StringRef Message, unsigned CurrentColumn,
- unsigned Columns, bool ShowColors);
+ static void printDiagnosticMessage(formatted_raw_ostream &OS,
+ bool IsSupplemental, StringRef Message,
+ unsigned CurrentColumn, unsigned Columns,
+ bool ShowColors);
protected:
void emitDiagnosticMessage(FullSourceLoc Loc, PresumedLoc PLoc,
diff --git a/clang/include/clang/Frontend/TextDiagnosticPrinter.h b/clang/include/clang/Frontend/TextDiagnosticPrinter.h
index dd1ca6b2248b7..d7d892f2b80e7 100644
--- a/clang/include/clang/Frontend/TextDiagnosticPrinter.h
+++ b/clang/include/clang/Frontend/TextDiagnosticPrinter.h
@@ -17,6 +17,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/LLVM.h"
#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/Support/FormattedStream.h"
#include <memory>
namespace clang {
@@ -25,7 +26,7 @@ class LangOptions;
class TextDiagnostic;
class TextDiagnosticPrinter : public DiagnosticConsumer {
- raw_ostream &OS;
+ llvm::formatted_raw_ostream OS;
DiagnosticOptions &DiagOpts;
/// Handle to the currently active text diagnostic emitter.
diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp
index 58885712fbdcc..94abaabc8868f 100644
--- a/clang/lib/Frontend/TextDiagnostic.cpp
+++ b/clang/lib/Frontend/TextDiagnostic.cpp
@@ -17,7 +17,6 @@
#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/Locale.h"
-#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <optional>
@@ -49,7 +48,7 @@ static constexpr raw_ostream::Colors LiteralColor = raw_ostream::GREEN;
static constexpr raw_ostream::Colors KeywordColor = raw_ostream::BLUE;
/// Add highlights to differences in template strings.
-static void applyTemplateHighlighting(raw_ostream &OS, StringRef Str,
+static void applyTemplateHighlighting(formatted_raw_ostream &OS, StringRef Str,
bool &Normal, bool Bold) {
while (true) {
size_t Pos = Str.find(ToggleHighlight);
@@ -59,11 +58,11 @@ static void applyTemplateHighlighting(raw_ostream &OS, StringRef Str,
Str = Str.substr(Pos + 1);
if (Normal)
- OS.changeColor(templateColor, true);
+ OS.changeColor(templateColor, true, false);
else {
OS.resetColor();
if (Bold)
- OS.changeColor(savedColor, true);
+ OS.changeColor(savedColor, true, false);
}
Normal = !Normal;
}
@@ -317,11 +316,11 @@ struct SourceColumnMap {
/// 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,
- const SourceColumnMap &map) {
+static void selectInterestingSourceRegion(
+ std::string &SourceLine, std::string &CaretLine,
+ std::string &FixItInsertionLine, unsigned Columns,
+ const SourceColumnMap &map,
+ SmallVector<clang::TextDiagnostic::StyleRange> &Styles) {
unsigned CaretColumns = CaretLine.size();
unsigned FixItColumns = llvm::sys::locale::columnWidth(FixItInsertionLine);
unsigned MaxColumns = std::max(static_cast<unsigned>(map.columns()),
@@ -410,10 +409,10 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
if (TargetColumns > ellipses_space+CaretColumnsOutsideSource)
TargetColumns -= ellipses_space+CaretColumnsOutsideSource;
- while (SourceStart>0 || SourceEnd<SourceLine.size()) {
+ while (SourceStart > 0 || SourceEnd < SourceLine.size()) {
bool ExpandedRegion = false;
- if (SourceStart>0) {
+ if (SourceStart > 0) {
unsigned NewStart = map.startOfPreviousColumn(SourceStart);
// Skip over any whitespace we see here; we're looking for
@@ -488,8 +487,41 @@ static void selectInterestingSourceRegion(std::string &SourceLine,
if (BackColumnsRemoved > strlen(back_ellipse))
SourceLine.replace(SourceEnd, std::string::npos, back_ellipse);
+ // Since we've modified the SourceLine, we also need to adjust the line's
+ // highlighting information. In particular, if we've removed
+ // from the front of the line, we need to move the style ranges to the
+ // left and remove unneeded ranges.
+ unsigned FrontDiff = FrontColumnsRemoved > strlen(front_ellipse)
+ ? FrontColumnsRemoved - strlen(front_ellipse)
+ : 0;
+ unsigned CodeStart =
+ FrontColumnsRemoved > strlen(front_ellipse) ? strlen(front_ellipse) : 0;
+ unsigned CodeEnd = CodeStart + ColumnsKept;
+ for (auto &R : Styles) {
+ // Style ranges too far left. Just move them where they don't bother.
+ if (R.End < FrontDiff) {
+ R.Start = R.End = std::numeric_limits<unsigned>::max();
+ continue;
+ }
+ // Move them left. (Note that this can wrap R.Start, but that doesn't
+ // matter).
+ R.Start -= FrontDiff;
+ R.End -= FrontDiff;
+
+ // Style ranges too far to the right.
+ if (R.Start >= (ColumnsKept + strlen(front_ellipse))) {
+ R.Start = R.End = std::numeric_limits<unsigned>::max();
+ continue;
+ }
+
+ // If the range overlaps the end of the code, don't leak into the back
+ // ellipse.
+ if (R.Start < CodeEnd && R.End > CodeEnd)
+ R.End = CodeEnd;
+ }
+
// If that's enough then we're done
- if (FrontColumnsRemoved+ColumnsKept <= Columns)
+ if (FrontColumnsRemoved + ColumnsKept <= Columns)
return;
// Otherwise remove the front as well
@@ -603,8 +635,8 @@ static unsigned findEndOfWord(unsigned Start, StringRef Str,
/// \param Bold if the current text should be bold
/// \returns true if word-wrapping was required, or false if the
/// string fit on the first line.
-static bool printWordWrapped(raw_ostream &OS, StringRef Str, unsigned Columns,
- unsigned Column, bool Bold) {
+static bool printWordWrapped(formatted_raw_ostream &OS, StringRef Str,
+ unsigned Columns, unsigned Column, bool Bold) {
const unsigned Length = std::min(Str.find('\n'), Str.size());
bool TextNormal = true;
@@ -651,7 +683,8 @@ static bool printWordWrapped(raw_ostream &OS, StringRef Str, unsigned Columns,
return Wrapped;
}
-TextDiagnostic::TextDiagnostic(raw_ostream &OS, const LangOptions &LangOpts,
+TextDiagnostic::TextDiagnostic(formatted_raw_ostream &OS,
+ const LangOptions &LangOpts,
DiagnosticOptions &DiagOpts,
const Preprocessor *PP)
: DiagnosticRenderer(LangOpts, DiagOpts), OS(OS), PP(PP) {}
@@ -662,7 +695,7 @@ void TextDiagnostic::emitDiagnosticMessage(
FullSourceLoc Loc, PresumedLoc PLoc, DiagnosticsEngine::Level Level,
StringRef Message, ArrayRef<clang::CharSourceRange> Ranges,
DiagOrStoredDiag D) {
- uint64_t StartOfLocationInfo = OS.tell();
+ uint64_t StartOfLocationInfo = OS.getColumn();
// Emit the location of this particular diagnostic.
if (Loc.isValid())
@@ -675,12 +708,12 @@ void TextDiagnostic::emitDiagnosticMessage(
printDiagnosticLevel(OS, Level, DiagOpts.ShowColors);
printDiagnosticMessage(OS,
/*IsSupplemental*/ Level == DiagnosticsEngine::Note,
- Message, OS.tell() - StartOfLocationInfo,
+ Message, OS.getColumn() - StartOfLocationInfo,
DiagOpts.MessageLength, DiagOpts.ShowColors);
}
/*static*/ void
-TextDiagnostic::printDiagnosticLevel(raw_ostream &OS,
+TextDiagnostic::printDiagnosticLevel(formatted_raw_ostream &OS,
DiagnosticsEngine::Level Level,
bool ShowColors) {
if (ShowColors) {
@@ -688,11 +721,21 @@ TextDiagnostic::printDiagnosticLevel(raw_ostream &OS,
switch (Level) {
case DiagnosticsEngine::Ignored:
llvm_unreachable("Invalid diagnostic type");
- case DiagnosticsEngine::Note: OS.changeColor(noteColor, true); break;
- case DiagnosticsEngine::Remark: OS.changeColor(remarkColor, true); break;
- case DiagnosticsEngine::Warning: OS.changeColor(warningColor, true); break;
- case DiagnosticsEngine::Error: OS.changeColor(errorColor, true); break;
- case DiagnosticsEngine::Fatal: OS.changeColor(fatalColor, true); break;
+ case DiagnosticsEngine::Note:
+ OS.changeColor(noteColor, true, false);
+ break;
+ case DiagnosticsEngine::Remark:
+ OS.changeColor(remarkColor, true, false);
+ break;
+ case DiagnosticsEngine::Warning:
+ OS.changeColor(warningColor, true, false);
+ break;
+ case DiagnosticsEngine::Error:
+ OS.changeColor(errorColor, true, false);
+ break;
+ case DiagnosticsEngine::Fatal:
+ OS.changeColor(fatalColor, true, false);
+ break;
}
}
@@ -711,7 +754,7 @@ TextDiagnostic::printDiagnosticLevel(raw_ostream &OS,
}
/*static*/
-void TextDiagnostic::printDiagnosticMessage(raw_ostream &OS,
+void TextDiagnostic::printDiagnosticMessage(formatted_raw_ostream &OS,
bool IsSupplemental,
StringRef Message,
unsigned CurrentColumn,
@@ -720,7 +763,7 @@ void TextDiagnostic::printDiagnosticMessage(raw_ostream &OS,
if (ShowColors && !IsSupplemental) {
// Print primary diagnostic messages in bold and without color, to visually
// indicate the transition from continuation notes and other output.
- OS.changeColor(savedColor, true);
+ OS.changeColor(savedColor, true, false);
Bold = true;
}
@@ -798,7 +841,7 @@ void TextDiagnostic::emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc,
return;
if (DiagOpts.ShowColors)
- OS.changeColor(savedColor, true);
+ OS.changeColor(savedColor, true, false);
emitFilename(PLoc.getFilename(), Loc.getManager());
switch (DiagOpts.getFormat()) {
@@ -1345,6 +1388,11 @@ void TextDiagnostic::emitSnippetAndCaret(
OS.indent(MaxLineNoDisplayWidth + 2) << "| ";
};
+ unsigned Columns = DiagOpts.MessageLength;
+ // If we don't have enough columns available, just abort now.
+ if (Columns && Columns <= (MaxLineNoDisplayWidth + 4))
+ return;
+
// Prepare source highlighting information for the lines we're about to
// emit, starting from the first line.
std::unique_ptr<SmallVector<StyleRange>[]> SourceStyles =
@@ -1402,10 +1450,11 @@ void TextDiagnostic::emitSnippetAndCaret(
// 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);
+ (Columns - (MaxLineNoDisplayWidth + 4)),
+ sourceColMap,
+ SourceStyles[LineNo - Lines.first]);
// If we are in -fdiagnostics-print-source-range-info mode, we are trying
// to produce easily machine parsable output. Add a space before the
@@ -1423,7 +1472,7 @@ void TextDiagnostic::emitSnippetAndCaret(
if (!CaretLine.empty()) {
indentForLineNumbers();
if (DiagOpts.ShowColors)
- OS.changeColor(caretColor, true);
+ OS.changeColor(caretColor, true, false);
OS << CaretLine << '\n';
if (DiagOpts.ShowColors)
OS.resetColor();
@@ -1433,7 +1482,7 @@ void TextDiagnostic::emitSnippetAndCaret(
indentForLineNumbers();
if (DiagOpts.ShowColors)
// Print fixit line in color
- OS.changeColor(fixitColor, false);
+ OS.changeColor(fixitColor, false, false);
if (DiagOpts.ShowSourceRanges)
OS << ' ';
OS << FixItInsertionLine << '\n';
@@ -1459,7 +1508,7 @@ void TextDiagnostic::emitSnippet(StringRef SourceLine,
// Print the source line one character at a time.
bool PrintReversed = false;
- std::optional<llvm::raw_ostream::Colors> CurrentColor;
+ std::optional<llvm::formatted_raw_ostream::Colors> CurrentColor;
size_t I = 0;
while (I < SourceLine.size()) {
auto [Str, WasPrintable] =
@@ -1485,7 +1534,7 @@ void TextDiagnostic::emitSnippet(StringRef SourceLine,
if (CharStyle != Styles.end()) {
if (!CurrentColor ||
(CurrentColor && *CurrentColor != CharStyle->Color)) {
- OS.changeColor(CharStyle->Color, false);
+ OS.changeColor(CharStyle->Color, false, false);
CurrentColor = CharStyle->Color;
}
} else if (CurrentColor) {
diff --git a/clang/lib/Frontend/TextDiagnosticPrinter.cpp b/clang/lib/Frontend/TextDiagnosticPrinter.cpp
index 83fd70e5f99f9..a4165b2593ccc 100644
--- a/clang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/clang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -16,6 +16,7 @@
#include "clang/Frontend/TextDiagnostic.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormattedStream.h"
#include "llvm/Support/raw_ostream.h"
using namespace clang;
|
25195df to
9220801
Compare
9220801 to
a1bcecb
Compare
After:


