https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/112466
>From 7c35b47aa81ee0d2151944eff59c159ea3fd4c5c Mon Sep 17 00:00:00 2001
From: Adrian Prantl <apra...@apple.com>
Date: Tue, 15 Oct 2024 16:20:33 -0700
Subject: [PATCH 1/2] [lldb] Fix a crash when two diagnostics are on the same
column or in reverse order
The second inner loop (only) was missing the check for
offset > column. Also this patch sorts the diagnostics before printing them.
---
lldb/source/Utility/DiagnosticsRendering.cpp | 32 ++++++++++++----
.../Utility/DiagnosticsRenderingTest.cpp | 37 ++++++++++++++++++-
2 files changed, 60 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp
b/lldb/source/Utility/DiagnosticsRendering.cpp
index 96caf934cc2315..dd059d6e98a63d 100644
--- a/lldb/source/Utility/DiagnosticsRendering.cpp
+++ b/lldb/source/Utility/DiagnosticsRendering.cpp
@@ -77,11 +77,7 @@ void RenderDiagnosticDetails(Stream &stream,
spacer = "";
}
- // Print a line with caret indicator(s) below the lldb prompt + command.
- const size_t padding = *offset_in_command;
- stream << std::string(padding, ' ');
-
- size_t offset = 1;
+ // Partition the diagnostics.
std::vector<DiagnosticDetail> remaining_details, other_details,
hidden_details;
for (const DiagnosticDetail &detail : details) {
@@ -98,10 +94,31 @@ void RenderDiagnosticDetails(Stream &stream,
continue;
}
- auto &loc = *detail.source_location;
remaining_details.push_back(detail);
+ }
+
+ // Sort the diagnostics.
+ auto sort = [](auto &ds) {
+ llvm::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{});
+ return std::pair(l1.line, l2.column) < std::pair(l1.line, l2.column);
+ });
+ };
+ sort(remaining_details);
+ sort(other_details);
+ sort(hidden_details);
+
+ // Print a line with caret indicator(s) below the lldb prompt + command.
+ const size_t padding = *offset_in_command;
+ stream << std::string(padding, ' ');
+ size_t offset = 1;
+ for (const DiagnosticDetail &detail : remaining_details) {
+ auto &loc = *detail.source_location;
+
if (offset > loc.column)
continue;
+
stream << std::string(loc.column - offset, ' ') << cursor;
for (unsigned i = 0; i + 1 < loc.length; ++i)
stream << underline;
@@ -121,7 +138,8 @@ void RenderDiagnosticDetails(Stream &stream,
for (auto &remaining_detail :
llvm::ArrayRef(remaining_details).drop_back(1)) {
uint16_t column = remaining_detail.source_location->column;
- stream << std::string(column - offset, ' ') << vbar;
+ if (offset <= column)
+ stream << std::string(column - offset, ' ') << vbar;
offset = column + 1;
}
diff --git a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
index 2bd80796b8074c..3d37403331b4c8 100644
--- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
+++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
@@ -16,12 +16,45 @@ std::string Render(std::vector<DiagnosticDetail> details) {
} // namespace
TEST_F(ErrorDisplayTest, RenderStatus) {
- DiagnosticDetail::SourceLocation inline_loc;
- inline_loc.in_user_input = true;
+ using SourceLocation = DiagnosticDetail::SourceLocation;
{
+ SourceLocation inline_loc;
+ inline_loc.in_user_input = true;
std::string result =
Render({DiagnosticDetail{inline_loc, eSeverityError, "foo", ""}});
ASSERT_TRUE(StringRef(result).contains("error:"));
ASSERT_TRUE(StringRef(result).contains("foo"));
}
+
+ {
+ // 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};
+ 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"));
+ }
+ {
+ // Test that diagnostics in reverse order are emitted correctly.
+ SourceLocation loc1 = {FileSpec{"a.c"}, 1, 20, 0, false, true};
+ SourceLocation loc2 = {FileSpec{"a.c"}, 2, 10, 0, false, true};
+ std::string result =
+ Render({DiagnosticDetail{loc2, eSeverityError, "X", "X"},
+ DiagnosticDetail{loc1, eSeverityError, "Y", "Y"}});
+ ASSERT_LT(StringRef(result).find("Y"), StringRef(result).find("X"));
+ }
+ {
+ // Test that diagnostics in reverse order are emitted correctly.
+ SourceLocation loc1 = {FileSpec{"a.c"}, 2, 10, 0, false, true};
+ SourceLocation loc2 = {FileSpec{"a.c"}, 1, 20, 0, false, true};
+ std::string result =
+ Render({DiagnosticDetail{loc2, eSeverityError, "X", "X"},
+ DiagnosticDetail{loc1, eSeverityError, "Y", "Y"}});
+ ASSERT_LT(StringRef(result).find("Y"), StringRef(result).find("X"));
+ }
}
>From cf2a43adbda2ea121dc333c1821bc8066146f2d3 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <apra...@apple.com>
Date: Tue, 15 Oct 2024 20:15:12 -0700
Subject: [PATCH 2/2] [lldb] Fix offset calculation when printing diagnostics
in multiple ranges
---
lldb/source/Utility/DiagnosticsRendering.cpp | 38 ++++++++++---------
.../Utility/DiagnosticsRenderingTest.cpp | 18 +++++++++
2 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp
b/lldb/source/Utility/DiagnosticsRendering.cpp
index dd059d6e98a63d..d28a9ab8958ba6 100644
--- a/lldb/source/Utility/DiagnosticsRendering.cpp
+++ b/lldb/source/Utility/DiagnosticsRendering.cpp
@@ -112,17 +112,21 @@ void RenderDiagnosticDetails(Stream &stream,
// Print a line with caret indicator(s) below the lldb prompt + command.
const size_t padding = *offset_in_command;
stream << std::string(padding, ' ');
- size_t offset = 1;
- for (const DiagnosticDetail &detail : remaining_details) {
- auto &loc = *detail.source_location;
-
- if (offset > loc.column)
- continue;
-
- stream << std::string(loc.column - offset, ' ') << cursor;
- for (unsigned i = 0; i + 1 < loc.length; ++i)
- stream << underline;
- offset = loc.column + 1;
+ {
+ size_t x_pos = 1;
+ for (const DiagnosticDetail &detail : remaining_details) {
+ auto &loc = *detail.source_location;
+
+ if (x_pos > loc.column)
+ continue;
+
+ stream << std::string(loc.column - x_pos, ' ') << cursor;
+ ++x_pos;
+ for (unsigned i = 0; i + 1 < loc.length; ++i) {
+ stream << underline;
+ ++x_pos;
+ }
+ }
}
stream << '\n';
@@ -134,19 +138,19 @@ void RenderDiagnosticDetails(Stream &stream,
// Get the information to print this detail and remove it from the stack.
// Print all the lines for all the other messages first.
stream << std::string(padding, ' ');
- size_t offset = 1;
+ size_t x_pos = 1;
for (auto &remaining_detail :
llvm::ArrayRef(remaining_details).drop_back(1)) {
uint16_t column = remaining_detail.source_location->column;
- if (offset <= column)
- stream << std::string(column - offset, ' ') << vbar;
- offset = column + 1;
+ if (x_pos <= column)
+ stream << std::string(column - x_pos, ' ') << vbar;
+ x_pos = column + 1;
}
// Print the line connecting the ^ with the error message.
uint16_t column = detail->source_location->column;
- if (offset <= column)
- stream << std::string(column - offset, ' ') << joint << hbar << spacer;
+ if (x_pos <= column)
+ stream << std::string(column - x_pos, ' ') << joint << hbar << spacer;
// Print a colorized string based on the message's severity type.
PrintSeverity(stream, detail->severity);
diff --git a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
index 3d37403331b4c8..39d8b1d558420d 100644
--- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
+++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
@@ -57,4 +57,22 @@ TEST_F(ErrorDisplayTest, RenderStatus) {
DiagnosticDetail{loc1, eSeverityError, "Y", "Y"}});
ASSERT_LT(StringRef(result).find("Y"), StringRef(result).find("X"));
}
+ {
+ // Test that range diagnostics are emitted correctly.
+ SourceLocation loc1 = {FileSpec{"a.c"}, 1, 1, 3, false, true};
+ SourceLocation loc2 = {FileSpec{"a.c"}, 1, 5, 3, 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");
+ }
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
