Timm =?utf-8?q?Bäder?= <[email protected]>, Timm =?utf-8?q?Bäder?= <[email protected]> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/[email protected]>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/164941 >From a1bcecb7a604ce7c9cee9b4443e67a173328bed1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Thu, 30 Oct 2025 14:34:00 +0100 Subject: [PATCH 1/3] [clang] Adjust TextDiagnostic style ranges for interesting source region --- clang/lib/Frontend/TextDiagnostic.cpp | 70 ++++++++++++++----- ...diags-interesting-source-region-colors.cpp | 30 ++++++++ 2 files changed, 81 insertions(+), 19 deletions(-) create mode 100644 clang/test/Frontend/diags-interesting-source-region-colors.cpp diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index aea3e72d92a84..9d2ce9a9a389e 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -349,14 +349,13 @@ 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, - Columns NonGutterColumns, - const SourceColumnMap &Map) { - Columns CaretColumns = Columns(CaretLine.size()); - Columns FixItColumns = - Columns(llvm::sys::locale::columnWidth(FixItInsertionLine)); +static void selectInterestingSourceRegion( + std::string &SourceLine, std::string &CaretLine, + std::string &FixItInsertionLine, Columns NonGutterColumns, + const SourceColumnMap &Map, + SmallVector<clang::TextDiagnostic::StyleRange> &Styles) { + Columns CaretColumns = CaretLine.size(); + Columns FixItColumns = llvm::sys::locale::columnWidth(FixItInsertionLine); Columns MaxColumns = std::max({Map.columns().V, CaretColumns.V, FixItColumns.V}); // if the number of columns is less than the desired number we're done @@ -369,13 +368,11 @@ static void selectInterestingSourceRegion(std::string &SourceLine, // Find the slice that we need to display the full caret line // correctly. Columns CaretStart = 0, CaretEnd = CaretLine.size(); - for (; CaretStart != CaretEnd; CaretStart = CaretStart.next()) - if (!isWhitespace(CaretLine[CaretStart.V])) - break; + while (CaretStart != CaretEnd && isWhitespace(CaretLine[CaretStart.V])) + CaretStart = CaretStart.next(); - for (; CaretEnd != CaretStart; CaretEnd = CaretEnd.prev()) - if (!isWhitespace(CaretLine[CaretEnd.V - 1])) - break; + while (CaretEnd != CaretStart && isWhitespace(CaretLine[CaretEnd.V])) + CaretEnd = CaretEnd.prev(); // caret has already been inserted into CaretLine so the above whitespace // check is guaranteed to include the caret @@ -516,13 +513,42 @@ static void selectInterestingSourceRegion(std::string &SourceLine, assert(FrontColumnsRemoved + ColumnsKept + BackColumnsRemoved > NonGutterColumns); + // 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. + Bytes BytesRemoved = + FrontColumnsRemoved > FrontEllipse.size() + ? (Map.columnToByte(FrontColumnsRemoved) - Bytes(FrontEllipse.size())) + : 0; + Bytes CodeEnd = Map.columnToByte(CaretEnd); + for (auto &R : Styles) { + if (R.End < static_cast<unsigned>(BytesRemoved.V)) { + R.Start = R.End = std::numeric_limits<int>::max(); + continue; + } + // Move them left. (Note that this can wrap R.Start, but that doesn't + // matter). + R.Start -= BytesRemoved.V; + R.End -= BytesRemoved.V; + + // Don't leak into the ellipse at the end. + if (R.Start < static_cast<unsigned>(CodeEnd.V) && + R.End > static_cast<unsigned>(CodeEnd.V)) + R.End = CodeEnd.V; + + // Remove style ranges after the end of the snippet. + if (R.Start >= static_cast<unsigned>(CodeEnd.V)) + R.Start = R.End = std::numeric_limits<int>::max(); + } + // The line needs some truncation, and we'd prefer to keep the front // if possible, so remove the back if (BackColumnsRemoved > Columns(BackEllipse.size())) SourceLine.replace(SourceEnd.V, std::string::npos, BackEllipse); // If that's enough then we're done - if (FrontColumnsRemoved + ColumnsKept <= Columns(NonGutterColumns)) + if (FrontColumnsRemoved + ColumnsKept <= NonGutterColumns) return; // Otherwise remove the front as well @@ -1391,6 +1417,12 @@ void TextDiagnostic::emitSnippetAndCaret( OS.indent(MaxLineNoDisplayWidth + 2) << "| "; }; + Columns MessageLength = DiagOpts.MessageLength; + + // If we don't have enough columns available, just abort now. + if (MessageLength != 0 && MessageLength <= 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 = @@ -1450,10 +1482,10 @@ void TextDiagnostic::emitSnippetAndCaret( // If the source line is too long for our terminal, select only the // "interesting" source region within that line. - Columns MessageLength = DiagOpts.MessageLength; - if (MessageLength.V != 0) + if (MessageLength != 0) selectInterestingSourceRegion(SourceLine, CaretLine, FixItInsertionLine, - MessageLength, SourceColMap); + MessageLength, 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 @@ -1508,7 +1540,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; - size_t I = 0; + size_t I = 0; // Bytes. while (I < SourceLine.size()) { auto [Str, WasPrintable] = printableTextForNextCharacter(SourceLine, &I, DiagOpts.TabStop); diff --git a/clang/test/Frontend/diags-interesting-source-region-colors.cpp b/clang/test/Frontend/diags-interesting-source-region-colors.cpp new file mode 100644 index 0000000000000..412820a5710a9 --- /dev/null +++ b/clang/test/Frontend/diags-interesting-source-region-colors.cpp @@ -0,0 +1,30 @@ +// RUN: not %clang_cc1 %s -fmessage-length=40 -fcolor-diagnostics -fno-show-source-location -Wunused-value -o - 2>&1 | FileCheck %s + +// REQUIRES: ansi-escape-sequences + +int main() { + 1 + + if; + // CHECK: expected expression + // CHECK-NEXT: ...+ [[MAGENTA:.\[0;34m]]if[[RESET:.\[0m]]; + + /*😂*/1 + + if; + // CHECK: expected expression + // CHECK-NEXT: ...+ [[MAGENTA:.\[0;34m]]if[[RESET:.\[0m]]; + + a + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1; + // CHECK: use of undeclared identifier + // CHECK-NEXT: a + [[GREEN:.\[0;32m]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] ... + + + /*😂😂😂*/ a + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1 + 1; + // CHECK: use of undeclared identifier + // CHECK-NEXT: [[YELLOW:.\[0;33m]]/*😂😂😂*/[[RESET]] a + [[GREEN:.\[0;32m]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] + [[GREEN]]1[[RESET]] ... + + "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; + // CHECK: [[GREEN:.\[0;32m]]"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"[[RESET]]; + + "😂xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"; + // CHECK: [[GREEN:.\[0;32m]]"😂xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"[[RESET]]; +} + + >From c47ddb1f0bae8989be2294a8e249f88f77659012 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Fri, 31 Oct 2025 16:12:49 +0100 Subject: [PATCH 2/3] nits --- clang/lib/Frontend/TextDiagnostic.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index 9d2ce9a9a389e..9e297fa78f58a 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -353,7 +353,7 @@ static void selectInterestingSourceRegion( std::string &SourceLine, std::string &CaretLine, std::string &FixItInsertionLine, Columns NonGutterColumns, const SourceColumnMap &Map, - SmallVector<clang::TextDiagnostic::StyleRange> &Styles) { + SmallVectorImpl<clang::TextDiagnostic::StyleRange> &Styles) { Columns CaretColumns = CaretLine.size(); Columns FixItColumns = llvm::sys::locale::columnWidth(FixItInsertionLine); Columns MaxColumns = @@ -522,7 +522,7 @@ static void selectInterestingSourceRegion( ? (Map.columnToByte(FrontColumnsRemoved) - Bytes(FrontEllipse.size())) : 0; Bytes CodeEnd = Map.columnToByte(CaretEnd); - for (auto &R : Styles) { + for (TextDiagnostic::StyleRange &R : Styles) { if (R.End < static_cast<unsigned>(BytesRemoved.V)) { R.Start = R.End = std::numeric_limits<int>::max(); continue; >From 7d40bef9f8d1ff0ce03fd16a2fd6e3faf9f50785 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <[email protected]> Date: Sat, 1 Nov 2025 06:53:28 +0100 Subject: [PATCH 3/3] Fix yet another mishap --- clang/lib/Frontend/TextDiagnostic.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/TextDiagnostic.cpp b/clang/lib/Frontend/TextDiagnostic.cpp index 9e297fa78f58a..7013d859d6543 100644 --- a/clang/lib/Frontend/TextDiagnostic.cpp +++ b/clang/lib/Frontend/TextDiagnostic.cpp @@ -517,11 +517,16 @@ static void selectInterestingSourceRegion( // 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. + // Note in particular that variables like CaretEnd are defined in the + // CaretLine, which only contains ASCII, while the style ranges are defined in + // the source line, where we have to care for the byte-index != column-index + // case. Bytes BytesRemoved = FrontColumnsRemoved > FrontEllipse.size() ? (Map.columnToByte(FrontColumnsRemoved) - Bytes(FrontEllipse.size())) : 0; - Bytes CodeEnd = Map.columnToByte(CaretEnd); + Bytes CodeEnd = + CaretEnd < Map.columns() ? Map.columnToByte(CaretEnd.V) : CaretEnd.V; for (TextDiagnostic::StyleRange &R : Styles) { if (R.End < static_cast<unsigned>(BytesRemoved.V)) { R.Start = R.End = std::numeric_limits<int>::max(); @@ -535,7 +540,7 @@ static void selectInterestingSourceRegion( // Don't leak into the ellipse at the end. if (R.Start < static_cast<unsigned>(CodeEnd.V) && R.End > static_cast<unsigned>(CodeEnd.V)) - R.End = CodeEnd.V; + R.End = CodeEnd.V + 1; // R.End is inclusive. // Remove style ranges after the end of the snippet. if (R.Start >= static_cast<unsigned>(CodeEnd.V)) _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
