[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 13:26:17 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/4] [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/4] [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/4] [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/4] [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;



More information about the lldb-commits mailing list