[Lldb-commits] [lldb] [lldb-dap] Add feature to remember last non-empty expression. (PR #107485)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 12 14:46:45 PDT 2024
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 <cmtice at 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 <cmtice at 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 <cmtice at 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 <cmtice at 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 <cmtice at 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
More information about the lldb-commits
mailing list