[Lldb-commits] [lldb] [lldb-dap] Improve support for variables with anonymous fields and types (PR #186482)

[John Harrison via lldb-commits]

================
@@ -47,73 +45,167 @@ template <typename T> StringMap<uint32_t> DistinctNames(T &container) {
   return variable_name_counts;
 }
 
-template <typename T>
-std::vector<Variable> MakeVariables(
-    VariableReferenceStorage &storage, const Configuration &config,
-    const VariablesArguments &args, T &container, bool is_permanent,
-    const std::map<lldb::user_id_t, std::string> &name_overrides = {}) {
-  std::vector<Variable> variables;
+protocol::Scope CreateScope(ScopeKind kind, var_ref_t variablesReference,
+                            bool expensive) {
+  protocol::Scope scope;
+  scope.variablesReference = variablesReference;
+  scope.expensive = expensive;
 
-  // We first find out which variable names are duplicated.
-  StringMap<uint32_t> variable_name_counts = DistinctNames(container);
+  switch (kind) {
+  case eScopeKindLocals:
+    scope.presentationHint = protocol::Scope::eScopePresentationHintLocals;
+    scope.name = "Locals";
+    break;
+  case eScopeKindGlobals:
+    scope.name = "Globals";
+    break;
+  case eScopeKindRegisters:
+    scope.presentationHint = protocol::Scope::eScopePresentationHintRegisters;
+    scope.name = "Registers";
+    break;
+  }
 
-  const bool format_hex = args.format ? args.format->hex : false;
-  auto start_it = begin(container) + args.start;
-  auto end_it = args.count == 0 ? end(container) : start_it + args.count;
+  return scope;
+}
 
-  // Now we construct the result with unique display variable names.
-  for (; start_it != end_it; start_it++) {
-    lldb::SBValue variable = *start_it;
-    if (!variable.IsValid())
-      break;
+class VariableStoreImpl : public VariableStore {
+public:
+  using VariableStore::VariableStore;
+  Variable CreateVariable(lldb::SBValue v, bool format_hex,
+                          bool is_name_duplicated,
+                          std::optional<llvm::StringRef> custom_name) {
+    VariableDescription desc(v, m_storage.config.enableAutoVariableSummaries,
+                             format_hex, is_name_duplicated, custom_name);
+    Variable var;
+    var.name = desc.name;
+    var.value = desc.display_value;
+    var.type = desc.display_type_name;
+
+    if (!desc.evaluate_name.empty())
+      var.evaluateName = desc.evaluate_name;
+
+    // If we have a type with many children, we would like to be able to
+    // give a hint to the IDE that the type has indexed children so that the
+    // request can be broken up in grabbing only a few children at a time. We
+    // want to be careful and only call "v.GetNumChildren()" if we have an array
+    // type or if we have a synthetic child provider producing indexed children.
+    // We don't want to call "v.GetNumChildren()" on all objects as class,
+    // struct and union types don't need to be completed if they are never
+    // expanded. So we want to avoid calling this to only cases where we it
+    // makes sense to keep performance high during normal debugging.
+
+    // If we have an array type, say that it is indexed and provide the number
+    // of children in case we have a huge array. If we don't do this, then we
+    // might take a while to produce all children at onces which can delay your
+    // debug session.
+    if (desc.type_obj.IsArrayType()) {
+      var.indexedVariables = v.GetNumChildren();
+    } else if (v.IsSynthetic()) {
+      // For a type with a synthetic child provider, the SBType of "v" won't
+      // tell us anything about what might be displayed. Instead, we check if
+      // the first child's name is "[0]" and then say it is indexed. We call
+      // GetNumChildren() only if the child name matches to avoid a potentially
+      // expensive operation.
+      if (lldb::SBValue first_child = v.GetChildAtIndex(0)) {
+        llvm::StringRef first_child_name = first_child.GetName();
+        if (first_child_name == "[0]") {
+          size_t num_children = v.GetNumChildren();
+          // If we are creating a "[raw]" fake child for each synthetic type, we
+          // have to account for it when returning indexed variables.
+          if (m_storage.config.enableSyntheticChildDebugging)
+            ++num_children;
+          var.indexedVariables = num_children;
+        }
+      }
+    }
+
+    const bool is_internal = var.name == "[raw]" || m_is_internal;
 
     const var_ref_t var_ref =
-        HasInnerVarref(variable)
-            ? storage.Insert(variable, /*is_permanent=*/is_permanent)
+        HasInnerVarref(v)
+            ? m_storage.Insert(v, /*is_permanent=*/m_is_permanent, is_internal)
             : var_ref_t(var_ref_t::k_no_child);
-    if (LLVM_UNLIKELY(var_ref.AsUInt32() >=
-                      var_ref_t::k_variables_reference_threshold)) {
-      DAP_LOG(storage.log,
-              "warning: variablesReference threshold reached. "
-              "current: {} threshold: {}, maximum {}.",
-              var_ref.AsUInt32(), var_ref_t::k_variables_reference_threshold,
-              var_ref_t::k_max_variables_references);
-      break;
-    }
 
-    if (LLVM_UNLIKELY(var_ref.Kind() == eReferenceKindInvalid))
-      break;
+    if (var.indexedVariables || v.MightHaveChildren())
+      var.variablesReference = var_ref;
+
+    if (v.GetDeclaration().IsValid())
+      var.declarationLocationReference =
+          PackLocation(var_ref.AsUInt32(), false);
 
-    std::optional<std::string> custom_name;
-    auto name_it = name_overrides.find(variable.GetID());
-    if (name_it != name_overrides.end())
-      custom_name = name_it->second;
+    if (ValuePointsToCode(v))
+      var.valueLocationReference = PackLocation(var_ref.AsUInt32(), true);
 
-    variables.emplace_back(CreateVariable(
-        variable, var_ref, format_hex, config.enableAutoVariableSummaries,
-        config.enableSyntheticChildDebugging,
-        variable_name_counts[GetNonNullVariableName(variable)] > 1,
-        custom_name));
+    if (lldb::addr_t addr = v.GetLoadAddress(); addr != LLDB_INVALID_ADDRESS)
+      var.memoryReference = addr;
+
+    bool is_readonly = is_internal || v.GetType().IsAggregateType() ||
+                       v.GetValueType() == lldb::eValueTypeRegisterSet;
+    if (is_readonly) {
+      if (!var.presentationHint)
+        var.presentationHint = {VariablePresentationHint()};
+      var.presentationHint->attributes.push_back("readOnly");
+    }
+
+    if (is_internal) {
+      if (!var.presentationHint)
+        var.presentationHint = {VariablePresentationHint()};
+      var.presentationHint->visibility = "internal";
+    }
----------------
ashgti wrote:

Done.

https://github.com/llvm/llvm-project/pull/186482