[Lldb-commits] [lldb] [lldb][progress] Correctly check total for deterministic progress (PR #79912)

[Chelsea Cassanova via lldb-commits]

https://github.com/chelcassanova updated https://github.com/llvm/llvm-project/pull/79912

>From af9a5581702b5c9ca8009fc32c7ae10a1654d391 Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Mon, 29 Jan 2024 15:29:46 -0800
Subject: [PATCH 1/2] [lldb][progress] Correctly check total for deterministic
 progress

The `total` parameter for the constructor for Progress was changed to a
std::optional in https://github.com/llvm/llvm-project/pull/77547. When
initializing the `m_total` member variable for progress, it is set to 1
if the `total` parameter is std::nullopt. Other areas of the code were
still checking if `m_total` was a UINT64_MAX to determine if the
progress was deterministic or not, so these have been changed to check
for the integer 1.

The member variable `m_total` could be changed to a std::optional as
well, but this means that the `ProgressEventData::GetTotal()` (which is
used for the public API) would
either need to return a std::optional value or it would return some
specific integer to represent non-deterministic progress if `m_total`
is std::nullopt.
---
 lldb/include/lldb/Core/DebuggerEvents.h | 2 +-
 lldb/source/Core/DebuggerEvents.cpp     | 2 +-
 lldb/source/Core/Progress.cpp           | 1 -
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 4a27766e94e3a..2c3fcd7069d5e 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -39,7 +39,7 @@ class ProgressEventData : public EventData {
   GetAsStructuredData(const Event *event_ptr);
 
   uint64_t GetID() const { return m_id; }
-  bool IsFinite() const { return m_total != UINT64_MAX; }
+  bool IsFinite() const { return m_total != 1; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   std::string GetMessage() const {
diff --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index dd77fff349a64..c83bc20fba97a 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/lldb/source/Core/DebuggerEvents.cpp
@@ -41,7 +41,7 @@ void ProgressEventData::Dump(Stream *s) const {
     s->PutCString(", type = update");
   // If m_total is UINT64_MAX, there is no progress to report, just "start"
   // and "end". If it isn't we will show the completed and total amounts.
-  if (m_total != UINT64_MAX)
+  if (m_total != 1)
     s->Printf(", progress = %" PRIu64 " of %" PRIu64, m_completed, m_total);
 }
 
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 355d6952e53ca..411a27fbf7e99 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -23,7 +23,6 @@ Progress::Progress(std::string title, std::string details,
                    lldb_private::Debugger *debugger)
     : m_title(title), m_details(details), m_id(++g_id), m_completed(0),
       m_total(1) {
-  assert(total == std::nullopt || total > 0);
   if (total)
     m_total = *total;
 

>From 466836d9f8c462ea1f9f81744a76f90b06dcca2b Mon Sep 17 00:00:00 2001
From: Chelsea Cassanova <chelsea_cassanova at apple.com>
Date: Tue, 30 Jan 2024 11:18:10 -0800
Subject: [PATCH 2/2] Use UINT64_MAX, use static class var instead of magic
 number

Uses a static class member set to UINT64_MAX instead of using a magic
number of 1 to represent non-deterministic progress.
---
 lldb/include/lldb/Core/DebuggerEvents.h | 3 ++-
 lldb/include/lldb/Core/Progress.h       | 3 +++
 lldb/source/Core/DebuggerEvents.cpp     | 3 ++-
 lldb/source/Core/Progress.cpp           | 2 +-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 2c3fcd7069d5e..74bb05e6e6bf8 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/ModuleSpec.h"
+#include "lldb/Core/Progress.h"
 #include "lldb/Utility/Event.h"
 #include "lldb/Utility/StructuredData.h"
 
@@ -39,7 +40,7 @@ class ProgressEventData : public EventData {
   GetAsStructuredData(const Event *event_ptr);
 
   uint64_t GetID() const { return m_id; }
-  bool IsFinite() const { return m_total != 1; }
+  bool IsFinite() const { return m_total != Progress::kNonDeterministicTotal; }
   uint64_t GetCompleted() const { return m_completed; }
   uint64_t GetTotal() const { return m_total; }
   std::string GetMessage() const {
diff --git a/lldb/include/lldb/Core/Progress.h b/lldb/include/lldb/Core/Progress.h
index 65d30ea25cd29..5d88291024605 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -93,6 +93,9 @@ class Progress {
   void Increment(uint64_t amount = 1,
                  std::optional<std::string> updated_detail = {});
 
+  /// Used to indicate a non-deterministic progress report
+  static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX;
+
 private:
   void ReportProgress();
   static std::atomic<uint64_t> g_id;
diff --git a/lldb/source/Core/DebuggerEvents.cpp b/lldb/source/Core/DebuggerEvents.cpp
index c83bc20fba97a..65aed0eba9c41 100644
--- a/lldb/source/Core/DebuggerEvents.cpp
+++ b/lldb/source/Core/DebuggerEvents.cpp
@@ -9,6 +9,7 @@
 #include "lldb/Core/DebuggerEvents.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/Progress.h"
 #include "llvm/Support/WithColor.h"
 
 using namespace lldb_private;
@@ -41,7 +42,7 @@ void ProgressEventData::Dump(Stream *s) const {
     s->PutCString(", type = update");
   // If m_total is UINT64_MAX, there is no progress to report, just "start"
   // and "end". If it isn't we will show the completed and total amounts.
-  if (m_total != 1)
+  if (m_total != Progress::kNonDeterministicTotal)
     s->Printf(", progress = %" PRIu64 " of %" PRIu64, m_completed, m_total);
 }
 
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index 411a27fbf7e99..732efbc342b45 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -22,7 +22,7 @@ Progress::Progress(std::string title, std::string details,
                    std::optional<uint64_t> total,
                    lldb_private::Debugger *debugger)
     : m_title(title), m_details(details), m_id(++g_id), m_completed(0),
-      m_total(1) {
+      m_total(Progress::kNonDeterministicTotal) {
   if (total)
     m_total = *total;