[lldb-dap] Provide declarationLocation for variables
#102928
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@walter-erquinigo I took a first stab at implementing the DAP proposal. Seemed rather straightforward. I would love to hear if I missed anything. Also, I am not sure how to implement |
|
@vogelsgesang , Mr. Bird, I'm swamped this week with work, but I'll look into this before saturday for sure. |
|
No worries. I will have to rework this commit anyway. In the meantime, a proposal was merged upstream and also already implemented in VS-Code. However, there were still larger changes which will require a complete rewrite of this commit |
vogelsgesang
force-pushed
the
avogelsgesang-lldb-dap-declaration-location
branch
from
5c3b345 to
a740c69
Compare
|
@llvm/pr-subscribers-lldb Author: Adrian Vogelsgesang (vogelsgesang) ChangesThis commit implements support for the "declaration location" recently added by microsoft/debug-adapter-protocol#494 to the debug adapter protocol. For the While I tested this against VS-Code Insiders. However, VS-Code Insiders currently only supports Patch is 24.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/102928.diff 9 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index a324af57b61df3..9879a34ed2020c 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -1079,6 +1079,17 @@ def request_setVariable(self, containingVarRef, name, value, id=None):
}
return self.send_recv(command_dict)
+ def request_locations(self, locationReference):
+ args_dict = {
+ "locationReference": locationReference,
+ }
+ command_dict = {
+ "command": "locations",
+ "type": "request",
+ "arguments": args_dict,
+ }
+ return self.send_recv(command_dict)
+
def request_testGetTargetBreakpoints(self):
"""A request packet used in the LLDB test suite to get all currently
set breakpoint infos for all breakpoints currently set in the
diff --git a/lldb/test/API/tools/lldb-dap/locations/Makefile b/lldb/test/API/tools/lldb-dap/locations/Makefile
new file mode 100644
index 00000000000000..10495940055b63
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := main.c
+
+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
new file mode 100644
index 00000000000000..26feb789db39d6
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/TestDAP_locations.py
@@ -0,0 +1,38 @@
+"""
+Test lldb-dap locations request
+"""
+
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbdap_testcase
+import os
+
+
+class TestDAP_locations(lldbdap_testcase.DAPTestCaseBase):
+ @skipIfWindows
+ def test_locations(self):
+ """
+ Tests the 'locations' request.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.c"
+ self.source_path = os.path.join(os.getcwd(), source)
+ self.set_source_breakpoints(
+ source,
+ [line_number(source, "// BREAK HERE")],
+ )
+ self.continue_to_next_stop()
+
+ locals = {l["name"]: l for l in self.dap_server.get_local_variables()}
+
+ # var1 has a declarationLocation but no valueLocation
+ self.assertIn("declarationLocationReference", locals["var1"].keys())
+ self.assertNotIn("valueLocationReference", locals["var1"].keys())
+ loc_var1 = self.dap_server.request_locations(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)
diff --git a/lldb/test/API/tools/lldb-dap/locations/main.c b/lldb/test/API/tools/lldb-dap/locations/main.c
new file mode 100644
index 00000000000000..6a8c86d00cb562
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/locations/main.c
@@ -0,0 +1,5 @@
+int main(void) {
+ int var1 = 1;
+ // BREAK HERE
+ return 0;
+}
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index c3c70e9d739846..baf02e5b35683a 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -813,7 +813,7 @@ void Variables::Clear() {
locals.Clear();
globals.Clear();
registers.Clear();
- expandable_variables.clear();
+ referenced_variables.clear();
}
int64_t Variables::GetNewVariableReference(bool is_permanent) {
@@ -828,24 +828,24 @@ bool Variables::IsPermanentVariableReference(int64_t var_ref) {
lldb::SBValue Variables::GetVariable(int64_t var_ref) const {
if (IsPermanentVariableReference(var_ref)) {
- auto pos = expandable_permanent_variables.find(var_ref);
- if (pos != expandable_permanent_variables.end())
+ auto pos = referenced_permanent_variables.find(var_ref);
+ if (pos != referenced_permanent_variables.end())
return pos->second;
} else {
- auto pos = expandable_variables.find(var_ref);
- if (pos != expandable_variables.end())
+ auto pos = referenced_variables.find(var_ref);
+ if (pos != referenced_variables.end())
return pos->second;
}
return lldb::SBValue();
}
-int64_t Variables::InsertExpandableVariable(lldb::SBValue variable,
- bool is_permanent) {
+int64_t Variables::InsertVariable(lldb::SBValue variable,
+ bool is_permanent) {
int64_t var_ref = GetNewVariableReference(is_permanent);
if (is_permanent)
- expandable_permanent_variables.insert(std::make_pair(var_ref, variable));
+ referenced_permanent_variables.insert(std::make_pair(var_ref, variable));
else
- expandable_variables.insert(std::make_pair(var_ref, variable));
+ referenced_variables.insert(std::make_pair(var_ref, variable));
return var_ref;
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 57562a14983519..848e69145da4f6 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -104,12 +104,12 @@ struct Variables {
int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
int64_t next_permanent_var_ref{PermanentVariableStartIndex};
- /// Expandable variables that are alive in this stop state.
+ /// Variables that are alive in this stop state.
/// Will be cleared when debuggee resumes.
- llvm::DenseMap<int64_t, lldb::SBValue> expandable_variables;
- /// Expandable variables that persist across entire debug session.
+ llvm::DenseMap<int64_t, lldb::SBValue> referenced_variables;
+ /// Variables that persist across entire debug session.
/// These are the variables evaluated from debug console REPL.
- llvm::DenseMap<int64_t, lldb::SBValue> expandable_permanent_variables;
+ llvm::DenseMap<int64_t, lldb::SBValue> referenced_permanent_variables;
/// Check if \p var_ref points to a variable that should persist for the
/// entire duration of the debug session, e.g. repl expandable variables
@@ -127,7 +127,7 @@ struct Variables {
/// Insert a new \p variable.
/// \return variableReference assigned to this expandable variable.
- int64_t InsertExpandableVariable(lldb::SBValue variable, bool is_permanent);
+ int64_t InsertVariable(lldb::SBValue variable, bool is_permanent);
/// Clear all scope variables and non-permanent expandable variables.
void Clear();
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index a8b85f55939e17..10e6b1fc6d9abe 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -614,9 +614,8 @@ CreateExceptionBreakpointFilter(const ExceptionBreakpoint &bp) {
// }
// }
// }
-llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
+llvm::json::Value CreateSource(const lldb::SBFileSpec &file) {
llvm::json::Object object;
- lldb::SBFileSpec file = line_entry.GetFileSpec();
if (file.IsValid()) {
const char *name = file.GetFilename();
if (name)
@@ -630,6 +629,10 @@ llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry) {
return llvm::json::Value(std::move(object));
}
+llvm::json::Value CreateSource(const lldb::SBLineEntry &line_entry) {
+ return CreateSource(line_entry.GetFileSpec());
+}
+
llvm::json::Value CreateSource(llvm::StringRef source_path) {
llvm::json::Object source;
llvm::StringRef name = llvm::sys::path::filename(source_path);
@@ -1143,65 +1146,75 @@ std::string VariableDescription::GetResult(llvm::StringRef context) {
// "description": "The number of indexed child variables. The client
// can use this optional information to present the
// children in a paged UI and fetch them in chunks."
-// }
-//
+// },
+// "declarationLocationReference": {
+// "type": "integer",
+// "description": "A reference that allows the client to request the
+// location where the variable is declared. 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",
// "properties": {
// "declaration": {
-// "type": "object",
-// "description": "The source location where the variable was declared.
-// This value won't be present if no declaration is
-// available.",
-// "properties": {
-// "path": {
-// "type": "string",
-// "description": "The source file path where the variable was
-// declared."
-// },
-// "line": {
-// "type": "number",
-// "description": "The 1-indexed source line where the variable was
-// declared."
-// },
-// "column": {
-// "type": "number",
-// "description": "The 1-indexed source column where the variable
-// was declared."
+// "type": "object",
+// "description": "The source location where the variable was declared.
+// This value won't be present if no declaration is
+// available.
+// Superseded by `declarationLocationReference`",
+// "properties": {
+// "path": {
+// "type": "string",
+// "description": "The source file path where the variable was
+// declared."
+// },
+// "line": {
+// "type": "number",
+// "description": "The 1-indexed source line where the variable
+// was declared."
+// },
+// "column": {
+// "type": "number",
+// "description": "The 1-indexed source column where the variable
+// was declared."
+// }
// }
+// },
+// "value": {
+// "type": "string",
+// "description": "The internal value of the variable as returned by
+// This is effectively SBValue.GetValue(). The other
+// `value` entry in the top-level variable response
+// is, on the other hand, just a display string for
+// the variable."
+// },
+// "summary": {
+// "type": "string",
+// "description": "The summary string of the variable. This is
+// effectively SBValue.GetSummary()."
+// },
+// "autoSummary": {
+// "type": "string",
+// "description": "The auto generated summary if using
+// `enableAutoVariableSummaries`."
+// },
+// "error": {
+// "type": "string",
+// "description": "An error message generated if LLDB couldn't inspect
+// the variable."
// }
-// },
-// "value":
-// "type": "string",
-// "description": "The internal value of the variable as returned by
-// This is effectively SBValue.GetValue(). The other
-// `value` entry in the top-level variable response is,
-// on the other hand, just a display string for the
-// variable."
-// },
-// "summary":
-// "type": "string",
-// "description": "The summary string of the variable. This is
-// effectively SBValue.GetSummary()."
-// },
-// "autoSummary":
-// "type": "string",
-// "description": "The auto generated summary if using
-// `enableAutoVariableSummaries`."
-// },
-// "error":
-// "type": "string",
-// "description": "An error message generated if LLDB couldn't inspect
-// the variable."
// }
// }
// },
// "required": [ "name", "value", "variablesReference" ]
// }
-llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
- int64_t varID, bool format_hex,
- bool is_name_duplicated,
+llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
+ bool format_hex, bool is_name_duplicated,
std::optional<std::string> custom_name) {
VariableDescription desc(v, format_hex, is_name_duplicated, custom_name);
llvm::json::Object object;
@@ -1246,12 +1259,18 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
}
}
EmplaceSafeString(object, "type", desc.display_type_name);
- if (varID != INT64_MAX)
- object.try_emplace("id", varID);
+
+ // A unique variable identifier to help in properly identifying variables with
+ // the same name. This is an extension to the VS protocol.
+ object.try_emplace("id", var_ref);
+
if (v.MightHaveChildren())
- object.try_emplace("variablesReference", variablesReference);
+ object.try_emplace("variablesReference", var_ref);
else
- object.try_emplace("variablesReference", (int64_t)0);
+ object.try_emplace("variablesReference", 0);
+
+ if (v.GetDeclaration().IsValid())
+ object.try_emplace("declarationLocationReference", var_ref << 1);
object.try_emplace("$__lldb_extensions", desc.GetVariableExtensionsJSON());
return llvm::json::Value(std::move(object));
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index 1515f5ba2e5f4d..0a88dd269bfbdf 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -282,6 +282,16 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
int64_t variablesReference,
int64_t namedVariables, bool expensive);
+/// Create a "Source" JSON object as described in the debug adaptor definition.
+///
+/// \param[in] file
+/// The SBFileSpec to use when populating out the "Source" object
+///
+/// \return
+/// A "Source" JSON object that follows the formal JSON
+/// definition outlined by Microsoft.
+llvm::json::Value CreateSource(const lldb::SBFileSpec &file);
+
/// Create a "Source" JSON object as described in the debug adaptor definition.
///
/// \param[in] line_entry
@@ -289,9 +299,9 @@ llvm::json::Value CreateScope(const llvm::StringRef name,
/// object
///
/// \return
-/// A "Source" JSON object with that follows the formal JSON
+/// A "Source" JSON object that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateSource(lldb::SBLineEntry &line_entry);
+llvm::json::Value CreateSource(const lldb::SBLineEntry &line_entry);
/// Create a "Source" object for a given source path.
///
@@ -433,15 +443,10 @@ struct VariableDescription {
/// The LLDB value to use when populating out the "Variable"
/// object.
///
-/// \param[in] variablesReference
-/// The variable reference. Zero if this value isn't structured
-/// and has no children, non-zero if it does have children and
-/// might be asked to expand itself.
-///
-/// \param[in] varID
-/// A unique variable identifier to help in properly identifying
-/// variables with the same name. This is an extension to the
-/// VS protocol.
+/// \param[in] var_ref
+/// The variable reference. Used to identify the value, e.g.
+/// in the `variablesReference` or `declarationLocationReference`
+/// properties.
///
/// \param[in] format_hex
/// It set to true the variable will be formatted as hex in
@@ -462,8 +467,8 @@ struct VariableDescription {
/// \return
/// A "Variable" JSON object with that follows the formal JSON
/// definition outlined by Microsoft.
-llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
- int64_t varID, bool format_hex,
+llvm::json::Value CreateVariable(lldb::SBValue v, int64_t var_ref,
+ bool format_hex,
bool is_name_duplicated = false,
std::optional<std::string> custom_name = {});
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index ea84f31aec3a6c..c203514fc3d395 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -8,6 +8,7 @@
#include "DAP.h"
#include "Watchpoint.h"
+#include "lldb/API/SBDeclaration.h"
#include "lldb/API/SBMemoryRegionInfo.h"
#include <cassert>
@@ -1405,7 +1406,7 @@ void request_evaluate(const llvm::json::Object &request) {
EmplaceSafeString(body, "result", desc.GetResult(context));
EmplaceSafeString(body, "type", desc.display_type_name);
if (value.MightHaveChildren()) {
- auto variableReference = g_dap.variables.InsertExpandableVariable(
+ auto variableReference = g_dap.variables.InsertVariable(
value, /*is_permanent=*/context == "repl");
body.try_emplace("variablesReference", variableReference);
} else {
@@ -3598,8 +3599,8 @@ void request_setVariable(const llvm::json::Object &request) {
// is_permanent is false because debug console does not support
// setVariable request.
if (variable.MightHaveChildren())
- newVariablesReference = g_dap.variables.InsertExpandableVariable(
- variable, /*is_permanent=*/false);
+ newVariablesReference =
+ g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
body.try_emplace("variablesReference", newVariablesReference);
} else {
@@ -3770,13 +3771,10 @@ void request_variables(const llvm::json::Object &request) {
if (!variable.IsValid())
break;
- int64_t var_ref = 0;
- if (variable.MightHaveChildren() || variable.IsSynthetic()) {
- var_ref = g_dap.variables.InsertExpandableVariable(
- variable, /*is_permanent=*/false);
- }
+ int64_t var_ref =
+ g_dap.variables.InsertVariable(variable, /*is_permanent=*/false);
variables.emplace_back(CreateVariable(
- variable, var_ref, var_ref != 0 ? var_ref : UINT64_MAX, hex,
+ variable, var_ref, hex,
variable_name_counts[GetNonNullVariableName(variable)] > 1));
}
} else {
@@ -3788,19 +3786,11 @@ void request_variables(const llvm::json::Object &request) {
std::optional<std::string> custom_name = {}) {
if (!child.IsValid())
return;
- if (child.MightHaveChildren()) {
- auto is_permanent =
- g_dap.variables.IsPermanentVariableReference(variablesReference);
- auto childVariablesReferences =
- g_dap.variables.InsertExpandableVariable(child, is_permanent);
- variables.emplace_back(CreateVariable(
- child, childVariablesReferences, childVariablesReferences, hex,
- /*is_name_duplicated=*/false, custom_name));
- } else {
- variables.emplace_back(CreateVariable(child, 0, INT64_MAX, hex,
- /*is_name_duplicated=*/false,
- custom_name));
- }
+ bool is_permanent =
+ g_dap.variables.IsPermanentVariableReference(variablesReference);
+ int64_t var_ref = g_dap.variables.InsertVariable(child, is_permanent);
+ variables.emplace_back(CreateVariable(
+ child, var_ref, hex, /*is_name_duplicated=*/false, custom_name));
};
const int64_t num_children = variable.GetNumChildren();
int64_t end_idx = start + ((count == 0) ? num_children : count);
@@ -3823,6 +3813,117 @@ void request_variables(const llvm::...
[truncated]
|
|
Ok, this is ready for review now. The changes in the upstream protocol are reflected by this commit now. I did not remove the |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
vogelsgesang
force-pushed
the
avogelsgesang-lldb-dap-declaration-location
branch
2 times, most recently
from
fcc6b56 to
079def8
Compare
vogelsgesang
force-pushed
the
avogelsgesang-lldb-dap-declaration-location
branch
from
079def8 to
5bdcb82
Compare
|
@vogelsgesang , the |
|
@vogelsgesang , can you show a screenshot of the variables pane after this change? I want to make sure that simple types, like ints, are not displayed as having children. |
There was a problem hiding this comment.
This looks good to me, but I'd like @clayborg to have a second look.
|
|
||
|
|
||
| class TestDAP_locations(lldbdap_testcase.DAPTestCaseBase): | ||
| @skipIfWindows |
There was a problem hiding this comment.
does this fail on windows? Otherwise just test this on Windows.
There was a problem hiding this comment.
copy-paste error :/
There was a problem hiding this comment.
@walter-erquinigo based on #105604, it seems that all DAP tests are in fact disabled for Windows. Should I reintroduce the SkipIfWindows?
There was a problem hiding this comment.
Oof, this is quite unfortunate. I remember that not long ago there was someone who was trying to make these tests past on window, but it seems that this effort wasn't successful.
And well, yes, in order to have an easier time with buildbots, feel free to reintroduce the SkipIfWindows decorator in your tests.
Is that extension's source code available? I wonder how you are currently using the declarationLocation in your frontend, and if your extension is based on |
With this commit (and also with the changes from #104589) 
Current 
As you can see, there are no differences re the absence / presence of the "expandable chevron" next to the variables. That being said, I do find it a bit surprising that function pointers apparently return |
The source code is not available and I couldn't include it in the typescript part of the lldb-dap. I'll explain what I do: The Mojo extension has a listener that captures the DAP traffic. Then, whenever it receives variable information, which includes 
Sadly I can't easily open source that because it requires the cooperation of an LSP. If you think that this feature would be useful to you, assuming that you have a preferred LSP, then we can talk about open sourcing this and adding an LSP hook somewhere. With at least 2 LSPs in question, we could design something relatively generic. |
walter-erquinigo
approved these changes
Aug 22, 2024
Thanks for that context! I don't currently have a LSP in mind. Let's see which UI VS-Code will be providing for this out-of-the-box (if any). I hope it will not conflict with your existing UI |
Thanks! I assume you still want me to wait for @clayborg's review before merging? |
|
If @clayborg takes too long to review, I'd suggest merging this and then acting on any late feedback. But give him a few days for sure if possible. He's a better reviewer than me. |
|
Hi @clayborg, Are you still planning to review this (as requested by @walter-erquinigo)? Or are you fine with me landing this change without your review? |
clayborg
approved these changes
Sep 16, 2024
This commit implements support for the "declaration location" recently added by microsoft/debug-adapter-protocol#494 to the debug adapter protocol. For the `declarationLocationReference` we need a variable ID similar to the the `variablesReference`. I decided to simply reuse the `variablesReference` here and renamed `Variables::expandable_variables` and friends accordingly. Given that almost all variables have a declaration location, we now assign those variable ids to all variables. While `declarationLocationReference` effectively supersedes `$__lldb_extensions.declaration`, I did not remove this extension, yet, since I assume that there are some closed-source extensions which rely on it. I tested this against VS-Code Insiders. However, VS-Code Insiders currently only supports `valueLoctionReference` and not `declarationLocationReference`, yet. Locally, I hence published the declaration locations as value locations, and VS Code Insiders navigated to the expected places. Looking forward to proper VS Code support for `declarationLocationReference`.
vogelsgesang
force-pushed
the
avogelsgesang-lldb-dap-declaration-location
branch
from
1a933e9 to
7dfcc90
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/5111 Here is the relevant piece of the build log for the reference |
|
The above test failure above is in |
vogelsgesang
deleted the
avogelsgesang-lldb-dap-declaration-location
branch
tmsri
pushed a commit
to tmsri/llvm-project
that referenced
this pull request
Sep 19, 2024
This commit implements support for the "declaration location" recently added by microsoft/debug-adapter-protocol#494 to the debug adapter protocol. For the `declarationLocationReference` we need a variable ID similar to the `variablesReference`. I decided to simply reuse the `variablesReference` here and renamed `Variables::expandable_variables` and friends accordingly. Given that almost all variables have a declaration location, we now assign those variable ids to all variables. While `declarationLocationReference` effectively supersedes `$__lldb_extensions.declaration`, I did not remove this extension, yet, since I assume that there are some closed-source extensions which rely on it. I tested this against VS-Code Insiders. However, VS-Code Insiders currently only supports `valueLoctionReference` and not `declarationLocationReference`, yet. Locally, I hence published the declaration locations as value locations, and VS Code Insiders navigated to the expected places. Looking forward to proper VS Code support for `declarationLocationReference`.
JDevlieghere
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Oct 16, 2024
This commit implements support for the "declaration location" recently added by microsoft/debug-adapter-protocol#494 to the debug adapter protocol. For the `declarationLocationReference` we need a variable ID similar to the `variablesReference`. I decided to simply reuse the `variablesReference` here and renamed `Variables::expandable_variables` and friends accordingly. Given that almost all variables have a declaration location, we now assign those variable ids to all variables. While `declarationLocationReference` effectively supersedes `$__lldb_extensions.declaration`, I did not remove this extension, yet, since I assume that there are some closed-source extensions which rely on it. I tested this against VS-Code Insiders. However, VS-Code Insiders currently only supports `valueLoctionReference` and not `declarationLocationReference`, yet. Locally, I hence published the declaration locations as value locations, and VS Code Insiders navigated to the expected places. Looking forward to proper VS Code support for `declarationLocationReference`. (cherry picked from commit 0cc2cd7)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit implements support for the "declaration location" recently added by microsoft/debug-adapter-protocol#494 to the debug adapter protocol.
For the
declarationLocationReferencewe need a variable ID similar to the thevariablesReference. I decided to simply reuse thevariablesReferencehere and renamedVariables::expandable_variablesand friends accordingly. Given that almost all variables have a declaration location, we now assign those variable ids to all variables.While
declarationLocationReferenceeffectively supersedes$__lldb_extensions.declaration, I did not remove this extension, yet, since I assume that there are some closed-source extensions which rely on it.I tested this against VS-Code Insiders. However, VS-Code Insiders currently only supports
valueLoctionReferenceand notdeclarationLocationReference, yet. Locally, I hence published the declaration locations as value locations, and VS Code Insiders navigated to the expected places. Looking forward to proper VS Code support fordeclarationLocationReference.