https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/71723
>From e92d7125be6b6677d0553967bb3f8e821de3ea18 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Tue, 7 Nov 2023 16:57:23 -0800 Subject: [PATCH 1/4] Improve DAP logpoint value summary --- .../API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py | 5 +++-- lldb/test/API/tools/lldb-dap/breakpoint/main.cpp | 2 ++ lldb/tools/lldb-dap/BreakpointBase.cpp | 8 +++++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py index 25e794a49d3ac12..44858c888a96940 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py @@ -48,7 +48,8 @@ def test_logmessage_basic(self): # 1. First at the loop line with logMessage # 2. Second guard breakpoint at a line after loop logMessage_prefix = "This is log message for { -- " - logMessage = logMessage_prefix + "{i + 3}" + message = '"Hello from main!"' + logMessage = logMessage_prefix + "{i + 3}, {message}" [loop_breakpoint_id, post_loop_breakpoint_id] = self.set_source_breakpoints( self.main_path, [loop_line, after_loop_line], @@ -75,7 +76,7 @@ def test_logmessage_basic(self): # Verify log message match for idx, logMessage_line in enumerate(logMessage_output): result = idx + 3 - self.assertEqual(logMessage_line, logMessage_prefix + str(result)) + self.assertEqual(logMessage_line, f"{logMessage_prefix}{result}, {message}") @skipIfWindows @skipIfRemote diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index 935a63fab6d0c36..84560dad92c286e 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -28,6 +28,7 @@ int main(int argc, char const *argv[]) { exit(1); } + const char * message = "Hello from main!"; int (*foo)(int) = (int (*)(int))dlsym(handle, "foo"); if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); @@ -38,6 +39,7 @@ int main(int argc, char const *argv[]) { for (int i = 0; i < 10; ++i) { int x = twelve(i) + thirteen(i) + a::fourteen(i); // break loop } + printf("%s\n", message); try { throw std::invalid_argument("throwing exception for testing"); } catch (...) { diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp index cd12f97fb13dfe9..d5253431053cc25 100644 --- a/lldb/tools/lldb-dap/BreakpointBase.cpp +++ b/lldb/tools/lldb-dap/BreakpointBase.cpp @@ -295,9 +295,11 @@ bool BreakpointBase::BreakpointHitCallback( frame.GetValueForVariablePath(expr, lldb::eDynamicDontRunTarget); if (value.GetError().Fail()) value = frame.EvaluateExpression(expr); - const char *expr_val = value.GetValue(); - if (expr_val) - output += expr_val; + llvm::StringRef summary_str = value.GetSummary(); + if (!summary_str.empty()) + output += summary_str.str(); + else + output += value.GetValue(); } else { output += messagePart.text; } >From 9664c6f13accbab2616e891ac031701566fd58b4 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Wed, 8 Nov 2023 14:54:45 -0800 Subject: [PATCH 2/4] Use the same format as variables view --- .../API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py | 4 +++- lldb/test/API/tools/lldb-dap/breakpoint/main.cpp | 2 +- lldb/tools/lldb-dap/BreakpointBase.cpp | 7 ++----- lldb/tools/lldb-dap/JSONUtils.cpp | 9 +++++++-- lldb/tools/lldb-dap/JSONUtils.h | 3 +++ 5 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py index 44858c888a96940..8614b3e19be6962 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py @@ -48,6 +48,7 @@ def test_logmessage_basic(self): # 1. First at the loop line with logMessage # 2. Second guard breakpoint at a line after loop logMessage_prefix = "This is log message for { -- " + message_addr_pattern = r'\b0x[0-9A-Fa-f]+\b' message = '"Hello from main!"' logMessage = logMessage_prefix + "{i + 3}, {message}" [loop_breakpoint_id, post_loop_breakpoint_id] = self.set_source_breakpoints( @@ -76,7 +77,8 @@ def test_logmessage_basic(self): # Verify log message match for idx, logMessage_line in enumerate(logMessage_output): result = idx + 3 - self.assertEqual(logMessage_line, f"{logMessage_prefix}{result}, {message}") + reg_str = f"{logMessage_prefix}{result}, {message_addr_pattern} {message}" + self.assertRegex(logMessage_line, reg_str) @skipIfWindows @skipIfRemote diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp index 84560dad92c286e..a84546a95af1533 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp +++ b/lldb/test/API/tools/lldb-dap/breakpoint/main.cpp @@ -28,7 +28,7 @@ int main(int argc, char const *argv[]) { exit(1); } - const char * message = "Hello from main!"; + const char *message = "Hello from main!"; int (*foo)(int) = (int (*)(int))dlsym(handle, "foo"); if (foo == nullptr) { fprintf(stderr, "%s\n", dlerror()); diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp index d5253431053cc25..b0946f0103c2a34 100644 --- a/lldb/tools/lldb-dap/BreakpointBase.cpp +++ b/lldb/tools/lldb-dap/BreakpointBase.cpp @@ -8,6 +8,7 @@ #include "BreakpointBase.h" #include "DAP.h" +#include "JSONUtils.h" #include "llvm/ADT/StringExtras.h" using namespace lldb_dap; @@ -295,11 +296,7 @@ bool BreakpointBase::BreakpointHitCallback( frame.GetValueForVariablePath(expr, lldb::eDynamicDontRunTarget); if (value.GetError().Fail()) value = frame.EvaluateExpression(expr); - llvm::StringRef summary_str = value.GetSummary(); - if (!summary_str.empty()) - output += summary_str.str(); - else - output += value.GetValue(); + output += ValeuToString(value); } else { output += messagePart.text; } diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 46528a2a28d4d6f..84c8037108cdcd4 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -210,8 +210,7 @@ static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue value) { return TryCreateAutoSummaryForContainer(value); } -void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object, - llvm::StringRef key) { +std::string ValeuToString(lldb::SBValue v) { std::string result; llvm::raw_string_ostream strm(result); @@ -242,6 +241,12 @@ void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object, } } } + return result; +} + +void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object, + llvm::StringRef key) { + std::string result = ValeuToString(v); EmplaceSafeString(object, key, result); } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index d829e650b8f31c4..39f2be6a2dde301 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -167,6 +167,9 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj, void FillResponse(const llvm::json::Object &request, llvm::json::Object &response); +/// Utility function to convert SBValue \v into a string. +std::string ValeuToString(lldb::SBValue v); + /// Emplace the string value from an SBValue into the supplied object /// using \a key as the key that will contain the value. /// >From 88d761d5d7fffa1dbb47320bde7e96c33d8d8c79 Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Wed, 8 Nov 2023 15:12:40 -0800 Subject: [PATCH 3/4] Fix python formatter --- lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py index 8614b3e19be6962..d5b8aab4af42a98 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py @@ -48,7 +48,7 @@ def test_logmessage_basic(self): # 1. First at the loop line with logMessage # 2. Second guard breakpoint at a line after loop logMessage_prefix = "This is log message for { -- " - message_addr_pattern = r'\b0x[0-9A-Fa-f]+\b' + message_addr_pattern = r"\b0x[0-9A-Fa-f]+\b" message = '"Hello from main!"' logMessage = logMessage_prefix + "{i + 3}, {message}" [loop_breakpoint_id, post_loop_breakpoint_id] = self.set_source_breakpoints( >From b15539893f3315c08f4ee10ab89e9b6dd1edd11c Mon Sep 17 00:00:00 2001 From: jeffreytan81 <jeffrey...@fb.com> Date: Wed, 8 Nov 2023 15:27:05 -0800 Subject: [PATCH 4/4] Fix typo and make test less confusing --- .../test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py | 6 +++--- lldb/tools/lldb-dap/BreakpointBase.cpp | 2 +- lldb/tools/lldb-dap/JSONUtils.cpp | 4 ++-- lldb/tools/lldb-dap/JSONUtils.h | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py index d5b8aab4af42a98..2367313b095ea26 100644 --- a/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py +++ b/lldb/test/API/tools/lldb-dap/breakpoint/TestDAP_logpoints.py @@ -48,8 +48,6 @@ def test_logmessage_basic(self): # 1. First at the loop line with logMessage # 2. Second guard breakpoint at a line after loop logMessage_prefix = "This is log message for { -- " - message_addr_pattern = r"\b0x[0-9A-Fa-f]+\b" - message = '"Hello from main!"' logMessage = logMessage_prefix + "{i + 3}, {message}" [loop_breakpoint_id, post_loop_breakpoint_id] = self.set_source_breakpoints( self.main_path, @@ -74,10 +72,12 @@ def test_logmessage_basic(self): loop_count = 10 self.assertEqual(len(logMessage_output), loop_count) + message_addr_pattern = r"\b0x[0-9A-Fa-f]+\b" + message_content = '"Hello from main!"' # Verify log message match for idx, logMessage_line in enumerate(logMessage_output): result = idx + 3 - reg_str = f"{logMessage_prefix}{result}, {message_addr_pattern} {message}" + reg_str = f"{logMessage_prefix}{result}, {message_addr_pattern} {message_content}" self.assertRegex(logMessage_line, reg_str) @skipIfWindows diff --git a/lldb/tools/lldb-dap/BreakpointBase.cpp b/lldb/tools/lldb-dap/BreakpointBase.cpp index b0946f0103c2a34..bc9bde97678adbd 100644 --- a/lldb/tools/lldb-dap/BreakpointBase.cpp +++ b/lldb/tools/lldb-dap/BreakpointBase.cpp @@ -296,7 +296,7 @@ bool BreakpointBase::BreakpointHitCallback( frame.GetValueForVariablePath(expr, lldb::eDynamicDontRunTarget); if (value.GetError().Fail()) value = frame.EvaluateExpression(expr); - output += ValeuToString(value); + output += ValueToString(value); } else { output += messagePart.text; } diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp index 84c8037108cdcd4..2ff17616c2e9986 100644 --- a/lldb/tools/lldb-dap/JSONUtils.cpp +++ b/lldb/tools/lldb-dap/JSONUtils.cpp @@ -210,7 +210,7 @@ static std::optional<std::string> TryCreateAutoSummary(lldb::SBValue value) { return TryCreateAutoSummaryForContainer(value); } -std::string ValeuToString(lldb::SBValue v) { +std::string ValueToString(lldb::SBValue v) { std::string result; llvm::raw_string_ostream strm(result); @@ -246,7 +246,7 @@ std::string ValeuToString(lldb::SBValue v) { void SetValueForKey(lldb::SBValue &v, llvm::json::Object &object, llvm::StringRef key) { - std::string result = ValeuToString(v); + std::string result = ValueToString(v); EmplaceSafeString(object, key, result); } diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h index 39f2be6a2dde301..02ee499445fa19b 100644 --- a/lldb/tools/lldb-dap/JSONUtils.h +++ b/lldb/tools/lldb-dap/JSONUtils.h @@ -168,7 +168,7 @@ void FillResponse(const llvm::json::Object &request, llvm::json::Object &response); /// Utility function to convert SBValue \v into a string. -std::string ValeuToString(lldb::SBValue v); +std::string ValueToString(lldb::SBValue v); /// Emplace the string value from an SBValue into the supplied object /// using \a key as the key that will contain the value. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits