https://github.com/cmtice updated https://github.com/llvm/llvm-project/pull/107485
>From 15541f354decf80586d590db9f9cb353be04b122 Mon Sep 17 00:00:00 2001 From: Caroline Tice <cmt...@google.com> Date: Thu, 5 Sep 2024 15:51:35 -0700 Subject: [PATCH 1/5] [lldb-dap] Add feature to remember last non-empty expression. Update lldb-dap so if the user just presses return, which sends an empty expression, it re-evaluates the most recent non-empty expression/command. Also udpated test to test this case. --- lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py | 3 +++ lldb/tools/lldb-dap/lldb-dap.cpp | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 29548a835c6919..9ed0fc564268a7 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -60,7 +60,10 @@ def run_test_evaluate_expressions( # Expressions at breakpoint 1, which is in main self.assertEvaluate("var1", "20") + # Empty expression should equate to the previous expression. + self.assertEvaluate("", "20") self.assertEvaluate("var2", "21") + self.assertEvaluate("", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") self.assertEvaluate("struct1.foo", "15") diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index c5c4b09f15622b..a6a701dc2219fa 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1363,6 +1363,14 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); + static std::string last_nonempty_expression; + + // Remember the last non-empty expression from the user, and use that if + // the current expression is empty (i.e. the user hit plain 'return'). + if (!expression.empty()) + last_nonempty_expression = expression; + else + expression = last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) { >From e35928e08f792163dd4886e797bc6de3d16ea6e6 Mon Sep 17 00:00:00 2001 From: Caroline Tice <cmt...@google.com> Date: Fri, 6 Sep 2024 10:02:18 -0700 Subject: [PATCH 2/5] [lldb] Add feature to remember last non-empty expression Make last_nonempty_spression part of DAP struct rather than a static variable. --- lldb/tools/lldb-dap/DAP.h | 1 + lldb/tools/lldb-dap/lldb-dap.cpp | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index f4fdec6e895ad1..4220c15d3ae70d 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -205,6 +205,7 @@ struct DAP { std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; lldb::SBFormat thread_format; + std::string last_nonempty_expression; DAP(); ~DAP(); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index a6a701dc2219fa..d3728df9183aa1 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1363,14 +1363,13 @@ void request_evaluate(const llvm::json::Object &request) { lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments); std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - static std::string last_nonempty_expression; // Remember the last non-empty expression from the user, and use that if // the current expression is empty (i.e. the user hit plain 'return'). if (!expression.empty()) - last_nonempty_expression = expression; + g_dap.last_nonempty_expression = expression; else - expression = last_nonempty_expression; + expression = g_dap.last_nonempty_expression; if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) { >From 616017152f3f0611462e9863273754036b52f7eb Mon Sep 17 00:00:00 2001 From: Caroline Tice <cmt...@google.com> Date: Thu, 12 Sep 2024 10:52:32 -0700 Subject: [PATCH 3/5] [lldb-dap] Add feature to remember last non-empty expression Update to handle commands & variables separately: empty command expressions are passed to the CommandIntepreter to handle as it normally does; empty variable expressions are updated to use the last non-empty variable expression, if the last expression was a variable (not a command). Also updated the test case to test these cases properly, and added a 'memory read' followed by an empty expression, to make sure it handles that sequence correctly. --- .../lldb-dap/evaluate/TestDAP_evaluate.py | 16 +++++++-- .../test/API/tools/lldb-dap/evaluate/main.cpp | 3 +- lldb/tools/lldb-dap/DAP.h | 13 ++++++- lldb/tools/lldb-dap/LLDBUtils.cpp | 3 +- lldb/tools/lldb-dap/lldb-dap.cpp | 35 ++++++++++++++----- 5 files changed, 56 insertions(+), 14 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 9ed0fc564268a7..510f44c1d7230c 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -54,6 +54,7 @@ def run_test_evaluate_expressions( line_number(source, "// breakpoint 5"), line_number(source, "// breakpoint 6"), line_number(source, "// breakpoint 7"), + line_number(source, "// breakpoint 8"), ], ) self.continue_to_next_stop() @@ -61,9 +62,12 @@ def run_test_evaluate_expressions( # Expressions at breakpoint 1, which is in main self.assertEvaluate("var1", "20") # Empty expression should equate to the previous expression. - self.assertEvaluate("", "20") + if context == "variable": + self.assertEvaluate("", "20") self.assertEvaluate("var2", "21") - self.assertEvaluate("", "21") + if context == "variable": + self.assertEvaluate("", "21") + self.assertEvaluate("", "21") self.assertEvaluate("static_int", "42") self.assertEvaluate("non_static_int", "43") self.assertEvaluate("struct1.foo", "15") @@ -194,6 +198,14 @@ def run_test_evaluate_expressions( self.continue_to_next_stop() self.assertEvaluate("my_bool_vec", "size=2") + # Test memory read, especially with 'empty' repeat commands. + if context == "repl": + self.continue_to_next_stop() + self.assertEvaluate("memory read &my_ints", + ".*00 00 00 00 02 00 00 00 04 00 00 00 06 00 00 00.*\n.*08 00 00 00 0a 00 00 00 0c 00 00 00 0e 00 00 00.*\n") + self.assertEvaluate("", + ".*10 00 00 00 12 00 00 00 14 00 00 00 16 00 00 00.*\n.*18 00 00 00 1a 00 00 00 1c 00 00 00.*\n") + @skipIfWindows def test_generic_evaluate_expressions(self): # Tests context-less expression evaluations diff --git a/lldb/test/API/tools/lldb-dap/evaluate/main.cpp b/lldb/test/API/tools/lldb-dap/evaluate/main.cpp index ca27b5ba5ca19d..4bd83e2e12f16c 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/main.cpp +++ b/lldb/test/API/tools/lldb-dap/evaluate/main.cpp @@ -45,5 +45,6 @@ int main(int argc, char const *argv[]) { my_bool_vec.push_back(false); // breakpoint 6 my_bool_vec.push_back(true); // breakpoint 7 - return 0; + int my_ints[] = {0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28}; + return my_ints[0]; // breakpoint 8 } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 4220c15d3ae70d..42d621fba671da 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -205,7 +205,18 @@ struct DAP { std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; lldb::SBFormat thread_format; - std::string last_nonempty_expression; + // The next two fields are for allowing empty expressions (user just hits + // 'return') to repeat the last non-empty expression. last_expression_context + // indicates whether the last non-empty expression was for a variable or for + // a command, as we treat the two cases differently: empty commands get + // passed to the CommandInterpreter, to handle in the usual manner; empty + // variable expressions are replaced by the last nonempty expression, which + // was a variable expression (last_expression_context says so). + // last_nonempty_var_expression is either empty, if the last expression was + // a command; or it contains the last nonempty expression, which was for a + // variable evaulation. + ExpressionContext last_expression_context; + std::string last_nonempty_var_expression; DAP(); ~DAP(); diff --git a/lldb/tools/lldb-dap/LLDBUtils.cpp b/lldb/tools/lldb-dap/LLDBUtils.cpp index 2da107887604b4..60f6a36a48c3b4 100644 --- a/lldb/tools/lldb-dap/LLDBUtils.cpp +++ b/lldb/tools/lldb-dap/LLDBUtils.cpp @@ -45,7 +45,8 @@ bool RunLLDBCommands(llvm::StringRef prefix, // RunTerminateCommands. static std::mutex handle_command_mutex; std::lock_guard<std::mutex> locker(handle_command_mutex); - interp.HandleCommand(command.str().c_str(), result); + interp.HandleCommand(command.str().c_str(), result, + /* add_to_history */ true); } const bool got_error = !result.Succeeded(); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index d3728df9183aa1..322899804b764f 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1364,15 +1364,20 @@ void request_evaluate(const llvm::json::Object &request) { std::string expression = GetString(arguments, "expression").str(); llvm::StringRef context = GetString(arguments, "context"); - // Remember the last non-empty expression from the user, and use that if - // the current expression is empty (i.e. the user hit plain 'return'). - if (!expression.empty()) - g_dap.last_nonempty_expression = expression; - else - expression = g_dap.last_nonempty_expression; - - if (context == "repl" && g_dap.DetectExpressionContext(frame, expression) == - ExpressionContext::Command) { + if (context == "repl" && + ((!expression.empty() && + g_dap.DetectExpressionContext(frame, expression) == + ExpressionContext::Command) || + (expression.empty() && + g_dap.last_expression_context == ExpressionContext::Command))) { + // If the current expression is empty, and the last expression context was + // for a command, pass the empty expression along to the + // CommandInterpreter, to repeat the previous command. Also set the + // expression context properly for the next (possibly empty) expression. + g_dap.last_expression_context = ExpressionContext::Command; + // Since the current expression context is not for a variable, clear the + // last_nonempty_var_expression field. + g_dap.last_nonempty_var_expression.clear(); // If we're evaluating a command relative to the current frame, set the // focus_tid to the current frame for any thread related events. if (frame.IsValid()) { @@ -1383,6 +1388,18 @@ void request_evaluate(const llvm::json::Object &request) { EmplaceSafeString(body, "result", result); body.try_emplace("variablesReference", (int64_t)0); } else { + if (context != "hover") { + // If the expression is empty and the last expression context was for a + // variable, set the expression to the previous expression (repeat the + // evaluation); otherwise save the current non-empty expression for the + // next (possibly empty) variable expression. Also set the expression + // context for the next (possibly empty) expression. + g_dap.last_expression_context = ExpressionContext::Variable; + if (expression.empty()) + expression = g_dap.last_nonempty_var_expression; + else + g_dap.last_nonempty_var_expression = expression; + } // Always try to get the answer from the local variables if possible. If // this fails, then if the context is not "hover", actually evaluate an // expression using the expression parser. >From ebcf2bdae31cc19adac4f703ec4a7eaddbc2403e Mon Sep 17 00:00:00 2001 From: Caroline Tice <cmt...@google.com> Date: Thu, 12 Sep 2024 13:24:43 -0700 Subject: [PATCH 4/5] [lldb-dap] Add feature to remember last non-empty expression. Update test to use single-byte ints for memory reads, and to read one byte at a time. Also fix space in comment. --- .../test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py | 9 +++++---- lldb/test/API/tools/lldb-dap/evaluate/main.cpp | 5 +++-- lldb/tools/lldb-dap/lldb-dap.cpp | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py index 510f44c1d7230c..8987350ccf2f46 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py +++ b/lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py @@ -201,10 +201,11 @@ def run_test_evaluate_expressions( # Test memory read, especially with 'empty' repeat commands. if context == "repl": self.continue_to_next_stop() - self.assertEvaluate("memory read &my_ints", - ".*00 00 00 00 02 00 00 00 04 00 00 00 06 00 00 00.*\n.*08 00 00 00 0a 00 00 00 0c 00 00 00 0e 00 00 00.*\n") - self.assertEvaluate("", - ".*10 00 00 00 12 00 00 00 14 00 00 00 16 00 00 00.*\n.*18 00 00 00 1a 00 00 00 1c 00 00 00.*\n") + self.assertEvaluate("memory read -c 1 &my_ints", ".* 05 .*\n") + self.assertEvaluate("", ".* 0a .*\n") + self.assertEvaluate("", ".* 0f .*\n") + self.assertEvaluate("", ".* 14 .*\n") + self.assertEvaluate("", ".* 19 .*\n") @skipIfWindows def test_generic_evaluate_expressions(self): diff --git a/lldb/test/API/tools/lldb-dap/evaluate/main.cpp b/lldb/test/API/tools/lldb-dap/evaluate/main.cpp index 4bd83e2e12f16c..1c68716e3a6e11 100644 --- a/lldb/test/API/tools/lldb-dap/evaluate/main.cpp +++ b/lldb/test/API/tools/lldb-dap/evaluate/main.cpp @@ -1,5 +1,6 @@ #include "foo.h" +#include <cstdint> #include <map> #include <vector> @@ -45,6 +46,6 @@ int main(int argc, char const *argv[]) { my_bool_vec.push_back(false); // breakpoint 6 my_bool_vec.push_back(true); // breakpoint 7 - int my_ints[] = {0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28}; - return my_ints[0]; // breakpoint 8 + uint8_t my_ints[] = {5, 10, 15, 20, 25, 30}; + return 0; // breakpoint 8 } diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 322899804b764f..ab9aa0e66ce187 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1371,7 +1371,7 @@ void request_evaluate(const llvm::json::Object &request) { (expression.empty() && g_dap.last_expression_context == ExpressionContext::Command))) { // If the current expression is empty, and the last expression context was - // for a command, pass the empty expression along to the + // for a command, pass the empty expression along to the // CommandInterpreter, to repeat the previous command. Also set the // expression context properly for the next (possibly empty) expression. g_dap.last_expression_context = ExpressionContext::Command; >From 09bd19d85701389dcd6a1d5e30cd8a93943aa1a5 Mon Sep 17 00:00:00 2001 From: Caroline Tice <cmt...@google.com> Date: Thu, 12 Sep 2024 14:44:51 -0700 Subject: [PATCH 5/5] [lldb-dap] Add feature to remember last non-empty expression. Update to need only one new additional field in DAP structure, instead of two. --- lldb/tools/lldb-dap/DAP.h | 16 +++++----------- lldb/tools/lldb-dap/lldb-dap.cpp | 16 ++++------------ 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 42d621fba671da..dd55d26f9dadd8 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -205,17 +205,11 @@ struct DAP { std::string command_escape_prefix = "`"; lldb::SBFormat frame_format; lldb::SBFormat thread_format; - // The next two fields are for allowing empty expressions (user just hits - // 'return') to repeat the last non-empty expression. last_expression_context - // indicates whether the last non-empty expression was for a variable or for - // a command, as we treat the two cases differently: empty commands get - // passed to the CommandInterpreter, to handle in the usual manner; empty - // variable expressions are replaced by the last nonempty expression, which - // was a variable expression (last_expression_context says so). - // last_nonempty_var_expression is either empty, if the last expression was - // a command; or it contains the last nonempty expression, which was for a - // variable evaulation. - ExpressionContext last_expression_context; + // This is used to allow request_evaluate to handle empty expressions + // (ie the user pressed 'return' and expects the previous expression to + // repeat). If the previous expression was a command, this string will be + // empty; if the previous expression was a variable expression, this string + // will contain that expression. std::string last_nonempty_var_expression; DAP(); diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index ab9aa0e66ce187..3002dba51ba06a 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1368,14 +1368,8 @@ void request_evaluate(const llvm::json::Object &request) { ((!expression.empty() && g_dap.DetectExpressionContext(frame, expression) == ExpressionContext::Command) || - (expression.empty() && - g_dap.last_expression_context == ExpressionContext::Command))) { - // If the current expression is empty, and the last expression context was - // for a command, pass the empty expression along to the - // CommandInterpreter, to repeat the previous command. Also set the - // expression context properly for the next (possibly empty) expression. - g_dap.last_expression_context = ExpressionContext::Command; - // Since the current expression context is not for a variable, clear the + (expression.empty() && g_dap.last_nonempty_var_expression.empty()))) { + // Since the current expression is not for a variable, clear the // last_nonempty_var_expression field. g_dap.last_nonempty_var_expression.clear(); // If we're evaluating a command relative to the current frame, set the @@ -1389,12 +1383,10 @@ void request_evaluate(const llvm::json::Object &request) { body.try_emplace("variablesReference", (int64_t)0); } else { if (context != "hover") { - // If the expression is empty and the last expression context was for a + // If the expression is empty and the last expression was for a // variable, set the expression to the previous expression (repeat the // evaluation); otherwise save the current non-empty expression for the - // next (possibly empty) variable expression. Also set the expression - // context for the next (possibly empty) expression. - g_dap.last_expression_context = ExpressionContext::Variable; + // next (possibly empty) variable expression. if (expression.empty()) expression = g_dap.last_nonempty_var_expression; else _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits