https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/146773
>From 2674e5af817e1e6b06521c7b7f39b23150aed747 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Wed, 2 Jul 2025 13:05:31 -0700 Subject: [PATCH 1/2] [lldb-dap] Improving 'variables' hover requests. This partially fixes https://github.com/llvm/llvm-project/issues/146559. When hovering over variables while debugging with lldb-dap we are receiving hover requests that include symbols. For example, if you have: ``` MyClass *foo; ``` and hover over 'foo', the request will contain the expression `expression="*foo"` and we're evaluating the dereference, so you end up with different hover results vs the variables view. A more complete solution would be to implement an [EvaluatableExpressionProvider](https://code.visualstudio.com/api/references/vscode-api#EvaluatableExpressionProvider) in the VS Code extension. For example, if you have a declaration without any whitespace like ```c char*foo = "bar"; ``` And try to hover over 'foo', the request will be `expression="char*foo"`. --- lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py | 2 +- lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py index 55fb4a961e783..30ee7a1631e29 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -92,7 +92,7 @@ def test_readMemory(self): ) self.continue_to_next_stop() - ptr_deref = self.dap_server.request_evaluate("*rawptr")["body"] + ptr_deref = self.dap_server.request_evaluate("*rawptr", context="repl")["body"] memref = ptr_deref["memoryReference"] # We can read the complete string diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index e1556846dff19..e2161eb999e82 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp @@ -181,7 +181,17 @@ 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 + // 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. + expression = llvm::StringRef(expression).ltrim("*&").str(); } + // 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 3da6fd0aa3be2767acbb829d82d37878c9f175b5 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Mon, 7 Jul 2025 10:53:56 -0700 Subject: [PATCH 2/2] Only trim leading '&*' characters on context='hover' and ensure we only evaluate expressions when we're in the repl specifically. --- .../lldb-dap/disconnect/TestDAP_disconnect.py | 5 +++-- .../tools/lldb-dap/locations/TestDAP_locations.py | 4 ++-- .../lldb-dap/Handler/EvaluateRequestHandler.cpp | 15 +++++++++------ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py b/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py index 09e3f62f0eead..952cbe57901c4 100644 --- a/lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py +++ b/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 diff --git a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py index 45f836a2fa3c3..1355f02af7341 100644 --- a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py +++ b/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()) diff --git a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp index e2161eb999e82..26cae84756210 100644 --- a/lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp +++ b/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()) { _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
