Only trim leading '&*' characters on context='hover' and ensure we only evaluate expressions when we're in the repl specifically.

Author: ashgti

lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py

@@ -2,7 +2,6 @@
 Test lldb-dap disconnect request
 """
 
-
 import dap_server
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -71,7 +70,9 @@ def test_attach(self):
         lldbutil.wait_for_file_on_target(self, sync_file_path)
 
         self.attach(pid=self.process.pid, disconnectAutomatically=False)
-        response = self.dap_server.request_evaluate("wait_for_attach = false;")
+        response = self.dap_server.request_evaluate(
+            "`expr -- wait_for_attach = false;", context="repl"
+        )
         self.assertTrue(response["success"])
 
         # verify we haven't produced the side effect file yet

lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py

@@ -76,6 +76,6 @@ def test_locations(self):
         self.assertEqual(val_loc_func_ref["body"]["line"], 3)
 
         # `evaluate` responses for function pointers also have locations associated
-        eval_res = self.dap_server.request_evaluate("greet")
-        self.assertTrue(eval_res["success"])
+        eval_res = self.dap_server.request_evaluate("greet", context="repl")
+        self.assertTrue(eval_res["success"], f"evaluate failed: {eval_res}")
         self.assertIn("valueLocationReference", eval_res["body"].keys())

lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp

@@ -181,14 +181,14 @@ void EvaluateRequestHandler::operator()(
         expression = dap.last_nonempty_var_expression;
       else
         dap.last_nonempty_var_expression = expression;
-    } else {
-      // If this isn't a REPL context, trim leading pointer/reference characters
+    } else if (context == "hover") {
+      // If we're in the hover context trim leading pointer/reference characters
       // to ensure we return the actual value of the expression.
       // This can come up if you hover over a pointer or reference declaration
       // like 'MyType *foo;' or `void fn(std::string &arg)`, which results in
-      // the hover request sending '*foo' or `&arg`. When we're not in the REPL,
-      // we should trim these characters to get to the actual variable, which
-      // should have the proper type encoded by the compiler.
+      // the hover request sending '*foo' or `&arg`. Trim these characters to
+      // get to the actual variable, which should have the proper type encoded
+      // by the compiler.
       expression = llvm::StringRef(expression).ltrim("*&").str();
     }
 
@@ -205,7 +205,10 @@ void EvaluateRequestHandler::operator()(
     if (value.GetError().Success() && context == "repl")
       value = value.Persist();
 
-    if (value.GetError().Fail() && context != "hover")
+    // Only the repl should evaluate expressions, which can mutate application
+    // state. Other contexts are used for observing debuggee state only, not
+    // mutating state.
+    if (value.GetError().Fail() && context == "repl")
       value = frame.EvaluateExpression(expression.data());
 
     if (value.GetError().Fail()) {