Merge pull request #9630 from adrian-prantl/cherry-pick-stable-20240723-lldb-Fix-a-positioning-bug-in-diagnostics-output-116711

[Cherry-pick into stable/20240723] [lldb] Fix a positioning bug in diagnostics output (llvm#116711)

Author: adrian-prantl

lldb/source/Utility/DiagnosticsRendering.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Utility/DiagnosticsRendering.h"
+#include <cstdint>
 
 using namespace lldb_private;
 using namespace lldb;
@@ -98,7 +99,7 @@ void RenderDiagnosticDetails(Stream &stream,
   }
 
   // Sort the diagnostics.
-  auto sort = [](auto &ds) {
+  auto sort = [](std::vector<DiagnosticDetail> &ds) {
     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{});
@@ -121,15 +122,27 @@ void RenderDiagnosticDetails(Stream &stream,
         continue;
 
       stream << std::string(loc.column - x_pos, ' ') << cursor;
-      ++x_pos;
+      x_pos = loc.column + 1;
       for (unsigned i = 0; i + 1 < loc.length; ++i) {
         stream << underline;
-        ++x_pos;
+        x_pos += 1;
       }
     }
   }
   stream << '\n';
 
+  // Reverse the order within groups of diagnostics that are on the same column.
+  auto group = [](std::vector<DiagnosticDetail> &details) {
+    for (auto it = details.begin(), end = details.end(); it != end;) {
+      auto eq_end = std::find_if(it, end, [&](const DiagnosticDetail &d) {
+        return d.source_location->column != it->source_location->column;
+      });
+      std::reverse(it, eq_end);
+      it = eq_end;
+    }
+  };
+  group(remaining_details);
+
   // Work through each detail in reverse order using the vector/stack.
   bool did_print = false;
   for (auto detail = remaining_details.rbegin();
@@ -142,14 +155,19 @@ void RenderDiagnosticDetails(Stream &stream,
     for (auto &remaining_detail :
          llvm::ArrayRef(remaining_details).drop_back(1)) {
       uint16_t column = remaining_detail.source_location->column;
-      if (x_pos <= column)
+      // Is this a note with the same column as another diagnostic?
+      if (column == detail->source_location->column)
+        continue;
+
+      if (column >= x_pos) {
         stream << std::string(column - x_pos, ' ') << vbar;
-      x_pos = column + 1;
+        x_pos = column + 1;
+      }
     }
 
-    // Print the line connecting the ^ with the error message.
     uint16_t column = detail->source_location->column;
-    if (x_pos <= column)
+    // Print the line connecting the ^ with the error message.
+    if (column >= x_pos)
       stream << std::string(column - x_pos, ' ') << joint << hbar << spacer;
 
     // Print a colorized string based on the message's severity type.

lldb/unittests/Utility/DiagnosticsRenderingTest.cpp

@@ -29,15 +29,22 @@ TEST_F(ErrorDisplayTest, RenderStatus) {
   {
     // 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};
+    SourceLocation loc1 = {FileSpec{"a.c"}, 13, 5, 0, false, true};
+    SourceLocation loc2 = {FileSpec{"a.c"}, 13, 7, 0, false, true};
+    SourceLocation loc3 = {FileSpec{"a.c"}, 13, 9, 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"));
+                DiagnosticDetail{loc2, eSeverityError, "2a", "2a"},
+                DiagnosticDetail{loc2, eSeverityInfo, "2b", "2b"},
+                DiagnosticDetail{loc3, eSeverityError, "3", "3"}});
+    llvm::SmallVector<StringRef> lines;
+    StringRef(result).split(lines, '\n');
+    //                1234567890123
+    ASSERT_EQ(lines[0], "    ^ ^ ^");
+    ASSERT_EQ(lines[1], "    | | error: 3");
+    ASSERT_EQ(lines[2], "    | error: 2a");
+    ASSERT_EQ(lines[3], "    | note: 2b");
+    ASSERT_EQ(lines[4], "    error: 1");
   }
   {
     // Test that diagnostics in reverse order are emitted correctly.
@@ -68,15 +75,25 @@ TEST_F(ErrorDisplayTest, RenderStatus) {
     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");
+    llvm::SmallVector<StringRef> lines;
+    StringRef(result).split(lines, '\n');
+    //                1234567
+    ASSERT_EQ(lines[0], "^~~ ^~~");
+    ASSERT_EQ(lines[1], "|   error: Y");
+    ASSERT_EQ(lines[2], "error: X");
+  }
+  {
+    // Test diagnostics on the same line are emitted correctly.
+    SourceLocation loc1 = {FileSpec{"a.c"}, 1, 2, 0, false, true};
+    SourceLocation loc2 = {FileSpec{"a.c"}, 1, 6, 0, false, true};
+    std::string result =
+        Render({DiagnosticDetail{loc1, eSeverityError, "X", "X"},
+                DiagnosticDetail{loc2, eSeverityError, "Y", "Y"}});
+    llvm::SmallVector<StringRef> lines;
+    StringRef(result).split(lines, '\n');
+    //                1234567
+    ASSERT_EQ(lines[0], " ^   ^");
+    ASSERT_EQ(lines[1], " |   error: Y");
+    ASSERT_EQ(lines[2], " error: X");
   }
 }