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

[Jorge Gorbe Moya via lldb-commits]

================
@@ -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);
----------------
slackito wrote:

Btw, I found that bug because this commit is causing flakiness in the test. So the bad comparison is producing the expected results, sometimes.

Some more data I gathered poking around to hopefully help debug. If I change the comparator to
```
  return std::pair(l1.line, l1.column) < std::pair(l2.line, l2.column)
```
I still get (non-flaky) test failures. I think at least one of these test cases is wrong:
```
    // 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"));
  }
```
In the first one, we have "X" in `loc2` (line 2, col 10), and "Y" in `loc1` (line 1, col 20) , so they are in reverse order.
In the second one, we have "X" is in `loc2` (line 1, col 20), and "Y" in `loc1` (line 2, col 10), so despite the comment they are not in reverse order if lines are supposed to be compared first.

https://github.com/llvm/llvm-project/pull/112451