Skip to content

[lldb-dap] Move the Variables struct into its own file #140393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JDevlieghere
Copy link
Member

@JDevlieghere JDevlieghere commented May 17, 2025

Move the Variables struct out of DAP.h and into its own file to reduce the complexity of the latter. This PR also makes the members that are implementation details private and adds a handful of basic unit tests.

Move the Variables struct out of DAP.h and into its own file to reduce
the complexity of the latter. This PR also makes the members that are
implementation details private.
@llvmbot
Copy link
Member

llvmbot commented May 17, 2025

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Move the Variables struct out of DAP.h and into its own file to reduce the complexity of the latter. This PR also makes the members that are implementation details private.


Full diff: https://github.com/llvm/llvm-project/pull/140393.diff

5 Files Affected:

  • (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1)
  • (modified) lldb/tools/lldb-dap/DAP.cpp (-93)
  • (modified) lldb/tools/lldb-dap/DAP.h (+1-50)
  • (added) lldb/tools/lldb-dap/Variables.cpp (+105)
  • (added) lldb/tools/lldb-dap/Variables.h (+71)
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt
index 5dedee8a87f41..f8e81eaff8606 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -23,6 +23,7 @@ add_lldb_library(lldbDAP
   RunInTerminal.cpp
   SourceBreakpoint.cpp
   Transport.cpp
+  Variables.cpp
   Watchpoint.cpp
 
   Handler/ResponseHandler.cpp
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 0d5eba6c40961..af46838f5671c 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -1021,45 +1021,6 @@ lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) {
   return error;
 }
 
