[lldb] Split ValueObject::CreateChildAtIndex into two functions (#94455)

labath · web-flow · commit 90b9922df2d8 · 2024-06-07T08:39:10.000+02:00
The the function is doing two fairly different things, depending on how
it is called. While this allows for some code reuse, it also makes it
hard to override it correctly. Possibly for this reason
ValueObjectSynthetic overerides GetChildAtIndex instead, which forces it
to reimplement some of its functionality, most notably caching of
generated children.

Splitting this up makes it easier to move the caching to a common place
(and hopefully makes the code easier to follow in general).
diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
@@ -959,9 +959,12 @@ class ValueObject {
   /// Should only be called by ValueObject::GetChildAtIndex().
   ///
   /// \return A ValueObject managed by this ValueObject's manager.
-  virtual ValueObject *CreateChildAtIndex(size_t idx,
-                                          bool synthetic_array_member,
-                                          int32_t synthetic_index);
+  virtual ValueObject *CreateChildAtIndex(size_t idx);
+
+  /// Should only be called by ValueObject::GetSyntheticArrayMember().
+  ///
+  /// \return A ValueObject managed by this ValueObject's manager.
+  virtual ValueObject *CreateSyntheticArrayMember(size_t idx);
 
   /// Should only be called by ValueObject::GetNumChildren().
   virtual llvm::Expected<uint32_t>
diff --git a/lldb/include/lldb/Core/ValueObjectConstResult.h b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -79,9 +79,6 @@ class ValueObjectConstResult : public ValueObject {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-                                  int32_t synthetic_index) override;
-
   lldb::ValueObjectSP GetSyntheticChildAtOffset(
       uint32_t offset, const CompilerType &type, bool can_create,
       ConstString name_const_str = ConstString()) override;
@@ -151,6 +148,13 @@ class ValueObjectConstResult : public ValueObject {
   ValueObjectConstResult(ExecutionContextScope *exe_scope,
                          ValueObjectManager &manager, const Status &error);
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+    return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+    return m_impl.CreateSyntheticArrayMember(idx);
+  }
+
   ValueObjectConstResult(const ValueObjectConstResult &) = delete;
   const ValueObjectConstResult &
   operator=(const ValueObjectConstResult &) = delete;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResultCast.h b/lldb/include/lldb/Core/ValueObjectConstResultCast.h
@@ -35,9 +35,6 @@ class ValueObjectConstResultCast : public ValueObjectCast {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-                                  int32_t synthetic_index) override;
-
   virtual CompilerType GetCompilerType() {
     return ValueObjectCast::GetCompilerType();
   }
@@ -61,6 +58,13 @@ class ValueObjectConstResultCast : public ValueObjectCast {
   friend class ValueObjectConstResult;
   friend class ValueObjectConstResultImpl;
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+    return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+    return m_impl.CreateSyntheticArrayMember(idx);
+  }
+
   ValueObjectConstResultCast(const ValueObjectConstResultCast &) = delete;
   const ValueObjectConstResultCast &
   operator=(const ValueObjectConstResultCast &) = delete;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResultChild.h b/lldb/include/lldb/Core/ValueObjectConstResultChild.h
@@ -41,9 +41,6 @@ class ValueObjectConstResultChild : public ValueObjectChild {
 
   lldb::ValueObjectSP Dereference(Status &error) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-                                  int32_t synthetic_index) override;
-
   virtual CompilerType GetCompilerType() {
     return ValueObjectChild::GetCompilerType();
   }
@@ -70,6 +67,13 @@ class ValueObjectConstResultChild : public ValueObjectChild {
   friend class ValueObjectConstResult;
   friend class ValueObjectConstResultImpl;
 
+  ValueObject *CreateChildAtIndex(size_t idx) override {
+    return m_impl.CreateChildAtIndex(idx);
+  }
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+    return m_impl.CreateSyntheticArrayMember(idx);
+  }
+
   ValueObjectConstResultChild(const ValueObjectConstResultChild &) = delete;
   const ValueObjectConstResultChild &
   operator=(const ValueObjectConstResultChild &) = delete;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResultImpl.h b/lldb/include/lldb/Core/ValueObjectConstResultImpl.h
@@ -38,8 +38,8 @@ class ValueObjectConstResultImpl {
 
   lldb::ValueObjectSP Dereference(Status &error);
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-                                  int32_t synthetic_index);
+  ValueObject *CreateChildAtIndex(size_t idx);
+  ValueObject *CreateSyntheticArrayMember(size_t idx);
 
   lldb::ValueObjectSP
   GetSyntheticChildAtOffset(uint32_t offset, const CompilerType &type,
diff --git a/lldb/include/lldb/Core/ValueObjectRegister.h b/lldb/include/lldb/Core/ValueObjectRegister.h
@@ -49,9 +49,6 @@ class ValueObjectRegisterSet : public ValueObject {
 
   llvm::Expected<uint32_t> CalculateNumChildren(uint32_t max) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-                                  int32_t synthetic_index) override;
-
   lldb::ValueObjectSP GetChildMemberWithName(llvm::StringRef name,
                                              bool can_create = true) override;
 
@@ -73,6 +70,11 @@ class ValueObjectRegisterSet : public ValueObject {
                          ValueObjectManager &manager,
                          lldb::RegisterContextSP &reg_ctx_sp, uint32_t set_idx);
 
+  ValueObject *CreateChildAtIndex(size_t idx) override;
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+    return nullptr;
+  }
+
   // For ValueObject only
   ValueObjectRegisterSet(const ValueObjectRegisterSet &) = delete;
   const ValueObjectRegisterSet &
diff --git a/lldb/include/lldb/Core/ValueObjectVTable.h b/lldb/include/lldb/Core/ValueObjectVTable.h
@@ -66,9 +66,6 @@ class ValueObjectVTable : public ValueObject {
 
   llvm::Expected<uint32_t> CalculateNumChildren(uint32_t max) override;
 
-  ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
-                                  int32_t synthetic_index) override;
-
   lldb::ValueType GetValueType() const override;
 
   ConstString GetTypeName() override;
@@ -95,6 +92,11 @@ class ValueObjectVTable : public ValueObject {
 private:
   ValueObjectVTable(ValueObject &parent);
 
+  ValueObject *CreateChildAtIndex(size_t idx) override;
+  ValueObject *CreateSyntheticArrayMember(size_t idx) override {
+    return nullptr;
+  }
+
   // For ValueObject only
   ValueObjectVTable(const ValueObjectVTable &) = delete;
   const ValueObjectVTable &operator=(const ValueObjectVTable &) = delete;
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
@@ -382,7 +382,7 @@ ValueObjectSP ValueObject::GetChildAtIndex(uint32_t idx, bool can_create) {
     if (can_create && !m_children.HasChildAtIndex(idx)) {
       // No we haven't created the child at this index, so lets have our
       // subclass do it and cache the result for quick future access.
-      m_children.SetChildAtIndex(idx, CreateChildAtIndex(idx, false, 0));
+      m_children.SetChildAtIndex(idx, CreateChildAtIndex(idx));
     }
 
     ValueObject *child = m_children.GetChildAtIndex(idx);
@@ -488,66 +488,85 @@ void ValueObject::SetNumChildren(uint32_t num_children) {
   m_children.SetChildrenCount(num_children);
 }
 
-ValueObject *ValueObject::CreateChildAtIndex(size_t idx,
-                                             bool synthetic_array_member,
-                                             int32_t synthetic_index) {
-  ValueObject *valobj = nullptr;
-
+ValueObject *ValueObject::CreateChildAtIndex(size_t idx) {
   bool omit_empty_base_classes = true;
-  bool ignore_array_bounds = synthetic_array_member;
-  std::string child_name_str;
+  bool ignore_array_bounds = false;
+  std::string child_name;
   uint32_t child_byte_size = 0;
   int32_t child_byte_offset = 0;
   uint32_t child_bitfield_bit_size = 0;
   uint32_t child_bitfield_bit_offset = 0;
   bool child_is_base_class = false;
   bool child_is_deref_of_parent = false;
   uint64_t language_flags = 0;
-
-  const bool transparent_pointers = !synthetic_array_member;
+  const bool transparent_pointers = true;
 
   ExecutionContext exe_ctx(GetExecutionContextRef());
 
   auto child_compiler_type_or_err =
       GetCompilerType().GetChildCompilerTypeAtIndex(
           &exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
-          ignore_array_bounds, child_name_str, child_byte_size,
-          child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
+          ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
+          child_bitfield_bit_size, child_bitfield_bit_offset,
           child_is_base_class, child_is_deref_of_parent, this, language_flags);
-  CompilerType child_compiler_type;
-  if (!child_compiler_type_or_err)
+  if (!child_compiler_type_or_err || !child_compiler_type_or_err->IsValid()) {
     LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
                    child_compiler_type_or_err.takeError(),
                    "could not find child: {0}");
-  else
-    child_compiler_type = *child_compiler_type_or_err;
+    return nullptr;
+  }
+
+  return new ValueObjectChild(
+      *this, *child_compiler_type_or_err, ConstString(child_name),
+      child_byte_size, child_byte_offset, child_bitfield_bit_size,
+      child_bitfield_bit_offset, child_is_base_class, child_is_deref_of_parent,
+      eAddressTypeInvalid, language_flags);
+}
+
+ValueObject *ValueObject::CreateSyntheticArrayMember(size_t idx) {
+  bool omit_empty_base_classes = true;
+  bool ignore_array_bounds = true;
+  std::string child_name;
+  uint32_t child_byte_size = 0;
+  int32_t child_byte_offset = 0;
+  uint32_t child_bitfield_bit_size = 0;
+  uint32_t child_bitfield_bit_offset = 0;
+  bool child_is_base_class = false;
+  bool child_is_deref_of_parent = false;
+  uint64_t language_flags = 0;
+  const bool transparent_pointers = false;
 
-  if (child_compiler_type) {
-    if (synthetic_index)
-      child_byte_offset += child_byte_size * synthetic_index;
+  ExecutionContext exe_ctx(GetExecutionContextRef());
 
-    ConstString child_name;
-    if (!child_name_str.empty())
-      child_name.SetCString(child_name_str.c_str());
+  auto child_compiler_type_or_err =
+      GetCompilerType().GetChildCompilerTypeAtIndex(
+          &exe_ctx, 0, transparent_pointers, omit_empty_base_classes,
+          ignore_array_bounds, child_name, child_byte_size, child_byte_offset,
+          child_bitfield_bit_size, child_bitfield_bit_offset,
+          child_is_base_class, child_is_deref_of_parent, this, language_flags);
+  if (!child_compiler_type_or_err) {
+    LLDB_LOG_ERROR(GetLog(LLDBLog::Types),
+                   child_compiler_type_or_err.takeError(),
+                   "could not find child: {0}");
+    return nullptr;
+  }
+
+  if (child_compiler_type_or_err->IsValid()) {
+    child_byte_offset += child_byte_size * idx;
 
-    valobj = new ValueObjectChild(
-        *this, child_compiler_type, child_name, child_byte_size,
-        child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
-        child_is_base_class, child_is_deref_of_parent, eAddressTypeInvalid,
-        language_flags);
+    return new ValueObjectChild(
+        *this, *child_compiler_type_or_err, ConstString(child_name),
+        child_byte_size, child_byte_offset, child_bitfield_bit_size,
+        child_bitfield_bit_offset, child_is_base_class,
+        child_is_deref_of_parent, eAddressTypeInvalid, language_flags);
   }
 
   // In case of an incomplete type, try to use the ValueObject's
   // synthetic value to create the child ValueObject.
-  if (!valobj && synthetic_array_member) {
-    if (ValueObjectSP synth_valobj_sp = GetSyntheticValue()) {
-      valobj = synth_valobj_sp
-                   ->GetChildAtIndex(synthetic_index, synthetic_array_member)
-                   .get();
-    }
-  }
+  if (ValueObjectSP synth_valobj_sp = GetSyntheticValue())
+    return synth_valobj_sp->GetChildAtIndex(idx, /*can_create=*/true).get();
 
-  return valobj;
+  return nullptr;
 }
 
 bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
@@ -1616,7 +1635,7 @@ ValueObjectSP ValueObject::GetSyntheticArrayMember(size_t index,
       ValueObject *synthetic_child;
       // We haven't made a synthetic array member for INDEX yet, so lets make
       // one and cache it for any future reference.
-      synthetic_child = CreateChildAtIndex(0, true, index);
+      synthetic_child = CreateSyntheticArrayMember(index);
 
       // Cache the value if we got one back...
       if (synthetic_child) {
diff --git a/lldb/source/Core/ValueObjectConstResult.cpp b/lldb/source/Core/ValueObjectConstResult.cpp
@@ -267,12 +267,6 @@ lldb::addr_t ValueObjectConstResult::GetAddressOf(bool scalar_is_load_address,
   return m_impl.GetAddressOf(scalar_is_load_address, address_type);
 }
 
-ValueObject *ValueObjectConstResult::CreateChildAtIndex(
-    size_t idx, bool synthetic_array_member, int32_t synthetic_index) {
-  return m_impl.CreateChildAtIndex(idx, synthetic_array_member,
-                                   synthetic_index);
-}
-
 size_t ValueObjectConstResult::GetPointeeData(DataExtractor &data,
                                               uint32_t item_idx,
                                               uint32_t item_count) {
diff --git a/lldb/source/Core/ValueObjectConstResultCast.cpp b/lldb/source/Core/ValueObjectConstResultCast.cpp
@@ -44,12 +44,6 @@ lldb::ValueObjectSP ValueObjectConstResultCast::AddressOf(Status &error) {
   return m_impl.AddressOf(error);
 }
 
-ValueObject *ValueObjectConstResultCast::CreateChildAtIndex(
-    size_t idx, bool synthetic_array_member, int32_t synthetic_index) {
-  return m_impl.CreateChildAtIndex(idx, synthetic_array_member,
-                                   synthetic_index);
-}
-
 size_t ValueObjectConstResultCast::GetPointeeData(DataExtractor &data,
                                                   uint32_t item_idx,
                                                   uint32_t item_count) {
diff --git a/lldb/source/Core/ValueObjectConstResultChild.cpp b/lldb/source/Core/ValueObjectConstResultChild.cpp
@@ -56,12 +56,6 @@ lldb::addr_t ValueObjectConstResultChild::GetAddressOf(
   return m_impl.GetAddressOf(scalar_is_load_address, address_type);
 }
 
-ValueObject *ValueObjectConstResultChild::CreateChildAtIndex(
-    size_t idx, bool synthetic_array_member, int32_t synthetic_index) {
-  return m_impl.CreateChildAtIndex(idx, synthetic_array_member,
-                                   synthetic_index);
-}
-
 size_t ValueObjectConstResultChild::GetPointeeData(DataExtractor &data,
                                                    uint32_t item_idx,
                                                    uint32_t item_count) {
diff --git a/lldb/source/Core/ValueObjectConstResultImpl.cpp b/lldb/source/Core/ValueObjectConstResultImpl.cpp
diff --git a/lldb/source/Core/ValueObjectRegister.cpp b/lldb/source/Core/ValueObjectRegister.cpp
diff --git a/lldb/source/Core/ValueObjectVTable.cpp b/lldb/source/Core/ValueObjectVTable.cpp
