[lldb] Fix offset calculation when printing diagnostics in multiple ranges

[adrian-prantl]

depends on #112451

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Adrian Prantl (adrian-prantl)

Changes

depends on #112451

---

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

2 Files Affected:

• (modified) lldb/source/Utility/DiagnosticsRendering.cpp (+39-17)
• (modified) lldb/unittests/Utility/DiagnosticsRenderingTest.cpp (+45-2)

diff --git a/lldb/source/Utility/DiagnosticsRendering.cpp b/lldb/source/Utility/DiagnosticsRendering.cpp
index 96caf934cc2315..d28a9ab8958ba6 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,14 +94,39 @@ void RenderDiagnosticDetails(Stream &stream,
       continue;
     }
 
-    auto &loc = *detail.source_location;
     remaining_details.push_back(detail);
-    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;
+  }
+
+  // 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 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';
 
@@ -117,18 +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;
-      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 2bd80796b8074c..c91c39a59d8ef4 100644
--- a/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
+++ b/lldb/unittests/Utility/DiagnosticsRenderingTest.cpp
@@ -16,12 +16,55 @@ 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"));
+  }
+  {
+    DiagnosticDetail::SourceLocation loc1 = {FileSpec{"a.c"}, 1,   1, 3,
+                                             false,           true};
+    DiagnosticDetail::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");
+  }
 }