lldb-dap: Stop using replicated variable ids

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

@@ -13,55 +13,6 @@
 using namespace lldb_dap::protocol;
 namespace lldb_dap {
 
-/// Creates a `protocol::Scope` struct.
-///
-///
-/// \param[in] name
-///     The value to place into the "name" key
-///
-/// \param[in] variablesReference
-///     The value to place into the "variablesReference" key
-///
-/// \param[in] namedVariables
-///     The value to place into the "namedVariables" key
-///
-/// \param[in] expensive
-///     The value to place into the "expensive" key
-///
-/// \return
-///     A `protocol::Scope`
-static Scope CreateScope(const ScopeKind kind, int64_t variablesReference,
-                         int64_t namedVariables, bool expensive) {
-  Scope scope;
-
-  // TODO: Support "arguments" and "return value" scope.
-  // At the moment lldb-dap includes the arguments and return_value  into the
-  // "locals" scope.
-  // vscode only expands the first non-expensive scope, this causes friction
-  // if we add the arguments above the local scope as the locals scope will not
-  // be expanded if we enter a function with arguments. It becomes more
-  // annoying when the scope has arguments, return_value and locals.
-  switch (kind) {
-  case ScopeKind::Locals:
-    scope.presentationHint = Scope::eScopePresentationHintLocals;
-    scope.name = "Locals";
-    break;
-  case ScopeKind::Globals:
-    scope.name = "Globals";
-    break;
-  case ScopeKind::Registers:
-    scope.presentationHint = Scope::eScopePresentationHintRegisters;
-    scope.name = "Registers";
-    break;
-  }
-
-  scope.variablesReference = variablesReference;
-  scope.namedVariables = namedVariables;
-  scope.expensive = expensive;
-
-  return scope;
-}
-
 llvm::Expected<ScopesResponseBody>
 ScopesRequestHandler::Run(const ScopesArguments &args) const {
   lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
@@ -86,32 +37,8 @@ ScopesRequestHandler::Run(const ScopesArguments &args) const {
   }
 
   uint32_t frame_id = frame.GetFrameID();
-
-  dap.variables.ReadyFrame(frame_id, frame);
-
-  std::vector<protocol::Scope> scopes = {};
-
-  int64_t variable_reference = dap.variables.GetNewVariableReference(false);
-  scopes.push_back(CreateScope(
-      ScopeKind::Locals, variable_reference,
-      dap.variables.GetScope(frame_id, ScopeKind::Locals)->GetSize(), false));
-
-  dap.variables.AddScopeKind(variable_reference, ScopeKind::Locals, frame_id);
-
-  variable_reference = dap.variables.GetNewVariableReference(false);
-  scopes.push_back(CreateScope(
-      ScopeKind::Globals, variable_reference,
-      dap.variables.GetScope(frame_id, ScopeKind::Globals)->GetSize(), false));
-  dap.variables.AddScopeKind(variable_reference, ScopeKind::Globals, frame_id);
-
-  variable_reference = dap.variables.GetNewVariableReference(false);
-  scopes.push_back(CreateScope(
-      ScopeKind::Registers, variable_reference,
-      dap.variables.GetScope(frame_id, ScopeKind::Registers)->GetSize(),
-      false));
-
-  dap.variables.AddScopeKind(variable_reference, ScopeKind::Registers,
-                             frame_id);
+  std::vector<protocol::Scope> scopes =
+      dap.variables.ReadyFrame(frame_id, frame);
 
   return ScopesResponseBody{std::move(scopes)};
 }

lldb/tools/lldb-dap/JSONUtils.cpp

@@ -11,7 +11,6 @@
 #include "ExceptionBreakpoint.h"
 #include "LLDBUtils.h"
 #include "ProtocolUtils.h"
-#include "Variables.h"
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBCompileUnit.h"
 #include "lldb/API/SBDeclaration.h"

lldb/tools/lldb-dap/JSONUtils.h

@@ -9,7 +9,6 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
 #define LLDB_TOOLS_LLDB_DAP_JSONUTILS_H
 
-#include "DAP.h"
 #include "DAPForward.h"
 #include "Protocol/ProtocolTypes.h"
 #include "lldb/API/SBCompileUnit.h"

lldb/tools/lldb-dap/Variables.cpp

@@ -8,13 +8,49 @@
 
 #include "Variables.h"
 #include "JSONUtils.h"
+#include "Protocol/ProtocolTypes.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBValueList.h"
 #include <cstdint>
 #include <optional>
+#include <vector>
 
 using namespace lldb_dap;
 
+namespace lldb_dap {
+
+protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference,
+                            int64_t namedVariables, bool expensive) {
+  protocol::Scope scope;
+
+  // TODO: Support "arguments" and "return value" scope.
+  // At the moment lldb-he arguments and return_value  into the
+  // "locals" scope.
+  // vscode only expands the first non-expensive scope, this causes friction
+  // if we add the arguments above the local scope as the locals scope will not
+  // be expanded if we enter a function with arguments. It becomes more
+  // annoying when the scope has arguments, return_value and locals.
+  switch (kind) {
+  case ScopeKind::Locals:
+    scope.presentationHint = protocol::Scope::eScopePresentationHintLocals;
+    scope.name = "Locals";
+    break;
+  case ScopeKind::Globals:
+    scope.name = "Globals";
+    break;
+  case ScopeKind::Registers:
+    scope.presentationHint = protocol::Scope::eScopePresentationHintRegisters;
+    scope.name = "Registers";
+    break;
+  }
+
+  scope.variablesReference = variablesReference;
+  scope.namedVariables = namedVariables;
+  scope.expensive = expensive;
+
+  return scope;
+}
+
 lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
   auto iter = m_scope_kinds.find(variablesReference);
   if (iter == m_scope_kinds.end()) {
@@ -168,7 +204,9 @@ lldb::SBValueList *Variables::GetScope(const uint32_t frame_id,
   return nullptr;
 }
 
-void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) {
+std::vector<protocol::Scope> Variables::ReadyFrame(uint32_t frame_id,
+                                                   lldb::SBFrame &frame) {
+
   if (m_frames.find(frame_id) == m_frames.end()) {
 
     auto locals = frame.GetVariables(/*arguments=*/true,
@@ -186,10 +224,34 @@ void Variables::ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame) {
     m_frames.insert(
         std::make_pair(frame_id, std::make_tuple(locals, globals, registers)));
   }
-}
 
-void Variables::AddScopeKind(int64_t variable_reference, ScopeKind kind,
-                             uint32_t frame_id) {
+  std::vector<protocol::Scope> scopes = {};
+
+  int64_t variable_reference = GetNewVariableReference(false);
+
+  scopes.push_back(CreateScope(ScopeKind::Locals, variable_reference,
+                               GetScope(frame_id, ScopeKind::Locals)->GetSize(),
+                               false));
 
-  m_scope_kinds[variable_reference] = std::make_pair(kind, frame_id);
+  m_scope_kinds[variable_reference] =
+      std::make_pair(ScopeKind::Locals, frame_id);
+
+  variable_reference = GetNewVariableReference(false);
+  scopes.push_back(
+      CreateScope(ScopeKind::Globals, variable_reference,
+                  GetScope(frame_id, ScopeKind::Globals)->GetSize(), false));
+  m_scope_kinds[variable_reference] =
+      std::make_pair(ScopeKind::Globals, frame_id);
+
+  variable_reference = GetNewVariableReference(false);
+  scopes.push_back(
+      CreateScope(ScopeKind::Registers, variable_reference,
+                  GetScope(frame_id, ScopeKind::Registers)->GetSize(), false));
+
+  m_scope_kinds[variable_reference] =
+      std::make_pair(ScopeKind::Registers, frame_id);
+
+  return scopes;
 }
+
+} // namespace lldb_dap

lldb/tools/lldb-dap/Variables.h

@@ -9,6 +9,7 @@
 #ifndef LLDB_TOOLS_LLDB_DAP_VARIABLES_H
 #define LLDB_TOOLS_LLDB_DAP_VARIABLES_H
 
+#include "Protocol/ProtocolTypes.h"
 #include "lldb/API/SBValue.h"
 #include "lldb/API/SBValueList.h"
 #include "llvm/ADT/DenseMap.h"
@@ -20,6 +21,24 @@
 namespace lldb_dap {
 
 enum ScopeKind { Locals, Globals, Registers };
+/// Creates a `protocol::Scope` struct.
+///
+/// \param[in] kind
+///     The kind of scope to create
+///
+/// \param[in] variablesReference
+///     The value to place into the "variablesReference" key
+///
+/// \param[in] namedVariables
+///     The value to place into the "namedVariables" key
+///
+/// \param[in] expensive
+///     The value to place into the "expensive" key
+///
+/// \return
+///     A `protocol::Scope`
+protocol::Scope CreateScope(const ScopeKind kind, int64_t variablesReference,
+                            int64_t namedVariables, bool expensive);
 
 struct ScopeData {
   ScopeKind kind;
@@ -55,15 +74,13 @@ struct Variables {
   lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name);
 
   /// Initialize a frame if it hasn't been already, otherwise do nothing
-  void ReadyFrame(uint32_t frame_id, lldb::SBFrame &frame);
+  std::vector<protocol::Scope> ReadyFrame(uint32_t frame_id,
+                                          lldb::SBFrame &frame);
   std::optional<ScopeData> GetScopeKind(const int64_t variablesReference);
 
   /// Clear all scope variables and non-permanent expandable variables.
   void Clear();
 
-  void AddScopeKind(int64_t variable_reference, ScopeKind kind,
-                    uint32_t frame_id);
-
 private:
   /// Variable_reference start index of permanent expandable variable.
   static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);