[Lldb-commits] [lldb] [lldb] Fix a crash when two diagnostics are on the same column or in … (PR #112451)

[Adrian Prantl via lldb-commits]

https://github.com/adrian-prantl updated https://github.com/llvm/llvm-project/pull/112451

>From d400a1358678162df02c7cada2a66b837d3ff005 Mon Sep 17 00:00:00 2001
From: Adrian Prantl <aprantl at apple.com>
Date: Tue, 15 Oct 2024 16:20:33 -0700
Subject: [PATCH] [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      | 28 ++++++++++++++--
 2 files changed, 51 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..e55b23368f0ea2 100644
--- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
+++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
@@ -16,12 +16,36 @@ std::string Render(std::vector<DiagnosticDetail> details) {
 } // namespace
 
 TEST_F(ErrorDisplayTest, RenderStatus) {
-  DiagnosticDetail::SourceLocation inline_loc;
-  inline_loc.in_user_input = true;
   {
+    DiagnosticDetail::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"));
   }
+
+  {
+    DiagnosticDetail::SourceLocation loc1 = {FileSpec{"a.c"}, 13,  11, 0,
+                                             false,           true};
+    DiagnosticDetail::SourceLocation loc2 = {FileSpec{"a.c"}, 13,  13, 0,
+                                             false,           true};
+    std::string result =
+        Render({DiagnosticDetail{loc1, eSeverityError, "1", "1"},
+                DiagnosticDetail{loc1, eSeverityError, "2", "3"},
+                DiagnosticDetail{loc2, eSeverityError, "3", "3"}});
+    ASSERT_TRUE(StringRef(result).contains("error: 1"));
+    ASSERT_TRUE(StringRef(result).contains("error: 3"));
+    ASSERT_TRUE(StringRef(result).contains("error: 2"));
+  }
+  {
+    DiagnosticDetail::SourceLocation loc1 = {FileSpec{"a.c"}, 1,   20, 0,
+                                             false,           true};
+    DiagnosticDetail::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"));
+  }
 }