[Lldb-commits] [lldb] [lldb] Split ValueObject::CreateChildAtIndex into two functions (PR #94455)
via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 5 03:35:02 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
<details>
<summary>Changes</summary>
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).
---
Patch is 25.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/94455.diff
14 Files Affected:
- (modified) lldb/include/lldb/Core/ValueObject.h (+6-3)
- (modified) lldb/include/lldb/Core/ValueObjectConstResult.h (+7-3)
- (modified) lldb/include/lldb/Core/ValueObjectConstResultCast.h (+7-3)
- (modified) lldb/include/lldb/Core/ValueObjectConstResultChild.h (+7-3)
- (modified) lldb/include/lldb/Core/ValueObjectConstResultImpl.h (+2-2)
- (modified) lldb/include/lldb/Core/ValueObjectRegister.h (+5-3)
- (modified) lldb/include/lldb/Core/ValueObjectVTable.h (+5-3)
- (modified) lldb/source/Core/ValueObject.cpp (+55-36)
- (modified) lldb/source/Core/ValueObjectConstResult.cpp (-6)
- (modified) lldb/source/Core/ValueObjectConstResultCast.cpp (-6)
- (modified) lldb/source/Core/ValueObjectConstResultChild.cpp (-6)
- (modified) lldb/source/Core/ValueObjectConstResultImpl.cpp (+78-37)
- (modified) lldb/source/Core/ValueObjectRegister.cpp (+5-9)
- (modified) lldb/source/Core/ValueObjectVTable.cpp (+1-5)
``````````diff
diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index e7e35e2b2bffc..1e8c7c9c00536 100644
--- 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
index 37dc0867f26c9..d3b3362bd0e9e 100644
--- 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
index efcbe0dc6a0bd..911a08363b393 100644
--- 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
index 7e9da14e8e97f..71a3c53befe78 100644
--- 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
index 5a7a079d3095c..68ba8ae7fba20 100644
--- 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
index fec8566ba33d9..d948c663a4f8b 100644
--- 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 ®_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
index 4662f395a4dde..7087dcc1d1bec 100644
--- 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
index 1443d9dfc3280..0e632f4cf49a3 100644
--- 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,14 +488,10 @@ 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;
@@ -503,51 +499,74 @@ ValueObject *ValueObject::CreateChildAtIndex(size_t idx,
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
index 8ac2c1cac2f66..879d3c3f6b037 100644
--- 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
index fceb2635f876f..bf7a12dc68236 100644
--- 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
index 36bf11a0b73af..39fc0c9fbb35b 100644
--- 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
index 493980d7ea960..2a7c907700765 100644
--- a/lldb/source/Core/ValueObjectConstResultImpl.cpp
+++ b/lldb/source/Core/ValueObjectConstResultImpl.cpp
@@ -46,18 +46,15 @@ lldb::ValueObjectSP ValueObjectConstResultImpl::Dereference(Status &error) {
return m_impl_backend->ValueObject::Dereference(error);
}
-ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(
- size_t idx, bool synthetic_array_member, int32_t synthetic_index) {
+ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(size_t idx) {
if (m_impl_backend == nullptr)
return nullptr;
m_impl_backend->UpdateValueIfNeeded(false);
- ValueObjectConstResultChild *valobj = nullptr;
-
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;
@@ -65,56 +62,100 @@ ValueObject *ValueObjectConstResultImpl::CreateChildAtIndex(
bool child_is_base_class = false;
bool child_is_deref_of_parent = false;
uint64_t language_flags;
-
- const bool transparent_pointers = !synthetic_array_member;
+ const bool transparent_pointers = true;
CompilerType compiler_type = m_impl_backend->GetCompilerType();
ExecutionContext exe_ctx(m_impl_backend->GetExecutionContextRef());
auto child_compiler_type_or_err = compiler_type.GetChildCompilerTypeAtIndex(
&exe_ctx, idx, transparent_pointers, omit_empty_base_classes,
- ignore_array_bounds, child_name_str, child_byte_size, child_byte_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, m_impl_backend, language_flags);
- CompilerType child_compiler_type;
- if (!child_compiler_type_or_err)
+
+ // One might think we should check that the size of the children
+ // is always strictly positive, hence we could avoid creating a
+ // ValueObject if that's not the case, but it turns out there
+ // are languages out there which allow zero-size types with
+ // children (e.g. Swift).
+ 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;
+ }
+ lldb::addr_t child_live_addr = LLDB_INVALID_ADDRESS;
+ // Transfer the live address (with offset) to the child. But if
+ // the parent is a pointer, the live address is where that pointer
+ // value lives in memory, so the children live addresses aren't
+ // offsets from that value, they are just other load addresses that
+ // are recorded in the Value of the child ValueObjects.
+ if (m_live_address != LLDB_INVALID_ADDRESS && !compiler_type.IsPointerType())
+ child_live_addr = m_live_address + child_byte_offset;
+
+ return new ValueObjectConstResultChild(
+ *m_impl_backend, *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,
+ child_live_addr, language_flags);
+}
+
+ValueObject *
+ValueObjectConstResultImpl::CreateSyntheticArrayMember(size_t idx) {
+ if (m_impl_backend == nullptr)
+ return nullptr;
+
+ m_impl_backend->UpdateValueIfNeeded(false);
+
+ 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;
+ ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/94455
More information about the lldb-commits
mailing list