[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
Jacob Lalonde via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 18 11:34:14 PST 2025
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/124648
>From 12ff645735c1dbf51e58b9f80d4e3e13a0babdf5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 27 Jan 2025 13:41:58 -0800
Subject: [PATCH 1/4] Only include title on the first message
---
lldb/include/lldb/Core/DebuggerEvents.h | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 49a4ecf8e537e..52e4f77d7637d 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -44,12 +44,15 @@ class ProgressEventData : public EventData {
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
- std::string message = m_title;
- if (!m_details.empty()) {
- message.append(": ");
- message.append(m_details);
- }
- return message;
+ if (m_completed == 0) {
+ std::string message = m_title;
+ if (!m_details.empty()) {
+ message.append(": ");
+ message.append(m_details);
+ }
+ return message;
+ } else
+ return !m_details.empty() ? m_details : std::string();
}
const std::string &GetTitle() const { return m_title; }
const std::string &GetDetails() const { return m_details; }
>From 82ed76ae3b6bef176bf54dd1031f0cb9c95081c1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Mon, 27 Jan 2025 14:48:01 -0800
Subject: [PATCH 2/4] Add comment explaining if and add a test
---
lldb/include/lldb/Core/DebuggerEvents.h | 1 +
lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index 52e4f77d7637d..ca06ee835fde3 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -44,6 +44,7 @@ class ProgressEventData : public EventData {
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
+ // Only put the title in the message of the progress create event.
if (m_completed == 0) {
std::string message = m_title;
if (!m_details.empty()) {
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
index 36c0cef9c4714..945c3f7633364 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -41,8 +41,13 @@ def test_output(self):
for event in self.dap_server.progress_events:
event_type = event["event"]
if "progressStart" in event_type:
+ title = event["body"]["title"]
+ self.assertIn("Progress tester", title)
start_found = True
if "progressUpdate" in event_type:
+ message = event["body"]["message"]
+ print(f"Progress update: {message}")
+ self.assertNotIn("Progres tester", message)
update_found = True
self.assertTrue(start_found)
>From e15090782a93e07e8a260a0594ca02430e68e09f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Thu, 30 Jan 2025 10:04:04 -0800
Subject: [PATCH 3/4] Migrate to structured data
Summary:
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D68927453
---
lldb/include/lldb/Core/DebuggerEvents.h | 16 +++-----
lldb/tools/lldb-dap/lldb-dap.cpp | 54 +++++++++++++++++++++----
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h
index ca06ee835fde3..49a4ecf8e537e 100644
--- a/lldb/include/lldb/Core/DebuggerEvents.h
+++ b/lldb/include/lldb/Core/DebuggerEvents.h
@@ -44,16 +44,12 @@ class ProgressEventData : public EventData {
uint64_t GetCompleted() const { return m_completed; }
uint64_t GetTotal() const { return m_total; }
std::string GetMessage() const {
- // Only put the title in the message of the progress create event.
- if (m_completed == 0) {
- std::string message = m_title;
- if (!m_details.empty()) {
- message.append(": ");
- message.append(m_details);
- }
- return message;
- } else
- return !m_details.empty() ? m_details : std::string();
+ std::string message = m_title;
+ if (!m_details.empty()) {
+ message.append(": ");
+ message.append(m_details);
+ }
+ return message;
}
const std::string &GetTitle() const { return m_title; }
const std::string &GetDetails() const { return m_details; }
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..4b3460190a7c9 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -55,6 +55,7 @@
#include <thread>
#include <vector>
+#include <iostream>
#if defined(_WIN32)
// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
// definitions that conflict with other system headers.
@@ -412,6 +413,30 @@ void SendStdOutStdErr(DAP &dap, lldb::SBProcess &process) {
dap.SendOutput(OutputType::Stderr, llvm::StringRef(buffer, count));
}
+static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
+ const char *key) {
+ lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+ if (!keyValue)
+ return std::string();
+
+ size_t size = keyValue.GetStringValue(nullptr, 0);
+ std::cout << "Size for " << key << " " << size << std::endl;
+ std::string stringValue;
+ stringValue.resize(size);
+ keyValue.GetStringValue(&stringValue[0], size + 1);
+ std::cout << "String value after: " << stringValue << std::endl;
+ return stringValue;
+}
+
+static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
+ const char *key) {
+ lldb::SBStructuredData keyValue = data.GetValueForKey(key);
+
+ if (!keyValue.IsValid())
+ return -1;
+ return keyValue.GetUnsignedIntegerValue();
+}
+
void ProgressEventThreadFunction(DAP &dap) {
lldb::SBListener listener("lldb-dap.progress.listener");
dap.debugger.GetBroadcaster().AddListener(
@@ -428,14 +453,27 @@ void ProgressEventThreadFunction(DAP &dap) {
done = true;
}
} else {
- uint64_t progress_id = 0;
- uint64_t completed = 0;
- uint64_t total = 0;
- bool is_debugger_specific = false;
- const char *message = lldb::SBDebugger::GetProgressFromEvent(
- event, progress_id, completed, total, is_debugger_specific);
- if (message)
- dap.SendProgressEvent(progress_id, message, completed, total);
+ lldb::SBStructuredData data =
+ lldb::SBDebugger::GetProgressDataFromEvent(event);
+
+ uint64_t progress_id = GetUintFromStructuredData(data, "progress_id");
+ uint64_t completed = GetUintFromStructuredData(data, "completed");
+ uint64_t total = GetUintFromStructuredData(data, "total");
+ std::string message;
+ // Include the title on the first event.
+ if (completed == 0) {
+ std::string title = GetStringFromStructuredData(data, "title");
+ message += title;
+ message += ": ";
+ }
+
+ std::string details = GetStringFromStructuredData(data, "details");
+ message += details;
+ // Verbose check, but we get -1 for the uint64 on failure to read
+ // so we check everything before broadcasting an event.
+ if (message.length() > 0 && progress_id > 0 && total >= 0 &&
+ completed >= 0)
+ dap.SendProgressEvent(progress_id, message.c_str(), completed, total);
}
}
}
>From 19a03d8c40103298f0cb51af173f7be086eefd62 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde <jalalonde at fb.com>
Date: Tue, 18 Feb 2025 11:28:58 -0800
Subject: [PATCH 4/4] Feedback from Jonas, and cleanup dev logging
---
lldb/tools/lldb-dap/lldb-dap.cpp | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 4b3460190a7c9..9635a683d299b 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -55,7 +55,6 @@
#include <thread>
#include <vector>
-#include <iostream>
#if defined(_WIN32)
// We need to #define NOMINMAX in order to skip `min()` and `max()` macro
// definitions that conflict with other system headers.
@@ -419,13 +418,10 @@ static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
if (!keyValue)
return std::string();
- size_t size = keyValue.GetStringValue(nullptr, 0);
- std::cout << "Size for " << key << " " << size << std::endl;
- std::string stringValue;
- stringValue.resize(size);
- keyValue.GetStringValue(&stringValue[0], size + 1);
- std::cout << "String value after: " << stringValue << std::endl;
- return stringValue;
+ const size_t length = keyValue.GetStringValue(nullptr, 0);
+ std::string str(length + 1, 0);
+ value.GetStringValue(&str[0], length);
+ return str;
}
static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
@@ -433,7 +429,7 @@ static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
lldb::SBStructuredData keyValue = data.GetValueForKey(key);
if (!keyValue.IsValid())
- return -1;
+ return 0;
return keyValue.GetUnsignedIntegerValue();
}
@@ -456,23 +452,23 @@ void ProgressEventThreadFunction(DAP &dap) {
lldb::SBStructuredData data =
lldb::SBDebugger::GetProgressDataFromEvent(event);
- uint64_t progress_id = GetUintFromStructuredData(data, "progress_id");
- uint64_t completed = GetUintFromStructuredData(data, "completed");
- uint64_t total = GetUintFromStructuredData(data, "total");
+ const uint64_t progress_id = GetUintFromStructuredData(data, "progress_id");
+ const uint64_t completed = GetUintFromStructuredData(data, "completed");
+ const uint64_t total = GetUintFromStructuredData(data, "total");
+ const std::string details = GetStringFromStructuredData(data, "details");
std::string message;
// Include the title on the first event.
if (completed == 0) {
- std::string title = GetStringFromStructuredData(data, "title");
+ const std::string title = GetStringFromStructuredData(data, "title");
message += title;
message += ": ";
}
- std::string details = GetStringFromStructuredData(data, "details");
+
message += details;
// Verbose check, but we get -1 for the uint64 on failure to read
// so we check everything before broadcasting an event.
- if (message.length() > 0 && progress_id > 0 && total >= 0 &&
- completed >= 0)
+ if (message.length() > 0)
dap.SendProgressEvent(progress_id, message.c_str(), completed, total);
}
}
More information about the lldb-commits
mailing list