[Lldb-commits] [lldb] [lldb] Don't report all progress event as completed. (PR #84281)

[Jonas Devlieghere via lldb-commits]

https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/84281

>From 57908c4924fbb339309e2a4c297178b2a3689bef Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Wed, 6 Mar 2024 22:28:14 -0800
Subject: [PATCH] [lldb] Don't report all progress event as completed.

Currently, progress events reported by the ProgressManager and broadcast
to eBroadcastBitProgressCategory always specify they're complete. The
problem is that the ProgressManager reports kNonDeterministicTotal for
both the total and the completed number of (sub)events. Because the
values are the same, the event reports itself as complete.

This patch fixes the issue by reporting 0 as the completed value for the
start event and kNonDeterministicTotal for the end event.
---
 lldb/include/lldb/Core/Progress.h          |  9 +++++++--
 lldb/source/Core/Progress.cpp              | 16 +++++++++-------
 lldb/unittests/Core/ProgressReportTest.cpp |  4 ++--
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index c6fc861fb71d86..c38f6dd0a140ed 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -148,9 +148,14 @@ class ProgressManager {
 
   static ProgressManager &Instance();
 
-  static void ReportProgress(const Progress::ProgressData &);
-
 private:
+  enum class EventType {
+    Begin,
+    End,
+  };
+  static void ReportProgress(const Progress::ProgressData &progress_data,
+                             EventType type);
+
   llvm::StringMap<std::pair<uint64_t, Progress::ProgressData>>
       m_progress_category_map;
   std::mutex m_progress_map_mutex;
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 9dcd7cf75ae057..b4b5e98b7ba493 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -97,7 +97,7 @@ void ProgressManager::Increment(const Progress::ProgressData &progress_data) {
   // initial progress report.
   if (!m_progress_category_map.contains(progress_data.title)) {
     m_progress_category_map[progress_data.title].second = progress_data;
-    ReportProgress(progress_data);
+    ReportProgress(progress_data, EventType::Begin);
   }
   m_progress_category_map[progress_data.title].first++;
 }
@@ -110,7 +110,7 @@ void ProgressManager::Decrement(const Progress::ProgressData &progress_data) {
     return;
 
   if (pos->second.first <= 1) {
-    ReportProgress(pos->second.second);
+    ReportProgress(pos->second.second, EventType::End);
     m_progress_category_map.erase(progress_data.title);
   } else {
     --pos->second.first;
@@ -118,12 +118,14 @@ void ProgressManager::Decrement(const Progress::ProgressData &progress_data) {
 }
 
 void ProgressManager::ReportProgress(
-    const Progress::ProgressData &progress_data) {
+    const Progress::ProgressData &progress_data, EventType type) {
   // The category bit only keeps track of when progress report categories have
   // started and ended, so clear the details and reset other fields when
   // broadcasting to it since that bit doesn't need that information.
-  Debugger::ReportProgress(
-      progress_data.progress_id, progress_data.title, "",
-      Progress::kNonDeterministicTotal, Progress::kNonDeterministicTotal,
-      progress_data.debugger_id, Debugger::eBroadcastBitProgressCategory);
+  const uint64_t completed =
+      (type == EventType::Begin) ? 0 : Progress::kNonDeterministicTotal;
+  Debugger::ReportProgress(progress_data.progress_id, progress_data.title, "",
+                           completed, Progress::kNonDeterministicTotal,
+                           progress_data.debugger_id,
+                           Debugger::eBroadcastBitProgressCategory);
 }
diff --git a/lldb/unittests/Core/ProgressReportTest.cpp b/lldb/unittests/Core/ProgressReportTest.cpp
index 98cbc475ce2835..e0253cbc4ec59b 100644
--- a/lldb/unittests/Core/ProgressReportTest.cpp
+++ b/lldb/unittests/Core/ProgressReportTest.cpp
@@ -168,7 +168,7 @@ TEST_F(ProgressReportTest, TestProgressManager) {
 
   ASSERT_EQ(data->GetDetails(), "");
   ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
+  ASSERT_FALSE(data->GetCompleted());
   ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
   ASSERT_EQ(data->GetMessage(), "Progress report 1");
 
@@ -199,7 +199,7 @@ TEST_F(ProgressReportTest, TestProgressManager) {
 
   ASSERT_EQ(data->GetDetails(), "");
   ASSERT_FALSE(data->IsFinite());
-  ASSERT_TRUE(data->GetCompleted());
+  ASSERT_FALSE(data->GetCompleted());
   ASSERT_EQ(data->GetTotal(), Progress::kNonDeterministicTotal);
   ASSERT_EQ(data->GetMessage(), "Overlapping report 1");