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

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

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.

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


3 Files Affected:

- (modified) lldb/include/lldb/Core/Progress.h (+7-2) 
- (modified) lldb/source/Core/Progress.cpp (+10-8) 
- (modified) lldb/unittests/Core/ProgressReportTest.cpp (+2-2) 


``````````diff
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..6f2f62c21a0c7e 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) {
-  // The category bit only keeps track of when progress report categories have
+    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");
 

``````````

</details>


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