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

…reverse order

The second inner loop (only) was missing the check for offset > column.
Also this patch sorts the diagnostics before printing them.

Author: adrian-prantl

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;
     }
 

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"));
+  }
 }