Skip to content

[lldb] Fix the sorting function for diagnostics #113220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

adrian-prantl
Copy link
Collaborator

No description provided.

@llvmbot
Copy link
Member

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/113220.diff

2 Files Affected:

  • (modified) lldb/source/Utility/DiagnosticsRendering.cpp (+2-2)
  • (modified) lldb/unittests/Utility/DiagnosticsRenderingTest.cpp (+7-3)
diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp b/lldb/source/Utility/DiagnosticsRendering.cpp
index d28a9ab8958ba6..208733ffc86853 100644
--- a/lldb/source/Utility/DiagnosticsRendering.cpp
+++ b/lldb/source/Utility/DiagnosticsRendering.cpp
@@ -99,10 +99,10 @@ void RenderDiagnosticDetails(Stream &stream,
 
   // Sort the diagnostics.
   auto sort = [](auto &ds) {
-    llvm::sort(ds.begin(), ds.end(), [](auto &d1, auto &d2) {
+    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{});
-      return std::pair(l1.line, l2.column) < std::pair(l1.line, l2.column);
+      return std::tie(l1.line, l1.column) < std::tie(l2.line, l2.column);
     });
   };
   sort(remaining_details);
diff --git a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
index 39d8b1d558420d..ad2ebf7ffe1e2f 100644
--- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
+++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
@@ -46,16 +46,20 @@ TEST_F(ErrorDisplayTest, RenderStatus) {
     std::string result =
         Render({DiagnosticDetail{loc2, eSeverityError, "X", "X"},
                 DiagnosticDetail{loc1, eSeverityError, "Y", "Y"}});
-    ASSERT_LT(StringRef(result).find("Y"), StringRef(result).find("X"));
+    // Unintuitively the later diagnostic appears first in the string:
+    //    ^   ^
+    //    |   second
+    //    first
+    ASSERT_GT(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 loc1 = {FileSpec{"a.c"}, 1, 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"));
+    ASSERT_GT(StringRef(result).find("Y"), StringRef(result).find("X"));
   }
   {
     // Test that range diagnostics are emitted correctly.

Copy link
Collaborator

@slackito slackito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@adrian-prantl adrian-prantl merged commit 74e1554 into llvm:main Oct 21, 2024
7 of 8 checks passed
adrian-prantl added a commit to adrian-prantl/llvm-project that referenced this pull request Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants