From 3703aadf78f6d3aafa23a1add5172dccff85c6c4 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 19 Nov 2024 08:58:49 -0800 Subject: [PATCH 1/3] [lldb] Fix a positioning bug in diagnostics output (#116711) The old code did not take the indentation into account. (cherry picked from commit 8b2dff960d9d987c583c3a6d5729f01d101dc401) --- lldb/source/Utility/DiagnosticsRendering.cpp | 4 ++-- .../Utility/DiagnosticsRenderingTest.cpp | 20 ++++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp b/lldb/source/Utility/DiagnosticsRendering.cpp index 208733ffc8685..dfc47ac460ac9 100644 --- a/lldb/source/Utility/DiagnosticsRendering.cpp +++ b/lldb/source/Utility/DiagnosticsRendering.cpp @@ -121,10 +121,10 @@ void RenderDiagnosticDetails(Stream &stream, continue; stream << std::string(loc.column - x_pos, ' ') << cursor; - ++x_pos; + x_pos = loc.column + 1; for (unsigned i = 0; i + 1 < loc.length; ++i) { stream << underline; - ++x_pos; + x_pos += 1; } } } diff --git a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp index ad2ebf7ffe1e2..1187f3f65f27f 100644 --- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp +++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp @@ -74,9 +74,27 @@ TEST_F(ErrorDisplayTest, RenderStatus) { auto line2 = lines.first; lines = lines.second.split('\n'); auto line3 = lines.first; - // 1234567 + // 1234567 ASSERT_EQ(line1, "^~~ ^~~"); ASSERT_EQ(line2, "| error: Y"); ASSERT_EQ(line3, "error: X"); } + { + // Test diagnostics on the same line are emitted correctly. + SourceLocation loc1 = {FileSpec{"a.c"}, 1, 2, 0, false, true}; + SourceLocation loc2 = {FileSpec{"a.c"}, 1, 6, 0, false, true}; + std::string result = + Render({DiagnosticDetail{loc1, eSeverityError, "X", "X"}, + DiagnosticDetail{loc2, eSeverityError, "Y", "Y"}}); + auto lines = StringRef(result).split('\n'); + auto line1 = lines.first; + lines = lines.second.split('\n'); + auto line2 = lines.first; + lines = lines.second.split('\n'); + auto line3 = lines.first; + // 1234567 + ASSERT_EQ(line1, " ^ ^"); + ASSERT_EQ(line2, " | error: Y"); + ASSERT_EQ(line3, " error: X"); + } } From 7e805509c6a822c3039fe254e0164f161ef0ba28 Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 19 Nov 2024 09:13:00 -0800 Subject: [PATCH 2/3] [lldb] Improve rendering of inline diagnostics on the same column (#116727) depends on https://github.com/llvm/llvm-project/pull/116711 [lldb] Improve rendering of inline diagnostics on the same column by fixing the indentation and printing these annotations in the original order. Before a+b+c; ^ ^ ^ | | error: 3 | |note: 2b | error: 2a error: 1 After a+b+c; ^ ^ ^ | | error: 3 | error: 2a | note: 2b error: 1 (cherry picked from commit 6b4f67545d87d5305cbbc20a678fb97ede995579) --- lldb/source/Utility/DiagnosticsRendering.cpp | 35 +++++++++++-- .../Utility/DiagnosticsRenderingTest.cpp | 49 +++++++++---------- 2 files changed, 54 insertions(+), 30 deletions(-) diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp b/lldb/source/Utility/DiagnosticsRendering.cpp index dfc47ac460ac9..a20d82ad4eb67 100644 --- a/lldb/source/Utility/DiagnosticsRendering.cpp +++ b/lldb/source/Utility/DiagnosticsRendering.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "lldb/Utility/DiagnosticsRendering.h" +#include using namespace lldb_private; using namespace lldb; @@ -98,7 +99,7 @@ void RenderDiagnosticDetails(Stream &stream, } // Sort the diagnostics. - auto sort = [](auto &ds) { + auto sort = [](std::vector &ds) { std::stable_sort(ds.begin(), ds.end(), [](auto &d1, auto &d2) { auto l1 = d1.source_location.value_or(DiagnosticDetail::SourceLocation{}); auto l2 = d2.source_location.value_or(DiagnosticDetail::SourceLocation{}); @@ -130,6 +131,25 @@ void RenderDiagnosticDetails(Stream &stream, } stream << '\n'; + // Reverse the order within groups of diagnostics that are on the same column. + auto group = [](const std::vector &details) { + uint16_t column = 0; + std::vector result, group; + for (auto &d : details) { + if (d.source_location->column == column) { + group.push_back(d); + continue; + } + result.insert(result.end(), group.rbegin(), group.rend()); + group.clear(); + column = d.source_location->column; + group.push_back(d); + } + result.insert(result.end(), group.rbegin(), group.rend()); + return result; + }; + remaining_details = group(remaining_details); + // Work through each detail in reverse order using the vector/stack. bool did_print = false; for (auto detail = remaining_details.rbegin(); @@ -142,14 +162,19 @@ void RenderDiagnosticDetails(Stream &stream, for (auto &remaining_detail : llvm::ArrayRef(remaining_details).drop_back(1)) { uint16_t column = remaining_detail.source_location->column; - if (x_pos <= column) + // Is this a note with the same column as another diagnostic? + if (column == detail->source_location->column) + continue; + + if (column >= x_pos) { stream << std::string(column - x_pos, ' ') << vbar; - x_pos = column + 1; + x_pos = column + 1; + } } - // Print the line connecting the ^ with the error message. uint16_t column = detail->source_location->column; - if (x_pos <= column) + // Print the line connecting the ^ with the error message. + if (column >= x_pos) stream << std::string(column - x_pos, ' ') << joint << hbar << spacer; // Print a colorized string based on the message's severity type. diff --git a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp index 1187f3f65f27f..4e5e0bb7dc355 100644 --- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp +++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp @@ -29,15 +29,22 @@ TEST_F(ErrorDisplayTest, RenderStatus) { { // Test that diagnostics on the same column can be handled and all // three errors are diagnosed. - SourceLocation loc1 = {FileSpec{"a.c"}, 13, 11, 0, false, true}; - SourceLocation loc2 = {FileSpec{"a.c"}, 13, 13, 0, false, true}; + SourceLocation loc1 = {FileSpec{"a.c"}, 13, 5, 0, false, true}; + SourceLocation loc2 = {FileSpec{"a.c"}, 13, 7, 0, false, true}; + SourceLocation loc3 = {FileSpec{"a.c"}, 13, 9, 0, false, true}; std::string result = Render({DiagnosticDetail{loc1, eSeverityError, "1", "1"}, - DiagnosticDetail{loc1, eSeverityError, "2", "2"}, - DiagnosticDetail{loc2, eSeverityError, "3", "3"}}); - ASSERT_TRUE(StringRef(result).contains("error: 1")); - ASSERT_TRUE(StringRef(result).contains("error: 2")); - ASSERT_TRUE(StringRef(result).contains("error: 3")); + DiagnosticDetail{loc2, eSeverityError, "2a", "2a"}, + DiagnosticDetail{loc2, eSeverityInfo, "2b", "2b"}, + DiagnosticDetail{loc3, eSeverityError, "3", "3"}}); + llvm::SmallVector lines; + StringRef(result).split(lines, '\n'); + // 1234567890123 + ASSERT_EQ(lines[0], " ^ ^ ^"); + ASSERT_EQ(lines[1], " | | error: 3"); + ASSERT_EQ(lines[2], " | error: 2a"); + ASSERT_EQ(lines[3], " | note: 2b"); + ASSERT_EQ(lines[4], " error: 1"); } { // Test that diagnostics in reverse order are emitted correctly. @@ -68,16 +75,12 @@ TEST_F(ErrorDisplayTest, RenderStatus) { std::string result = Render({DiagnosticDetail{loc1, eSeverityError, "X", "X"}, DiagnosticDetail{loc2, eSeverityError, "Y", "Y"}}); - auto lines = StringRef(result).split('\n'); - auto line1 = lines.first; - lines = lines.second.split('\n'); - auto line2 = lines.first; - lines = lines.second.split('\n'); - auto line3 = lines.first; + llvm::SmallVector lines; + StringRef(result).split(lines, '\n'); // 1234567 - ASSERT_EQ(line1, "^~~ ^~~"); - ASSERT_EQ(line2, "| error: Y"); - ASSERT_EQ(line3, "error: X"); + ASSERT_EQ(lines[0], "^~~ ^~~"); + ASSERT_EQ(lines[1], "| error: Y"); + ASSERT_EQ(lines[2], "error: X"); } { // Test diagnostics on the same line are emitted correctly. @@ -86,15 +89,11 @@ TEST_F(ErrorDisplayTest, RenderStatus) { std::string result = Render({DiagnosticDetail{loc1, eSeverityError, "X", "X"}, DiagnosticDetail{loc2, eSeverityError, "Y", "Y"}}); - auto lines = StringRef(result).split('\n'); - auto line1 = lines.first; - lines = lines.second.split('\n'); - auto line2 = lines.first; - lines = lines.second.split('\n'); - auto line3 = lines.first; + llvm::SmallVector lines; + StringRef(result).split(lines, '\n'); // 1234567 - ASSERT_EQ(line1, " ^ ^"); - ASSERT_EQ(line2, " | error: Y"); - ASSERT_EQ(line3, " error: X"); + ASSERT_EQ(lines[0], " ^ ^"); + ASSERT_EQ(lines[1], " | error: Y"); + ASSERT_EQ(lines[2], " error: X"); } } From a4b1912c7b55a333436e3c6e43c674743c4f0d4f Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Tue, 19 Nov 2024 13:02:47 -0800 Subject: [PATCH 3/3] [lldb] Refactor helper by using iterators and in-place edits (NFC) (#116876) Based on post-commit review feedback by Felipe Piovezan! (cherry picked from commit 174899f738b31216750ac59562475966b0b0be42) --- lldb/source/Utility/DiagnosticsRendering.cpp | 23 +++++++------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp b/lldb/source/Utility/DiagnosticsRendering.cpp index a20d82ad4eb67..f5aa27baadfef 100644 --- a/lldb/source/Utility/DiagnosticsRendering.cpp +++ b/lldb/source/Utility/DiagnosticsRendering.cpp @@ -132,23 +132,16 @@ void RenderDiagnosticDetails(Stream &stream, stream << '\n'; // Reverse the order within groups of diagnostics that are on the same column. - auto group = [](const std::vector &details) { - uint16_t column = 0; - std::vector result, group; - for (auto &d : details) { - if (d.source_location->column == column) { - group.push_back(d); - continue; - } - result.insert(result.end(), group.rbegin(), group.rend()); - group.clear(); - column = d.source_location->column; - group.push_back(d); + auto group = [](std::vector &details) { + for (auto it = details.begin(), end = details.end(); it != end;) { + auto eq_end = std::find_if(it, end, [&](const DiagnosticDetail &d) { + return d.source_location->column != it->source_location->column; + }); + std::reverse(it, eq_end); + it = eq_end; } - result.insert(result.end(), group.rbegin(), group.rend()); - return result; }; - remaining_details = group(remaining_details); + group(remaining_details); // Work through each detail in reverse order using the vector/stack. bool did_print = false;