caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[LLDB-DAP] SBDebugger don't prefix title on progress updates (#124648)

Browse Source 
In my last DAP patch (#123837), we piped the DAP update message into the
update event. However, we had the title embedded into the update
message. This makes sense for progress Start, but makes the update
message look pretty wonky.


![image](https://github.com/user-attachments/assets/9f6083d1-fc50-455c-a1a7-a2f9bdaba22e)

Instead, we only use the title when it's the first message, removing the
duplicate title problem.

![image](https://github.com/user-attachments/assets/ee7aefd1-1852-46f7-94bc-84b8faef6dac)
This commit is contained in:
Jacob Lalonde
2025-03-05 12:02:44 -08:00
committed by GitHub
parent 2bbe30bf35
commit 02f024ca97
4 changed files with 246 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
lldb/bindings/interface/SBProgressDocstrings.i
View File

@@ -11,7 +11,48 @@ The Progress class helps make sure that progress is correctly reported
and will always send an initial progress update, updates when
Progress::Increment() is called, and also will make sure that a progress
completed update is reported even if the user doesn't explicitly cause one
to be sent.") lldb::SBProgress;
to be sent.
Progress can either be deterministic, incrementing up to a known total or non-deterministic
with an unbounded total. Deterministic is better if you know the items of work in advance, but non-deterministic
exposes a way to update a user during a long running process that work is taking place.
For all progresses the details provided in the constructor will be sent until an increment detail
is provided. This detail will also continue to be broadcasted on any subsequent update that doesn't
specify a new detail. Some implementations differ on throttling updates and this behavior differs primarily
if the progress is deterministic or non-deterministic. For DAP, non-deterministic update messages have a higher
throttling rate than deterministic ones.
Below are examples in Python for deterministic and non-deterministic progresses.
deterministic_progress1 = lldb.SBProgress('Deterministic Progress', 'Detail', 3, lldb.SBDebugger)
for i in range(3):
deterministic_progress1.Increment(1, f'Update {i}')
# The call to Finalize() is a no-op as we already incremented the right amount of
# times and caused the end event to be sent.
deterministic_progress1.Finalize()
deterministic_progress2 = lldb.SBProgress('Deterministic Progress', 'Detail', 10, lldb.SBDebugger)
for i in range(3):
deterministic_progress2.Increment(1, f'Update {i}')
# Cause the progress end event to be sent even if we didn't increment the right
# number of times. Real world examples would be in a try-finally block to ensure
# progress clean-up.
deterministic_progress2.Finalize()
If you don't call Finalize() when the progress is not done, the progress object will eventually get
garbage collected by the Python runtime, the end event will eventually get sent, but it is best not to
rely on the garbage collection when using lldb.SBProgress.
Non-deterministic progresses behave the same, but omit the total in the constructor.
non_deterministic_progress = lldb.SBProgress('Non deterministic progress, 'Detail', lldb.SBDebugger)
for i in range(10):
non_deterministic_progress.Increment(1)
# Explicitly send a progressEnd, otherwise this will be sent
# when the python runtime cleans up this object.
non_deterministic_progress.Finalize()
") lldb::SBProgress;
%feature("docstring",
"Finalize the SBProgress, which will cause a progress end event to be emitted. This

36
lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
View File

@@ -37,7 +37,11 @@ class ProgressTesterCommand:
)
parser.add_option(
"--total", dest="total", help="Total to count up.", type="int"
"--total",
dest="total",
help="Total items in this progress object. When this option is not specified, this will be an indeterminate progress.",
type="int",
default=None,
)
parser.add_option(
@@ -47,6 +51,14 @@ class ProgressTesterCommand:
type="int",
)
parser.add_option(
"--no-details",
dest="no_details",
help="Do not display details",
action="store_true",
default=False,
)
return parser
def get_short_help(self):
@@ -68,12 +80,30 @@ class ProgressTesterCommand:
return
total = cmd_options.total
progress = lldb.SBProgress("Progress tester", "Detail", total, debugger)
if total is None:
progress = lldb.SBProgress(
"Progress tester", "Initial Indeterminate Detail", debugger
)
else:
progress = lldb.SBProgress(
"Progress tester", "Initial Detail", total, debugger
)
# Check to see if total is set to None to indicate an indeterminate progress
# then default to 10 steps.
if total is None:
total = 10
for i in range(1, total):
progress.Increment(1, f"Step {i}")
if cmd_options.no_details:
progress.Increment(1)
else:
progress.Increment(1, f"Step {i}")
time.sleep(cmd_options.seconds)
# Not required for deterministic progress, but required for indeterminate progress.
progress.Finalize()
def __lldb_init_module(debugger, dict):
# Register all classes that have a register_lldb_command method

122
lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
View File

@@ -4,6 +4,7 @@ Test lldb-dap output events
from lldbsuite.test.decorators import *
from lldbsuite.test.lldbtest import *
import json
import os
import time
@@ -11,14 +12,46 @@ import lldbdap_testcase
class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
def verify_progress_events(
self,
expected_title,
expected_message=None,
expected_not_in_message=None,
only_verify_first_update=False,
):
self.dap_server.wait_for_event("progressEnd", 15)
self.assertTrue(len(self.dap_server.progress_events) > 0)
start_found = False
update_found = False
end_found = False
for event in self.dap_server.progress_events:
event_type = event["event"]
if "progressStart" in event_type:
title = event["body"]["title"]
self.assertIn(expected_title, title)
start_found = True
if "progressUpdate" in event_type:
message = event["body"]["message"]
if only_verify_first_update and update_found:
continue
if expected_message is not None:
self.assertIn(expected_message, message)
if expected_not_in_message is not None:
self.assertNotIn(expected_not_in_message, message)
update_found = True
if "progressEnd" in event_type:
end_found = True
self.assertTrue(start_found)
self.assertTrue(update_found)
self.assertTrue(end_found)
@skipIfWindows
def test_output(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
print(f"Progress emitter path: {progress_emitter}")
source = "main.cpp"
# Set breakpoint in the thread function so we can step the threads
breakpoint_ids = self.set_source_breakpoints(
source, [line_number(source, "// break here")]
)
@@ -30,20 +63,73 @@ class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
"`test-progress --total 3 --seconds 1", context="repl"
)
self.dap_server.wait_for_event("progressEnd", 15)
# Expect at least a start, an update, and end event
# However because the underlying Progress instance is an RAII object and we can't guaruntee
# it's deterministic destruction in the python API, we verify just start and update
# otherwise this test could be flakey.
self.assertTrue(len(self.dap_server.progress_events) > 0)
start_found = False
update_found = False
for event in self.dap_server.progress_events:
event_type = event["event"]
if "progressStart" in event_type:
start_found = True
if "progressUpdate" in event_type:
update_found = True
self.verify_progress_events(
expected_title="Progress tester",
expected_not_in_message="Progress tester",
)
self.assertTrue(start_found)
self.assertTrue(update_found)
@skipIfWindows
def test_output_nodetails(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
source = "main.cpp"
breakpoint_ids = self.set_source_breakpoints(
source, [line_number(source, "// break here")]
)
self.continue_to_breakpoints(breakpoint_ids)
self.dap_server.request_evaluate(
f"`command script import {progress_emitter}", context="repl"
)
self.dap_server.request_evaluate(
"`test-progress --total 3 --seconds 1 --no-details", context="repl"
)
self.verify_progress_events(
expected_title="Progress tester",
expected_message="Initial Detail",
)
@skipIfWindows
def test_output_indeterminate(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
source = "main.cpp"
breakpoint_ids = self.set_source_breakpoints(
source, [line_number(source, "// break here")]
)
self.continue_to_breakpoints(breakpoint_ids)
self.dap_server.request_evaluate(
f"`command script import {progress_emitter}", context="repl"
)
self.dap_server.request_evaluate("`test-progress --seconds 1", context="repl")
self.verify_progress_events(
expected_title="Progress tester: Initial Indeterminate Detail",
expected_message="Step 1",
only_verify_first_update=True,
)
@skipIfWindows
def test_output_nodetails_indeterminate(self):
program = self.getBuildArtifact("a.out")
self.build_and_launch(program)
progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
source = "main.cpp"
breakpoint_ids = self.set_source_breakpoints(
source, [line_number(source, "// break here")]
)
self.dap_server.request_evaluate(
f"`command script import {progress_emitter}", context="repl"
)
self.dap_server.request_evaluate(
"`test-progress --seconds 1 --no-details", context="repl"
)
self.verify_progress_events(
expected_title="Progress tester: Initial Indeterminate Detail",
expected_message="Initial Indeterminate Detail",
only_verify_first_update=True,
)

77
lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
View File

@@ -18,8 +18,32 @@ using namespace lldb;
namespace lldb_dap {
static void ProgressEventThreadFunction(DAP &dap) {
llvm::set_thread_name(dap.name + ".progress_handler");
static std::string GetStringFromStructuredData(lldb::SBStructuredData &data,
const char *key) {
lldb::SBStructuredData keyValue = data.GetValueForKey(key);
if (!keyValue)
return std::string();
const size_t length = keyValue.GetStringValue(nullptr, 0);
if (length == 0)
return std::string();
std::string str(length + 1, 0);
keyValue.GetStringValue(&str[0], length + 1);
return str;
}
static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data,
const char *key) {
lldb::SBStructuredData keyValue = data.GetValueForKey(key);
if (!keyValue.IsValid())
return 0;
return keyValue.GetUnsignedIntegerValue();
}
void ProgressEventThreadFunction(DAP &dap) {
lldb::SBListener listener("lldb-dap.progress.listener");
dap.debugger.GetBroadcaster().AddListener(
listener, lldb::SBDebugger::eBroadcastBitProgress |
@@ -35,14 +59,47 @@ static 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);
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");
if (completed == 0) {
if (total == UINT64_MAX) {
// This progress is non deterministic and won't get updated until it
// is completed. Send the "message" which will be the combined title
// and detail. The only other progress event for thus
// non-deterministic progress will be the completed event So there
// will be no need to update the detail.
const std::string message =
GetStringFromStructuredData(data, "message");
dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
} else {
// This progress is deterministic and will receive updates,
// on the progress creation event VSCode will save the message in
// the create packet and use that as the title, so we send just the
// title in the progressCreate packet followed immediately by a
// detail packet, if there is any detail.
const std::string title =
GetStringFromStructuredData(data, "title");
dap.SendProgressEvent(progress_id, title.c_str(), completed, total);
if (!details.empty())
dap.SendProgressEvent(progress_id, details.c_str(), completed,
total);
}
} else {
// This progress event is either the end of the progress dialog, or an
// update with possible detail. The "detail" string we send to VS Code
// will be appended to the progress dialog's initial text from when it
// was created.
dap.SendProgressEvent(progress_id, details.c_str(), completed, total);
}
}
}
}