-void Variables::Clear() {
-  locals.Clear();
-  globals.Clear();
-  registers.Clear();
-  referenced_variables.clear();
-}
-
-int64_t Variables::GetNewVariableReference(bool is_permanent) {
-  if (is_permanent)
-    return next_permanent_var_ref++;
-  return next_temporary_var_ref++;
-}
-
-bool Variables::IsPermanentVariableReference(int64_t var_ref) {
-  return var_ref >= PermanentVariableStartIndex;
-}
-
-lldb::SBValue Variables::GetVariable(int64_t var_ref) const {
-  if (IsPermanentVariableReference(var_ref)) {
-    auto pos = referenced_permanent_variables.find(var_ref);
-    if (pos != referenced_permanent_variables.end())
-      return pos->second;
-  } else {
-    auto pos = referenced_variables.find(var_ref);
-    if (pos != referenced_variables.end())
-      return pos->second;
-  }
-  return lldb::SBValue();
-}
-
-int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) {
-  int64_t var_ref = GetNewVariableReference(is_permanent);
-  if (is_permanent)
-    referenced_permanent_variables.insert(std::make_pair(var_ref, variable));
-  else
-    referenced_variables.insert(std::make_pair(var_ref, variable));
-  return var_ref;
-}
-
 bool StartDebuggingRequestHandler::DoExecute(
     lldb::SBDebugger debugger, char **command,
     lldb::SBCommandReturnObject &result) {
@@ -1296,60 +1257,6 @@ DAP::GetInstructionBPFromStopReason(lldb::SBThread &thread) {
   return inst_bp;
 }
 
-lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
-  switch (variablesReference) {
-  case VARREF_LOCALS:
-    return &locals;
-  case VARREF_GLOBALS:
-    return &globals;
-  case VARREF_REGS:
-    return &registers;
-  default:
-    return nullptr;
-  }
-}
-
-lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
-                                      llvm::StringRef name) {
-  lldb::SBValue variable;
-  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
-    bool is_duplicated_variable_name = name.contains(" @");
-    // variablesReference is one of our scopes, not an actual variable it is
-    // asking for a variable in locals or globals or registers
-    int64_t end_idx = top_scope->GetSize();
-    // Searching backward so that we choose the variable in closest scope
-    // among variables of the same name.
-    for (int64_t i = end_idx - 1; i >= 0; --i) {
-      lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
-      std::string variable_name = CreateUniqueVariableNameForDisplay(
-          curr_variable, is_duplicated_variable_name);
-      if (variable_name == name) {
-        variable = curr_variable;
-        break;
-      }
-    }
-  } else {
-    // This is not under the globals or locals scope, so there are no
-    // duplicated names.
-
-    // We have a named item within an actual variable so we need to find it
-    // withing the container variable by name.
-    lldb::SBValue container = GetVariable(variablesReference);
-    variable = container.GetChildMemberWithName(name.data());
-    if (!variable.IsValid()) {
-      if (name.starts_with("[")) {
-        llvm::StringRef index_str(name.drop_front(1));
-        uint64_t index = 0;
-        if (!index_str.consumeInteger(0, index)) {
-          if (index_str == "]")
-            variable = container.GetChildAtIndex(index);
-        }
-      }
-    }
-  }
-  return variable;
-}
-
 protocol::Capabilities DAP::GetCapabilities() {
   protocol::Capabilities capabilities;
 
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 8f24c6cf82924..033203309b4c6 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -20,6 +20,7 @@
 #include "Protocol/ProtocolTypes.h"
 #include "SourceBreakpoint.h"
 #include "Transport.h"
+#include "Variables.h"
 #include "lldb/API/SBBroadcaster.h"
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBDebugger.h"
@@ -30,8 +31,6 @@
 #include "lldb/API/SBMutex.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/API/SBThread.h"
-#include "lldb/API/SBValue.h"
-#include "lldb/API/SBValueList.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -52,10 +51,6 @@
 #include <thread>
 #include <vector>
 
-#define VARREF_LOCALS (int64_t)1
-#define VARREF_GLOBALS (int64_t)2
-#define VARREF_REGS (int64_t)3
-#define VARREF_FIRST_VAR_IDX (int64_t)4
 #define NO_TYPENAME "<no-type>"
 
 namespace lldb_dap {
@@ -88,50 +83,6 @@ enum class PacketStatus {
 
 enum class ReplMode { Variable = 0, Command, Auto };
 
-struct Variables {
-  /// Variable_reference start index of permanent expandable variable.
-  static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);
-
-  lldb::SBValueList locals;
-  lldb::SBValueList globals;
-  lldb::SBValueList registers;
-
-  int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
-  int64_t next_permanent_var_ref{PermanentVariableStartIndex};
-
-  /// Variables that are alive in this stop state.
-  /// Will be cleared when debuggee resumes.
-  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> 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
-  static bool IsPermanentVariableReference(int64_t var_ref);
-
-  /// \return a new variableReference.
-  /// Specify is_permanent as true for variable that should persist entire
-  /// debug session.
-  int64_t GetNewVariableReference(bool is_permanent);
-
-  /// \return the expandable variable corresponding with variableReference
-  /// value of \p value.
-  /// If \p var_ref is invalid an empty SBValue is returned.
-  lldb::SBValue GetVariable(int64_t var_ref) const;
-
-  /// Insert a new \p variable.
-  /// \return variableReference assigned to this expandable variable.
-  int64_t InsertVariable(lldb::SBValue variable, bool is_permanent);
-
-  lldb::SBValueList *GetTopLevelScope(int64_t variablesReference);
-
-  lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name);
-
-  /// Clear all scope variables and non-permanent expandable variables.
-  void Clear();
-};
-
 struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
   DAP &dap;
   explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
diff --git a/lldb/tools/lldb-dap/Variables.cpp b/lldb/tools/lldb-dap/Variables.cpp
new file mode 100644
index 0000000000000..777e3183d8c0d
--- /dev/null
+++ b/lldb/tools/lldb-dap/Variables.cpp
@@ -0,0 +1,105 @@
+//===-- Variables.cpp ---------------------------------------------------*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Variables.h"
+#include "JSONUtils.h"
+
+using namespace lldb_dap;
+
+lldb::SBValueList *Variables::GetTopLevelScope(int64_t variablesReference) {
+  switch (variablesReference) {
+  case VARREF_LOCALS:
+    return &locals;
+  case VARREF_GLOBALS:
+    return &globals;
+  case VARREF_REGS:
+    return &registers;
+  default:
+    return nullptr;
+  }
+}
+
+void Variables::Clear() {
+  locals.Clear();
+  globals.Clear();
+  registers.Clear();
+  m_referencedvariables.clear();
+}
+
+int64_t Variables::GetNewVariableReference(bool is_permanent) {
+  if (is_permanent)
+    return m_next_permanent_var_ref++;
+  return m_next_temporary_var_ref++;
+}
+
+bool Variables::IsPermanentVariableReference(int64_t var_ref) {
+  return var_ref >= PermanentVariableStartIndex;
+}
+
+lldb::SBValue Variables::GetVariable(int64_t var_ref) const {
+  if (IsPermanentVariableReference(var_ref)) {
+    auto pos = m_referencedpermanent_variables.find(var_ref);
+    if (pos != m_referencedpermanent_variables.end())
+      return pos->second;
+  } else {
+    auto pos = m_referencedvariables.find(var_ref);
+    if (pos != m_referencedvariables.end())
+      return pos->second;
+  }
+  return lldb::SBValue();
+}
+
+int64_t Variables::InsertVariable(lldb::SBValue variable, bool is_permanent) {
+  int64_t var_ref = GetNewVariableReference(is_permanent);
+  if (is_permanent)
+    m_referencedpermanent_variables.insert(std::make_pair(var_ref, variable));
+  else
+    m_referencedvariables.insert(std::make_pair(var_ref, variable));
+  return var_ref;
+}
+
+lldb::SBValue Variables::FindVariable(uint64_t variablesReference,
+                                      llvm::StringRef name) {
+  lldb::SBValue variable;
+  if (lldb::SBValueList *top_scope = GetTopLevelScope(variablesReference)) {
+    bool is_duplicated_variable_name = name.contains(" @");
+    // variablesReference is one of our scopes, not an actual variable it is
+    // asking for a variable in locals or globals or registers
+    int64_t end_idx = top_scope->GetSize();
+    // Searching backward so that we choose the variable in closest scope
+    // among variables of the same name.
+    for (int64_t i = end_idx - 1; i >= 0; --i) {
+      lldb::SBValue curr_variable = top_scope->GetValueAtIndex(i);
+      std::string variable_name = CreateUniqueVariableNameForDisplay(
+          curr_variable, is_duplicated_variable_name);
+      if (variable_name == name) {
+        variable = curr_variable;
+        break;
+      }
+    }
+  } else {
+    // This is not under the globals or locals scope, so there are no
+    // duplicated names.
+
+    // We have a named item within an actual variable so we need to find it
+    // withing the container variable by name.
+    lldb::SBValue container = GetVariable(variablesReference);
+    variable = container.GetChildMemberWithName(name.data());
+    if (!variable.IsValid()) {
+      if (name.starts_with("[")) {
+        llvm::StringRef index_str(name.drop_front(1));
+        uint64_t index = 0;
+        if (!index_str.consumeInteger(0, index)) {
+          if (index_str == "]")
+            variable = container.GetChildAtIndex(index);
+        }
+      }
+    }
+  }
+  return variable;
+}
diff --git a/lldb/tools/lldb-dap/Variables.h b/lldb/tools/lldb-dap/Variables.h
new file mode 100644
index 0000000000000..0ed84b36aef99
--- /dev/null
+++ b/lldb/tools/lldb-dap/Variables.h
@@ -0,0 +1,71 @@
+//===-- Variables.h -----------------------------------------------------*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_TOOLS_LLDB_DAP_VARIABLES_H
+#define LLDB_TOOLS_LLDB_DAP_VARIABLES_H
+
+#include "lldb/API/SBValue.h"
+#include "lldb/API/SBValueList.h"
+#include "llvm/ADT/DenseMap.h"
+
+#define VARREF_FIRST_VAR_IDX (int64_t)4
+#define VARREF_LOCALS (int64_t)1
+#define VARREF_GLOBALS (int64_t)2
+#define VARREF_REGS (int64_t)3
+
+namespace lldb_dap {
+
+struct Variables {
+  lldb::SBValueList locals;
+  lldb::SBValueList globals;
+  lldb::SBValueList registers;
+
+  /// 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
+  static bool IsPermanentVariableReference(int64_t var_ref);
+
+  /// \return a new variableReference.
+  /// Specify is_permanent as true for variable that should persist entire
+  /// debug session.
+  int64_t GetNewVariableReference(bool is_permanent);
+
+  /// \return the expandable variable corresponding with variableReference
+  /// value of \p value.
+  /// If \p var_ref is invalid an empty SBValue is returned.
+  lldb::SBValue GetVariable(int64_t var_ref) const;
+
+  /// Insert a new \p variable.
+  /// \return variableReference assigned to this expandable variable.
+  int64_t InsertVariable(lldb::SBValue variable, bool is_permanent);
+
+  lldb::SBValueList *GetTopLevelScope(int64_t variablesReference);
+
+  lldb::SBValue FindVariable(uint64_t variablesReference, llvm::StringRef name);
+
+  /// Clear all scope variables and non-permanent expandable variables.
+  void Clear();
+
+private:
+  /// Variable_reference start index of permanent expandable variable.
+  static constexpr int64_t PermanentVariableStartIndex = (1ll << 32);
+
+  /// Variables that are alive in this stop state.
+  /// Will be cleared when debuggee resumes.
+  llvm::DenseMap<int64_t, lldb::SBValue> m_referencedvariables;
+
+  /// Variables that persist across entire debug session.
+  /// These are the variables evaluated from debug console REPL.
+  llvm::DenseMap<int64_t, lldb::SBValue> m_referencedpermanent_variables;
+
+  int64_t m_next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
+  int64_t m_next_permanent_var_ref{PermanentVariableStartIndex};
+};
+
+} // namespace lldb_dap
+
+#endif

@JDevlieghere JDevlieghere changed the title [lldb-dap] Move the Variables struct into its own file (NFC) [lldb-dap] Move the Variables struct into its own file May 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants