https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/123837
>From bc7bb2471e44ca7441f592611fd293f2b8c90435 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 21 Jan 2025 14:34:49 -0800 Subject: [PATCH 1/5] Have Progress message updates include their messages when sent over DAP --- lldb/tools/lldb-dap/ProgressEvent.cpp | 15 ++++++++++----- lldb/tools/lldb-dap/ProgressEvent.h | 3 ++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp b/lldb/tools/lldb-dap/ProgressEvent.cpp index 0dcc2ee81001d5..7807ecfb6c34d0 100644 --- a/lldb/tools/lldb-dap/ProgressEvent.cpp +++ b/lldb/tools/lldb-dap/ProgressEvent.cpp @@ -117,6 +117,10 @@ json::Value ProgressEvent::ToJSON() const { body.try_emplace("cancellable", false); } + if (m_event_type == progressUpdate) { + EmplaceSafeString(body, "message", m_message); + } + std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count())); EmplaceSafeString(body, "timestamp", timestamp); @@ -164,10 +168,11 @@ const ProgressEvent &ProgressEventManager::GetMostRecentEvent() const { return m_last_update_event ? *m_last_update_event : m_start_event; } -void ProgressEventManager::Update(uint64_t progress_id, uint64_t completed, - uint64_t total) { - if (std::optional<ProgressEvent> event = ProgressEvent::Create( - progress_id, std::nullopt, completed, total, &GetMostRecentEvent())) { +void ProgressEventManager::Update(uint64_t progress_id, const char *message, + uint64_t completed, uint64_t total) { + if (std::optional<ProgressEvent> event = + ProgressEvent::Create(progress_id, StringRef(message), completed, + total, &GetMostRecentEvent())) { if (event->GetEventType() == progressEnd) m_finished = true; @@ -227,7 +232,7 @@ void ProgressEventReporter::Push(uint64_t progress_id, const char *message, m_unreported_start_events.push(event_manager); } } else { - it->second->Update(progress_id, completed, total); + it->second->Update(progress_id, message, completed, total); if (it->second->Finished()) m_event_managers.erase(it); } diff --git a/lldb/tools/lldb-dap/ProgressEvent.h b/lldb/tools/lldb-dap/ProgressEvent.h index 72317b879c803a..8494221890a1c8 100644 --- a/lldb/tools/lldb-dap/ProgressEvent.h +++ b/lldb/tools/lldb-dap/ProgressEvent.h @@ -99,7 +99,8 @@ class ProgressEventManager { /// Receive a new progress event for the start event and try to report it if /// appropriate. - void Update(uint64_t progress_id, uint64_t completed, uint64_t total); + void Update(uint64_t progress_id, const char *message, uint64_t completed, + uint64_t total); /// \return /// \b true if a \a progressEnd event has been notified. There's no >From faefdb6487eb97cca56755a20597f3d6a7348c6c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Tue, 21 Jan 2025 15:16:46 -0800 Subject: [PATCH 2/5] PR feedback, pass string ref instead of char * --- lldb/tools/lldb-dap/ProgressEvent.cpp | 12 +++++------- lldb/tools/lldb-dap/ProgressEvent.h | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp b/lldb/tools/lldb-dap/ProgressEvent.cpp index 7807ecfb6c34d0..6a4978c055e516 100644 --- a/lldb/tools/lldb-dap/ProgressEvent.cpp +++ b/lldb/tools/lldb-dap/ProgressEvent.cpp @@ -117,9 +117,8 @@ json::Value ProgressEvent::ToJSON() const { body.try_emplace("cancellable", false); } - if (m_event_type == progressUpdate) { + if (m_event_type == progressUpdate) EmplaceSafeString(body, "message", m_message); - } std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count())); EmplaceSafeString(body, "timestamp", timestamp); @@ -168,11 +167,10 @@ const ProgressEvent &ProgressEventManager::GetMostRecentEvent() const { return m_last_update_event ? *m_last_update_event : m_start_event; } -void ProgressEventManager::Update(uint64_t progress_id, const char *message, +void ProgressEventManager::Update(uint64_t progress_id, llvm::StringRef message, uint64_t completed, uint64_t total) { - if (std::optional<ProgressEvent> event = - ProgressEvent::Create(progress_id, StringRef(message), completed, - total, &GetMostRecentEvent())) { + if (std::optional<ProgressEvent> event = ProgressEvent::Create( + progress_id, message, completed, total, &GetMostRecentEvent())) { if (event->GetEventType() == progressEnd) m_finished = true; @@ -232,7 +230,7 @@ void ProgressEventReporter::Push(uint64_t progress_id, const char *message, m_unreported_start_events.push(event_manager); } } else { - it->second->Update(progress_id, message, completed, total); + it->second->Update(progress_id, StringRef(message), completed, total); if (it->second->Finished()) m_event_managers.erase(it); } diff --git a/lldb/tools/lldb-dap/ProgressEvent.h b/lldb/tools/lldb-dap/ProgressEvent.h index 8494221890a1c8..d1b9b9dd887cd8 100644 --- a/lldb/tools/lldb-dap/ProgressEvent.h +++ b/lldb/tools/lldb-dap/ProgressEvent.h @@ -99,7 +99,7 @@ class ProgressEventManager { /// Receive a new progress event for the start event and try to report it if /// appropriate. - void Update(uint64_t progress_id, const char *message, uint64_t completed, + void Update(uint64_t progress_id, llvm::StringRef message, uint64_t completed, uint64_t total); /// \return >From 56846b2fb5f491311af5ebfe98750c3159259e66 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 22 Jan 2025 11:56:15 -0800 Subject: [PATCH 3/5] Add new test for progress --- .../test/API/tools/lldb-dap/progress/Makefile | 3 + .../lldb-dap/progress/Progress_emitter.py | 86 +++++++++++++++++++ .../lldb-dap/progress/TestDAP_Progress.py | 49 +++++++++++ .../test/API/tools/lldb-dap/progress/main.cpp | 5 ++ 4 files changed, 143 insertions(+) create mode 100644 lldb/test/API/tools/lldb-dap/progress/Makefile create mode 100644 lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py create mode 100755 lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py create mode 100644 lldb/test/API/tools/lldb-dap/progress/main.cpp diff --git a/lldb/test/API/tools/lldb-dap/progress/Makefile b/lldb/test/API/tools/lldb-dap/progress/Makefile new file mode 100644 index 00000000000000..99998b20bcb050 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/progress/Makefile @@ -0,0 +1,3 @@ +CXX_SOURCES := main.cpp + +include Makefile.rules diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py new file mode 100644 index 00000000000000..8f2a504f8320f1 --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py @@ -0,0 +1,86 @@ +import inspect +import optparse +import shlex +import sys +import time + +import lldb + + +class ProgressTesterCommand: + program = "test-progress" + + @classmethod + def register_lldb_command(cls, debugger, module_name): + parser = cls.create_options() + cls.__doc__ = parser.format_help() + # Add any commands contained in this module to LLDB + command = "command script add -c %s.%s %s" % ( + module_name, + cls.__name__, + cls.program, + ) + debugger.HandleCommand(command) + print( + 'The "{0}" command has been installed, type "help {0}" or "{0} ' + '--help" for detailed help.'.format(cls.program) + ) + + @classmethod + def create_options(cls): + usage = "usage: %prog [options]" + description = "Jacob Lalonde's sbprogress testing tool" + # Opt parse is deprecated, but leaving this the way it is because it allows help formating + # Additionally all our commands use optparse right now, ideally we migrate them all in one go. + parser = optparse.OptionParser( + description=description, prog=cls.program, usage=usage + ) + + parser.add_option( + "--total", dest="total", help="Total to count up.", type="int" + ) + + parser.add_option( + "--seconds", + dest="seconds", + help="Total number of seconds to wait between increments", + type="int", + ) + + return parser + + def get_short_help(self): + return "Progress Tester" + + def get_long_help(self): + return self.help_string + + def __init__(self, debugger, unused): + self.parser = self.create_options() + self.help_string = self.parser.format_help() + + def __call__(self, debugger, command, exe_ctx, result): + + command_args = shlex.split(command) + try: + (cmd_options, args) = self.parser.parse_args(command_args) + except: + result.SetError("option parsing failed") + return + + total = cmd_options.total + progress = lldb.SBProgress("Progress tester", "Detail", total, debugger) + + # This actually should start at 1 but it's 6:30 on a Friday... + for i in range(1, total): + progress.Increment(1, f"Step {i}") + time.sleep(cmd_options.seconds) + + +def __lldb_init_module(debugger, dict): + # Register all classes that have a register_lldb_command method + for _name, cls in inspect.getmembers(sys.modules[__name__]): + if inspect.isclass(cls) and callable( + getattr(cls, "register_lldb_command", None) + ): + cls.register_lldb_command(debugger, __name__) diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py new file mode 100755 index 00000000000000..ad166578e224cd --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py @@ -0,0 +1,49 @@ +""" +Test lldb-dap output events +""" + +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import * +import os +import time + +import lldbdap_testcase + + +class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase): + @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")] + ) + 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", context="repl" + ) + + self.dap_server.wait_for_event("progressEnd", 15) + # Expect at least a start, an update, and end event + # However because the progress 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.assertTrue(start_found) + self.assertTrue(update_found) diff --git a/lldb/test/API/tools/lldb-dap/progress/main.cpp b/lldb/test/API/tools/lldb-dap/progress/main.cpp new file mode 100644 index 00000000000000..3bac5d0fd6db1a --- /dev/null +++ b/lldb/test/API/tools/lldb-dap/progress/main.cpp @@ -0,0 +1,5 @@ +int main() { + char *ptr = "unused"; + // break here + return 0; +} >From f6d8d81fcabec54b8d03755a12dde708e6cfde96 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 22 Jan 2025 12:14:43 -0800 Subject: [PATCH 4/5] Appease darker --- lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py | 1 - 1 file changed, 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py index 8f2a504f8320f1..f2ac77dce8f3a6 100644 --- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py +++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py @@ -60,7 +60,6 @@ def __init__(self, debugger, unused): self.help_string = self.parser.format_help() def __call__(self, debugger, command, exe_ctx, result): - command_args = shlex.split(command) try: (cmd_options, args) = self.parser.parse_args(command_args) >From e7bc9023fa4a4267dad486223f11618ca547991f Mon Sep 17 00:00:00 2001 From: Jacob Lalonde <jalalo...@fb.com> Date: Wed, 22 Jan 2025 13:08:59 -0800 Subject: [PATCH 5/5] Clean up code when copying personal tool, explain the raii comment better. --- lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py | 3 +-- lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py index f2ac77dce8f3a6..7f4055cab9ddda 100644 --- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py +++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py @@ -29,7 +29,7 @@ def register_lldb_command(cls, debugger, module_name): @classmethod def create_options(cls): usage = "usage: %prog [options]" - description = "Jacob Lalonde's sbprogress testing tool" + description = "SBProgress testing tool" # Opt parse is deprecated, but leaving this the way it is because it allows help formating # Additionally all our commands use optparse right now, ideally we migrate them all in one go. parser = optparse.OptionParser( @@ -70,7 +70,6 @@ def __call__(self, debugger, command, exe_ctx, result): total = cmd_options.total progress = lldb.SBProgress("Progress tester", "Detail", total, debugger) - # This actually should start at 1 but it's 6:30 on a Friday... for i in range(1, total): progress.Increment(1, f"Step {i}") time.sleep(cmd_options.seconds) 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 ad166578e224cd..36c0cef9c47143 100755 --- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py +++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py @@ -32,7 +32,7 @@ def test_output(self): self.dap_server.wait_for_event("progressEnd", 15) # Expect at least a start, an update, and end event - # However because the progress is an RAII object and we can't guaruntee + # 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) _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits