[Lldb-commits] [lldb] [lldb-dap] Implement value locations for function pointers (PR #104589)

Adrian Vogelsgesang via lldb-commits lldb-commits at lists.llvm.org
Sat Sep 21 16:20:36 PDT 2024


https://github.com/vogelsgesang updated https://github.com/llvm/llvm-project/pull/104589

>From a7876a20b5caa53dace056c84a1b7f1e798088f0 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Mon, 12 Aug 2024 14:53:31 +0000
Subject: [PATCH 1/4] [lldb-dap] Implement value locations for function
 pointers

This commit adds `valueLocationReference` to function pointers and
function references. Thereby, users can navigate directly to the
pointed-to function from within the "variables" pane.

In general, it would be useful to also a similar location references
also to member function pointers, `std::source_location`,
`std::function`,  and many more. Doing so would require extending the
formatters to provide such a source code location.

There were two RFCs about this a while ago:
https://discourse.llvm.org/t/rfc-extending-formatters-with-a-source-code-reference/68375
https://discourse.llvm.org/t/rfc-sbvalue-metadata-provider/68377/26

However, both RFCs ended without a conclusion. As such, this commit now
solve the lowest-hanging fruit, i.e. function pointers. If people find
it useful, I will revive the RFC afterwards.
---
 .../API/tools/lldb-dap/locations/Makefile     |   2 +-
 .../lldb-dap/locations/TestDAP_locations.py   |  49 +++++++-
 lldb/test/API/tools/lldb-dap/locations/main.c |   5 -
 .../API/tools/lldb-dap/locations/main.cpp     |  11 ++
 lldb/tools/lldb-dap/JSONUtils.cpp             |  33 ++++-
 lldb/tools/lldb-dap/JSONUtils.h               |   3 +
 lldb/tools/lldb-dap/lldb-dap.cpp              | 113 +++++++++++++-----
 7 files changed, 176 insertions(+), 40 deletions(-)
 delete mode 100644 lldb/test/API/tools/lldb-dap/locations/main.c
 create mode 100644 lldb/test/API/tools/lldb-dap/locations/main.cpp

diff --git a/lldb/test/API/tools/lldb-dap/locations/Makefile b/lldb/test/API/tools/lldb-dap/locations/Makefile
index 10495940055b63..99998b20bcb050 100644
--- a/lldb/test/API/tools/lldb-dap/locations/Makefile
+++ b/lldb/test/API/tools/lldb-dap/locations/Makefile
@@ -1,3 +1,3 @@
-C_SOURCES := main.c
+CXX_SOURCES := main.cpp
 
 include Makefile.rules
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 76d938d3908492..45f836a2fa3c39 100644
--- a/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
+++ b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
@@ -19,11 +19,11 @@ def test_locations(self):
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program)
-        source = "main.c"
+        source = "main.cpp"
         self.source_path = os.path.join(os.getcwd(), source)
         self.set_source_breakpoints(
             source,
-            [line_number(source, "// BREAK HERE")],
+            [line_number(source, "break here")],
         )
         self.continue_to_next_stop()
 
@@ -36,5 +36,46 @@ def test_locations(self):
             locals["var1"]["declarationLocationReference"]
         )
         self.assertTrue(loc_var1["success"])
-        self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.c"))
-        self.assertEqual(loc_var1["body"]["line"], 2)
+        self.assertTrue(loc_var1["body"]["source"]["path"].endswith("main.cpp"))
+        self.assertEqual(loc_var1["body"]["line"], 6)
+
+        # func_ptr has both a declaration and a valueLocation
+        self.assertIn("declarationLocationReference", locals["func_ptr"].keys())
+        self.assertIn("valueLocationReference", locals["func_ptr"].keys())
+        decl_loc_func_ptr = self.dap_server.request_locations(
+            locals["func_ptr"]["declarationLocationReference"]
+        )
+        self.assertTrue(decl_loc_func_ptr["success"])
+        self.assertTrue(
+            decl_loc_func_ptr["body"]["source"]["path"].endswith("main.cpp")
+        )
+        self.assertEqual(decl_loc_func_ptr["body"]["line"], 7)
+        val_loc_func_ptr = self.dap_server.request_locations(
+            locals["func_ptr"]["valueLocationReference"]
+        )
+        self.assertTrue(val_loc_func_ptr["success"])
+        self.assertTrue(val_loc_func_ptr["body"]["source"]["path"].endswith("main.cpp"))
+        self.assertEqual(val_loc_func_ptr["body"]["line"], 3)
+
+        # func_ref has both a declaration and a valueLocation
+        self.assertIn("declarationLocationReference", locals["func_ref"].keys())
+        self.assertIn("valueLocationReference", locals["func_ref"].keys())
+        decl_loc_func_ref = self.dap_server.request_locations(
+            locals["func_ref"]["declarationLocationReference"]
+        )
+        self.assertTrue(decl_loc_func_ref["success"])
+        self.assertTrue(
+            decl_loc_func_ref["body"]["source"]["path"].endswith("main.cpp")
+        )
+        self.assertEqual(decl_loc_func_ref["body"]["line"], 8)
+        val_loc_func_ref = self.dap_server.request_locations(
+            locals["func_ref"]["valueLocationReference"]
+        )
+        self.assertTrue(val_loc_func_ref["success"])
+        self.assertTrue(val_loc_func_ref["body"]["source"]["path"].endswith("main.cpp"))
+        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"])
+        self.assertIn("valueLocationReference", eval_res["body"].keys())
diff --git a/lldb/test/API/tools/lldb-dap/locations/main.c b/lldb/test/API/tools/lldb-dap/locations/main.c
deleted file mode 100644
index 6a8c86d00cb562..00000000000000
--- a/lldb/test/API/tools/lldb-dap/locations/main.c
+++ /dev/null
@@ -1,5 +0,0 @@
-int main(void) {
-  int var1 = 1;
-  // BREAK HERE
-  return 0;
-}
diff --git a/lldb/test/API/tools/lldb-dap/locations/main.cpp b/lldb/test/API/tools/lldb-dap/locations/main.cpp
new file mode 100644
index 00000000000000..e3b2e73f9837f8
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/main.cpp
@@ -0,0 +1,11 @@
+#include <cstdio>
+
+void greet() { printf("Hello"); }
+
+int main(void) {
+  int var1 = 1;
+  void (*func_ptr)() = &greet;
+  void (&func_ref)() = greet;
+  __builtin_printf("break here");
+  return 0;
+}
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4f9c9c01cf4b6b..1d3fc110791d2f 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1198,6 +1198,18 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
   return description.trim().str();
 }
 
+bool HasValueLocation(lldb::SBValue v) {
+  if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType()) {
+    return false;
+  }
+
+  lldb::addr_t addr = v.GetValueAsAddress();
+  lldb::SBLineEntry line_entry =
+      g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
+
+  return line_entry.IsValid();
+}
+
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable
@@ -1277,6 +1289,18 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
 //                       Object References' in the Overview section for
 //                       details."
 //     },
+//     "valueLocationReference": {
+//       "type": "integer",
+//       "description": "A reference that allows the client to request the
+//                       location where the variable's value is declared. For
+//                       example, if the variable contains a function pointer,
+//                       the adapter may be able to look up the function's
+//                       location. This should be present only if the adapter
+//                       is likely to be able to resolve the location.\n\nThis
+//                       reference shares the same lifetime as the
+//                       `variablesReference`. See 'Lifetime of Object
+//                       References' in the Overview section for details."
+//     },
 //
 //     "$__lldb_extensions": {
 //       "description": "Unofficial extensions to the protocol",
@@ -1390,7 +1414,10 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
     object.try_emplace("variablesReference", 0);
 
   if (v.GetDeclaration().IsValid())
-    object.try_emplace("declarationLocationReference", var_ref);
+    object.try_emplace("declarationLocationReference", var_ref << 1);
+
+  if (HasValueLocation(v))
+    object.try_emplace("valueLocationReference", (var_ref << 1) | 1);
 
   if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
     object.try_emplace("memoryReference", EncodeMemoryReference(addr));
@@ -1416,8 +1443,8 @@ CreateRunInTerminalReverseRequest(const llvm::json::Object &launch_request,
                                   llvm::StringRef comm_file,
                                   lldb::pid_t debugger_pid) {
   llvm::json::Object run_in_terminal_args;
-  // This indicates the IDE to open an embedded terminal, instead of opening the
-  // terminal in a new window.
+  // This indicates the IDE to open an embedded terminal, instead of opening
+  // the terminal in a new window.
   run_in_terminal_args.try_emplace("kind", "integrated");
 
   auto launch_request_arguments = launch_request.getObject("arguments");
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 13018458ffe0ad..0e00c0e6bcb8c5 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -458,6 +458,9 @@ struct VariableDescription {
   std::string GetResult(llvm::StringRef context);
 };
 
+/// Does the given variable have an associated value location?
+bool HasValueLocation(lldb::SBValue v);
+
 /// Create a "Variable" object for a LLDB thread object.
 ///
 /// This function will fill in the following keys in the returned
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 2fb86f675b4516..caf2d31bc92b1d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1542,6 +1542,19 @@ void request_completions(const llvm::json::Object &request) {
 //                              client can use this optional information to
 //                              present the variables in a paged UI and fetch
 //                              them in chunks."
+//            },
+//            "valueLocationReference": {
+//              "type": "integer",
+//              "description": "A reference that allows the client to request
+//                              the location where the returned value is
+//                              declared. For example, if a function pointer is
+//                              returned, the adapter may be able to look up the
+//                              function's location. This should be present only
+//                              if the adapter is likely to be able to resolve
+//                              the location.\n\nThis reference shares the same
+//                              lifetime as the `variablesReference`. See
+//                              'Lifetime of Object References' in the
+//              Overview section for details."
 //            }
 //            "memoryReference": {
 //               "type": "string",
@@ -1611,16 +1624,17 @@ void request_evaluate(const llvm::json::Object &request) {
       VariableDescription desc(value);
       EmplaceSafeString(body, "result", desc.GetResult(context));
       EmplaceSafeString(body, "type", desc.display_type_name);
-      if (value.MightHaveChildren()) {
-        auto variableReference = g_dap.variables.InsertVariable(
-            value, /*is_permanent=*/context == "repl");
-        body.try_emplace("variablesReference", variableReference);
-      } else {
+      auto var_ref = g_dap.variables.InsertVariable(
+          value, /*is_permanent=*/context == "repl");
+      if (value.MightHaveChildren())
+        body.try_emplace("variablesReference", var_ref);
+      else
         body.try_emplace("variablesReference", (int64_t)0);
-      }
       if (lldb::addr_t addr = value.GetLoadAddress();
           addr != LLDB_INVALID_ADDRESS)
         body.try_emplace("memoryReference", EncodeMemoryReference(addr));
+      if (HasValueLocation(value))
+        body.try_emplace("valueLocationReference", var_ref);
     }
   }
   response.try_emplace("body", std::move(body));
@@ -3735,6 +3749,17 @@ void request_threads(const llvm::json::Object &request) {
 //             "description": "The number of indexed child variables. The client
 //             can use this optional information to present the variables in a
 //             paged UI and fetch them in chunks."
+//           },
+//           "valueLocationReference": {
+//             "type": "integer",
+//             "description": "A reference that allows the client to request the
+//             location where the new value is declared. For example, if the new
+//             value is function pointer, the adapter may be able to look up the
+//             function's location. This should be present only if the adapter
+//             is likely to be able to resolve the location.\n\nThis reference
+//             shares the same lifetime as the `variablesReference`. See
+//             'Lifetime of Object References' in the Overview section for
+//             details."
 //           }
 //         },
 //         "required": [ "value" ]
@@ -3759,7 +3784,6 @@ void request_setVariable(const llvm::json::Object &request) {
   response.try_emplace("success", false);
 
   lldb::SBValue variable;
-  int64_t newVariablesReference = 0;
 
   // The "id" is the unique integer ID that is unique within the enclosing
   // variablesReference. It is optionally added to any "interface Variable"
@@ -3789,14 +3813,17 @@ void request_setVariable(const llvm::json::Object &request) {
       // so always insert a new one to get its variablesReference.
       // is_permanent is false because debug console does not support
       // setVariable request.
+      int64_t new_var_ref =
+          g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
       if (variable.MightHaveChildren())
-        newVariablesReference =
-            g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
-      body.try_emplace("variablesReference", newVariablesReference);
-
+        body.try_emplace("variablesReference", new_var_ref);
+      else
+        body.try_emplace("variablesReference", 0);
       if (lldb::addr_t addr = variable.GetLoadAddress();
           addr != LLDB_INVALID_ADDRESS)
         body.try_emplace("memoryReference", EncodeMemoryReference(addr));
+      if (HasValueLocation(variable))
+        body.try_emplace("valueLocationReference", new_var_ref);
     } else {
       EmplaceSafeString(body, "message", std::string(error.GetCString()));
     }
@@ -4087,10 +4114,13 @@ void request_variables(const llvm::json::Object &request) {
 void request_locations(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
-  auto arguments = request.getObject("arguments");
+  auto *arguments = request.getObject("arguments");
 
   uint64_t reference_id = GetUnsigned(arguments, "locationReference", 0);
-  lldb::SBValue variable = g_dap.variables.GetVariable(reference_id);
+  // We use the lowest bit to distinguish between value location and declaration
+  // location
+  bool isValueLocation = reference_id & 1;
+  lldb::SBValue variable = g_dap.variables.GetVariable(reference_id >> 1);
   if (!variable.IsValid()) {
     response["success"] = false;
     response["message"] = "Invalid variable reference";
@@ -4098,21 +4128,50 @@ void request_locations(const llvm::json::Object &request) {
     return;
   }
 
-  // Get the declaration location
-  lldb::SBDeclaration decl = variable.GetDeclaration();
-  if (!decl.IsValid()) {
-    response["success"] = false;
-    response["message"] = "No declaration location available";
-    g_dap.SendJSON(llvm::json::Value(std::move(response)));
-    return;
-  }
-
   llvm::json::Object body;
-  body.try_emplace("source", CreateSource(decl.GetFileSpec()));
-  if (int line = decl.GetLine())
-    body.try_emplace("line", line);
-  if (int column = decl.GetColumn())
-    body.try_emplace("column", column);
+  if (isValueLocation) {
+    // Get the value location
+    if (!variable.GetType().IsPointerType() &&
+        !variable.GetType().IsReferenceType()) {
+      response["success"] = false;
+      response["message"] =
+          "Value locations are only available for pointers and references";
+      g_dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+
+    lldb::addr_t addr = variable.GetValueAsAddress();
+    lldb::SBLineEntry line_entry =
+        g_dap.target.ResolveLoadAddress(addr).GetLineEntry();
+
+    if (!line_entry.IsValid()) {
+      response["success"] = false;
+      response["message"] = "Failed to resolve line entry for location";
+      g_dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+
+    body.try_emplace("source", CreateSource(line_entry.GetFileSpec()));
+    if (int line = line_entry.GetLine())
+      body.try_emplace("line", line);
+    if (int column = line_entry.GetColumn())
+      body.try_emplace("column", column);
+  } else {
+    // Get the declaration location
+    lldb::SBDeclaration decl = variable.GetDeclaration();
+    if (!decl.IsValid()) {
+      response["success"] = false;
+      response["message"] = "No declaration location available";
+      g_dap.SendJSON(llvm::json::Value(std::move(response)));
+      return;
+    }
+
+    body.try_emplace("source", CreateSource(decl.GetFileSpec()));
+    if (int line = decl.GetLine())
+      body.try_emplace("line", line);
+    if (int column = decl.GetColumn())
+      body.try_emplace("column", column);
+  }
 
   response.try_emplace("body", std::move(body));
   g_dap.SendJSON(llvm::json::Value(std::move(response)));

>From 344965b1b61c24382ddb41375c241c36f855e0f7 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Wed, 18 Sep 2024 14:30:13 +0000
Subject: [PATCH 2/4] Address comments

---
 lldb/tools/lldb-dap/JSONUtils.cpp | 12 ++++++++++--
 lldb/tools/lldb-dap/JSONUtils.h   |  7 +++++++
 lldb/tools/lldb-dap/lldb-dap.cpp  | 12 +++++++-----
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 1d3fc110791d2f..1a011a50549d9d 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1210,6 +1210,14 @@ bool HasValueLocation(lldb::SBValue v) {
   return line_entry.IsValid();
 }
 
+int64_t PackLocation(int64_t var_ref, bool is_value_location) {
+   return var_ref << 1 | is_value_location;
+}
+
+std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
+   return std::pair{ location_id >> 1, location_id & 1};
+}
+
 // "Variable": {
 //   "type": "object",
 //   "description": "A Variable is a name/value pair. Optionally a variable
@@ -1414,10 +1422,10 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
     object.try_emplace("variablesReference", 0);
 
   if (v.GetDeclaration().IsValid())
-    object.try_emplace("declarationLocationReference", var_ref << 1);
+    object.try_emplace("declarationLocationReference", PackLocation(var_ref, false));
 
   if (HasValueLocation(v))
-    object.try_emplace("valueLocationReference", (var_ref << 1) | 1);
+    object.try_emplace("valueLocationReference", PackLocation(var_ref, true));
 
   if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
     object.try_emplace("memoryReference", EncodeMemoryReference(addr));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 0e00c0e6bcb8c5..a28b1b20cbaf01 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -461,6 +461,13 @@ struct VariableDescription {
 /// Does the given variable have an associated value location?
 bool HasValueLocation(lldb::SBValue v);
 
+/// Pack a location into a single integer which we can send via
+/// the debug adapter protocol.
+int64_t PackLocation(int64_t var_ref, bool is_value_location);
+
+/// Reverse of `PackLocation`
+std::pair<int64_t, bool> UnpackLocation(int64_t location_id);
+
 /// Create a "Variable" object for a LLDB thread object.
 ///
 /// This function will fill in the following keys in the returned
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index caf2d31bc92b1d..6489ca234dc8d5 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1624,7 +1624,9 @@ void request_evaluate(const llvm::json::Object &request) {
       VariableDescription desc(value);
       EmplaceSafeString(body, "result", desc.GetResult(context));
       EmplaceSafeString(body, "type", desc.display_type_name);
-      auto var_ref = g_dap.variables.InsertVariable(
+      int64_t var_ref = 0;
+      if (value.MightHaveChildren() || HasValueLocation(value))
+         var_ref = g_dap.variables.InsertVariable(
           value, /*is_permanent=*/context == "repl");
       if (value.MightHaveChildren())
         body.try_emplace("variablesReference", var_ref);
@@ -4116,11 +4118,11 @@ void request_locations(const llvm::json::Object &request) {
   FillResponse(request, response);
   auto *arguments = request.getObject("arguments");
 
-  uint64_t reference_id = GetUnsigned(arguments, "locationReference", 0);
+  uint64_t location_id = GetUnsigned(arguments, "locationReference", 0);
   // We use the lowest bit to distinguish between value location and declaration
   // location
-  bool isValueLocation = reference_id & 1;
-  lldb::SBValue variable = g_dap.variables.GetVariable(reference_id >> 1);
+  auto [var_ref, is_value_location] = UnpackLocation(location_id);
+  lldb::SBValue variable = g_dap.variables.GetVariable(var_ref);
   if (!variable.IsValid()) {
     response["success"] = false;
     response["message"] = "Invalid variable reference";
@@ -4129,7 +4131,7 @@ void request_locations(const llvm::json::Object &request) {
   }
 
   llvm::json::Object body;
-  if (isValueLocation) {
+  if (is_value_location) {
     // Get the value location
     if (!variable.GetType().IsPointerType() &&
         !variable.GetType().IsReferenceType()) {

>From 3b81f768c50be339d8412d5452ece9d190a84a58 Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Wed, 18 Sep 2024 21:19:00 +0000
Subject: [PATCH 3/4] Address comments (2)

---
 lldb/test/API/tools/lldb-dap/locations/main.cpp | 3 +--
 lldb/tools/lldb-dap/JSONUtils.cpp               | 7 +++----
 lldb/tools/lldb-dap/JSONUtils.h                 | 2 +-
 lldb/tools/lldb-dap/lldb-dap.cpp                | 6 +++---
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/lldb/test/API/tools/lldb-dap/locations/main.cpp b/lldb/test/API/tools/lldb-dap/locations/main.cpp
index e3b2e73f9837f8..fb7789ffd86fdf 100644
--- a/lldb/test/API/tools/lldb-dap/locations/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/locations/main.cpp
@@ -6,6 +6,5 @@ int main(void) {
   int var1 = 1;
   void (*func_ptr)() = &greet;
   void (&func_ref)() = greet;
-  __builtin_printf("break here");
-  return 0;
+  return 0; // break here
 }
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 1a011a50549d9d..7f77f357e25400 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1198,10 +1198,9 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
   return description.trim().str();
 }
 
-bool HasValueLocation(lldb::SBValue v) {
-  if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType()) {
+bool ValuePointsToCode(lldb::SBValue v) {
+  if (!v.GetType().GetPointeeType().IsFunctionType())
     return false;
-  }
 
   lldb::addr_t addr = v.GetValueAsAddress();
   lldb::SBLineEntry line_entry =
@@ -1424,7 +1423,7 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
   if (v.GetDeclaration().IsValid())
     object.try_emplace("declarationLocationReference", PackLocation(var_ref, false));
 
-  if (HasValueLocation(v))
+  if (ValuePointsToCode(v))
     object.try_emplace("valueLocationReference", PackLocation(var_ref, true));
 
   if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index a28b1b20cbaf01..aa3df62ebef7fd 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -459,7 +459,7 @@ struct VariableDescription {
 };
 
 /// Does the given variable have an associated value location?
-bool HasValueLocation(lldb::SBValue v);
+bool ValuePointsToCode(lldb::SBValue v);
 
 /// Pack a location into a single integer which we can send via
 /// the debug adapter protocol.
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 6489ca234dc8d5..fda766235a6b59 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1625,7 +1625,7 @@ void request_evaluate(const llvm::json::Object &request) {
       EmplaceSafeString(body, "result", desc.GetResult(context));
       EmplaceSafeString(body, "type", desc.display_type_name);
       int64_t var_ref = 0;
-      if (value.MightHaveChildren() || HasValueLocation(value))
+      if (value.MightHaveChildren() || ValuePointsToCode(value))
          var_ref = g_dap.variables.InsertVariable(
           value, /*is_permanent=*/context == "repl");
       if (value.MightHaveChildren())
@@ -1635,7 +1635,7 @@ void request_evaluate(const llvm::json::Object &request) {
       if (lldb::addr_t addr = value.GetLoadAddress();
           addr != LLDB_INVALID_ADDRESS)
         body.try_emplace("memoryReference", EncodeMemoryReference(addr));
-      if (HasValueLocation(value))
+      if (ValuePointsToCode(value))
         body.try_emplace("valueLocationReference", var_ref);
     }
   }
@@ -3824,7 +3824,7 @@ void request_setVariable(const llvm::json::Object &request) {
       if (lldb::addr_t addr = variable.GetLoadAddress();
           addr != LLDB_INVALID_ADDRESS)
         body.try_emplace("memoryReference", EncodeMemoryReference(addr));
-      if (HasValueLocation(variable))
+      if (ValuePointsToCode(variable))
         body.try_emplace("valueLocationReference", new_var_ref);
     } else {
       EmplaceSafeString(body, "message", std::string(error.GetCString()));

>From 06de2dfed86e1b6f4508d71664f2f41e3068ac1d Mon Sep 17 00:00:00 2001
From: Adrian Vogelsgesang <avogelsgesang at salesforce.com>
Date: Sat, 21 Sep 2024 22:53:30 +0000
Subject: [PATCH 4/4] code formatting

---
 lldb/tools/lldb-dap/JSONUtils.cpp | 7 ++++---
 lldb/tools/lldb-dap/lldb-dap.cpp  | 4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 7f77f357e25400..d55742e44bd17c 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -1210,11 +1210,11 @@ bool ValuePointsToCode(lldb::SBValue v) {
 }
 
 int64_t PackLocation(int64_t var_ref, bool is_value_location) {
-   return var_ref << 1 | is_value_location;
+  return var_ref << 1 | is_value_location;
 }
 
 std::pair<int64_t, bool> UnpackLocation(int64_t location_id) {
-   return std::pair{ location_id >> 1, location_id & 1};
+  return std::pair{location_id >> 1, location_id & 1};
 }
 
 // "Variable": {
@@ -1421,7 +1421,8 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
     object.try_emplace("variablesReference", 0);
 
   if (v.GetDeclaration().IsValid())
-    object.try_emplace("declarationLocationReference", PackLocation(var_ref, false));
+    object.try_emplace("declarationLocationReference",
+                       PackLocation(var_ref, false));
 
   if (ValuePointsToCode(v))
     object.try_emplace("valueLocationReference", PackLocation(var_ref, true));
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index fda766235a6b59..fadfc971396e53 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1626,8 +1626,8 @@ void request_evaluate(const llvm::json::Object &request) {
       EmplaceSafeString(body, "type", desc.display_type_name);
       int64_t var_ref = 0;
       if (value.MightHaveChildren() || ValuePointsToCode(value))
-         var_ref = g_dap.variables.InsertVariable(
-          value, /*is_permanent=*/context == "repl");
+        var_ref = g_dap.variables.InsertVariable(
+            value, /*is_permanent=*/context == "repl");
       if (value.MightHaveChildren())
         body.try_emplace("variablesReference", var_ref);
       else



More information about the lldb-commits mailing list