Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lldb/include/lldb/Core/Progress.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@

namespace lldb_private {

/// Enum to indicate the origin of a progress event, internal or external.
enum class ProgressOrigin : uint8_t {
eLLDBInternal = 0,
eExternal = 1,
};

/// A Progress indicator helper class.
///
/// Any potentially long running sections of code in LLDB should report
Expand Down Expand Up @@ -83,7 +89,8 @@ class Progress {
Progress(std::string title, std::string details = {},
std::optional<uint64_t> total = std::nullopt,
lldb_private::Debugger *debugger = nullptr,
Timeout<std::nano> minimum_report_time = std::nullopt);
Timeout<std::nano> minimum_report_time = std::nullopt,
ProgressOrigin origin = ProgressOrigin::eLLDBInternal);

/// Destroy the progress object.
///
Expand Down Expand Up @@ -149,6 +156,9 @@ class Progress {

/// The "completed" value of the last reported event.
std::optional<uint64_t> m_prev_completed;

/// The origin of this progress event.
ProgressOrigin m_origin;
};

/// A class used to group progress reports by category. This is done by using a
Expand Down
13 changes: 7 additions & 6 deletions lldb/include/lldb/lldb-enumerations.h
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,10 @@ enum Format {
///< character arrays that can contain non printable
///< characters
eFormatAddressInfo, ///< Describe what an address points to (func + offset
///< with file/line, symbol + offset, data, etc)
eFormatHexFloat, ///< ISO C99 hex float string
eFormatInstruction, ///< Disassemble an opcode
eFormatVoid, ///< Do not print this
///< with file/line, symbol + offset, data, etc)
eFormatHexFloat, ///< ISO C99 hex float string
eFormatInstruction, ///< Disassemble an opcode
eFormatVoid, ///< Do not print this
eFormatUnicode8,
kNumFormats
};
Expand Down Expand Up @@ -302,7 +302,7 @@ enum ConnectionStatus {
eConnectionStatusNoConnection, ///< No connection
eConnectionStatusLostConnection, ///< Lost connection while connected to a
///< valid connection
eConnectionStatusInterrupted ///< Interrupted read
eConnectionStatusInterrupted ///< Interrupted read
};

enum ErrorType {
Expand Down Expand Up @@ -1094,7 +1094,7 @@ enum PathType {
ePathTypeGlobalLLDBTempSystemDir, ///< The LLDB temp directory for this
///< system, NOT cleaned up on a process
///< exit.
ePathTypeClangDir ///< Find path to Clang builtin headers
ePathTypeClangDir ///< Find path to Clang builtin headers
};

/// Kind of member function.
Expand Down Expand Up @@ -1357,6 +1357,7 @@ enum DebuggerBroadcastBit {
eBroadcastBitError = (1 << 2),
eBroadcastSymbolChange = (1 << 3),
eBroadcastBitProgressCategory = (1 << 4),
eBroadcastBitExternalProgressCategory = (1 << 5),
};

/// Used for expressing severity in logs and diagnostics.
Expand Down
13 changes: 10 additions & 3 deletions lldb/source/Core/Progress.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ static llvm::ManagedStatic<llvm::SignpostEmitter> g_progress_signposts;
Progress::Progress(std::string title, std::string details,
std::optional<uint64_t> total,
lldb_private::Debugger *debugger,
Timeout<std::nano> minimum_report_time)
Timeout<std::nano> minimum_report_time,
ProgressOrigin origin)
: m_total(total.value_or(Progress::kNonDeterministicTotal)),
m_minimum_report_time(minimum_report_time),
m_progress_data{title, ++g_id,
Expand All @@ -38,7 +39,7 @@ Progress::Progress(std::string title, std::string details,
std::chrono::nanoseconds(
std::chrono::steady_clock::now().time_since_epoch())
.count()),
m_details(std::move(details)) {
m_details(std::move(details)), m_origin(origin) {
std::lock_guard<std::mutex> guard(m_mutex);
ReportProgress();

Expand Down Expand Up @@ -106,9 +107,15 @@ void Progress::ReportProgress() {
if (completed < m_prev_completed)
return; // An overflow in the m_completed counter. Just ignore these events.

// Change the category bit if we're an internal or external progress.
uint32_t progress_category_bit =
m_origin == ProgressOrigin::eExternal
? lldb::eBroadcastBitExternalProgressCategory
: lldb::eBroadcastBitProgressCategory;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, it will report all progress events as aggregated by category. You'll need to check both eBroadcastBitProgress and eBroadcastBitProgressCategory and pick the right external variant.

Copy link
Contributor Author

@Jlalond Jlalond Dec 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand we have two categories and two defined bits here, but I'm confused what you mean by

You'll need to check both eBroadcastBitProgress and eBroadcastBitProgressCategory

Wouldn't I just want to set both bits (and unset internal), and pass that to the debugger to see if there is a listener for either?

EDIT:

I read around the class and a comment in ProgressManager mentioned the category is only used for beginning or ending of the progress, so I made ReportProgress set the external bit if it's external, and the manager sets the category if it's completed (or started).


Debugger::ReportProgress(m_progress_data.progress_id, m_progress_data.title,
m_details, completed, m_total,
m_progress_data.debugger_id);
m_progress_data.debugger_id, progress_category_bit);
m_prev_completed = completed;
}

Expand Down
Loading