Change the return type of CalculateNumChildren to uint32_t. #83501
Conversation
adrian-prantl
requested review from
clayborg
and removed request for
JDevlieghere
|
@llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) ChangesIn the end this value comes from TypeSystem::GetNumChildren which returns a uint32_t, so ValueObject should be consistent with that. Full diff: https://github.com/llvm/llvm-project/pull/83501.diff 20 Files Affected:
diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index 4c0b0b2dae6cd4..05dd64f5634fda 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -958,7 +958,7 @@ class ValueObject {
int32_t synthetic_index);
/// Should only be called by ValueObject::GetNumChildren().
- virtual size_t CalculateNumChildren(uint32_t max = UINT32_MAX) = 0;
+ virtual uint32_t CalculateNumChildren(uint32_t max = UINT32_MAX) = 0;
void SetNumChildren(size_t num_children);
diff --git a/lldb/include/lldb/Core/ValueObjectCast.h b/lldb/include/lldb/Core/ValueObjectCast.h
index fe053c12d9c343..51c647680d5227 100644
--- a/lldb/include/lldb/Core/ValueObjectCast.h
+++ b/lldb/include/lldb/Core/ValueObjectCast.h
@@ -33,7 +33,7 @@ class ValueObjectCast : public ValueObject {
std::optional<uint64_t> GetByteSize() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
lldb::ValueType GetValueType() const override;
diff --git a/lldb/include/lldb/Core/ValueObjectChild.h b/lldb/include/lldb/Core/ValueObjectChild.h
index 46b14e6840f0dc..47a13be08bb83b 100644
--- a/lldb/include/lldb/Core/ValueObjectChild.h
+++ b/lldb/include/lldb/Core/ValueObjectChild.h
@@ -39,7 +39,7 @@ class ValueObjectChild : public ValueObject {
lldb::ValueType GetValueType() const override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
ConstString GetTypeName() override;
diff --git a/lldb/include/lldb/Core/ValueObjectConstResult.h b/lldb/include/lldb/Core/ValueObjectConstResult.h
index d61df859bebce4..9f1246cf2a7874 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -67,7 +67,7 @@ class ValueObjectConstResult : public ValueObject {
lldb::ValueType GetValueType() const override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
ConstString GetTypeName() override;
diff --git a/lldb/include/lldb/Core/ValueObjectDynamicValue.h b/lldb/include/lldb/Core/ValueObjectDynamicValue.h
index 2758b4e5bb564d..21a9b409fd5bd7 100644
--- a/lldb/include/lldb/Core/ValueObjectDynamicValue.h
+++ b/lldb/include/lldb/Core/ValueObjectDynamicValue.h
@@ -43,7 +43,7 @@ class ValueObjectDynamicValue : public ValueObject {
ConstString GetDisplayTypeName() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
lldb::ValueType GetValueType() const override;
diff --git a/lldb/include/lldb/Core/ValueObjectMemory.h b/lldb/include/lldb/Core/ValueObjectMemory.h
index 3c01df388d2e6d..a74b325546b03c 100644
--- a/lldb/include/lldb/Core/ValueObjectMemory.h
+++ b/lldb/include/lldb/Core/ValueObjectMemory.h
@@ -47,7 +47,7 @@ class ValueObjectMemory : public ValueObject {
ConstString GetDisplayTypeName() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
lldb::ValueType GetValueType() const override;
diff --git a/lldb/include/lldb/Core/ValueObjectRegister.h b/lldb/include/lldb/Core/ValueObjectRegister.h
index 2e47eee3d7f793..6c470c1a686503 100644
--- a/lldb/include/lldb/Core/ValueObjectRegister.h
+++ b/lldb/include/lldb/Core/ValueObjectRegister.h
@@ -47,7 +47,7 @@ class ValueObjectRegisterSet : public ValueObject {
ConstString GetQualifiedTypeName() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
int32_t synthetic_index) override;
@@ -95,7 +95,7 @@ class ValueObjectRegister : public ValueObject {
ConstString GetTypeName() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
bool SetValueFromCString(const char *value_str, Status &error) override;
diff --git a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
index 67596232eafd1e..57794072ff9229 100644
--- a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -47,7 +47,7 @@ class ValueObjectSynthetic : public ValueObject {
bool MightHaveChildren() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
lldb::ValueType GetValueType() const override;
diff --git a/lldb/include/lldb/Core/ValueObjectVTable.h b/lldb/include/lldb/Core/ValueObjectVTable.h
index 217ff8d0d334ce..e7e14fc83d7892 100644
--- a/lldb/include/lldb/Core/ValueObjectVTable.h
+++ b/lldb/include/lldb/Core/ValueObjectVTable.h
@@ -64,7 +64,7 @@ class ValueObjectVTable : public ValueObject {
std::optional<uint64_t> GetByteSize() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
ValueObject *CreateChildAtIndex(size_t idx, bool synthetic_array_member,
int32_t synthetic_index) override;
diff --git a/lldb/include/lldb/Core/ValueObjectVariable.h b/lldb/include/lldb/Core/ValueObjectVariable.h
index bba28ce567b2a0..da270300df0b30 100644
--- a/lldb/include/lldb/Core/ValueObjectVariable.h
+++ b/lldb/include/lldb/Core/ValueObjectVariable.h
@@ -46,7 +46,7 @@ class ValueObjectVariable : public ValueObject {
ConstString GetDisplayTypeName() override;
- size_t CalculateNumChildren(uint32_t max) override;
+ uint32_t CalculateNumChildren(uint32_t max) override;
lldb::ValueType GetValueType() const override;
diff --git a/lldb/include/lldb/Target/StackFrameRecognizer.h b/lldb/include/lldb/Target/StackFrameRecognizer.h
index 419f0c0aac1f86..e111f4a4dc7029 100644
--- a/lldb/include/lldb/Target/StackFrameRecognizer.h
+++ b/lldb/include/lldb/Target/StackFrameRecognizer.h
@@ -164,7 +164,7 @@ class ValueObjectRecognizerSynthesizedValue : public ValueObject {
m_value = m_parent->GetValue();
return true;
}
- size_t CalculateNumChildren(uint32_t max = UINT32_MAX) override {
+ uint32_t CalculateNumChildren(uint32_t max = UINT32_MAX) override {
return m_parent->GetNumChildren(max);
}
CompilerType GetCompilerTypeImpl() override {
diff --git a/lldb/source/Core/ValueObjectCast.cpp b/lldb/source/Core/ValueObjectCast.cpp
index 0882d4b3677619..a5c555f86b1372 100644
--- a/lldb/source/Core/ValueObjectCast.cpp
+++ b/lldb/source/Core/ValueObjectCast.cpp
@@ -41,7 +41,7 @@ ValueObjectCast::~ValueObjectCast() = default;
CompilerType ValueObjectCast::GetCompilerTypeImpl() { return m_cast_type; }
-size_t ValueObjectCast::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectCast::CalculateNumChildren(uint32_t max) {
ExecutionContext exe_ctx(GetExecutionContextRef());
auto children_count = GetCompilerType().GetNumChildren(
true, &exe_ctx);
diff --git a/lldb/source/Core/ValueObjectChild.cpp b/lldb/source/Core/ValueObjectChild.cpp
index 39067387dc9782..2e55dd7726bdd9 100644
--- a/lldb/source/Core/ValueObjectChild.cpp
+++ b/lldb/source/Core/ValueObjectChild.cpp
@@ -49,7 +49,7 @@ lldb::ValueType ValueObjectChild::GetValueType() const {
return m_parent->GetValueType();
}
-size_t ValueObjectChild::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectChild::CalculateNumChildren(uint32_t max) {
ExecutionContext exe_ctx(GetExecutionContextRef());
auto children_count = GetCompilerType().GetNumChildren(true, &exe_ctx);
return children_count <= max ? children_count : max;
diff --git a/lldb/source/Core/ValueObjectConstResult.cpp b/lldb/source/Core/ValueObjectConstResult.cpp
index 693da1a551f8eb..5c7aa4452b70db 100644
--- a/lldb/source/Core/ValueObjectConstResult.cpp
+++ b/lldb/source/Core/ValueObjectConstResult.cpp
@@ -216,7 +216,7 @@ std::optional<uint64_t> ValueObjectConstResult::GetByteSize() {
void ValueObjectConstResult::SetByteSize(size_t size) { m_byte_size = size; }
-size_t ValueObjectConstResult::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectConstResult::CalculateNumChildren(uint32_t max) {
ExecutionContext exe_ctx(GetExecutionContextRef());
auto children_count = GetCompilerType().GetNumChildren(true, &exe_ctx);
return children_count <= max ? children_count : max;
diff --git a/lldb/source/Core/ValueObjectDynamicValue.cpp b/lldb/source/Core/ValueObjectDynamicValue.cpp
index e6e30dce9d1e4a..4e64760371ae52 100644
--- a/lldb/source/Core/ValueObjectDynamicValue.cpp
+++ b/lldb/source/Core/ValueObjectDynamicValue.cpp
@@ -85,7 +85,7 @@ ConstString ValueObjectDynamicValue::GetDisplayTypeName() {
return m_parent->GetDisplayTypeName();
}
-size_t ValueObjectDynamicValue::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectDynamicValue::CalculateNumChildren(uint32_t max) {
const bool success = UpdateValueIfNeeded(false);
if (success && m_dynamic_type_info.HasType()) {
ExecutionContext exe_ctx(GetExecutionContextRef());
diff --git a/lldb/source/Core/ValueObjectMemory.cpp b/lldb/source/Core/ValueObjectMemory.cpp
index 3f125a7bee8c77..7f68236c7884ec 100644
--- a/lldb/source/Core/ValueObjectMemory.cpp
+++ b/lldb/source/Core/ValueObjectMemory.cpp
@@ -126,7 +126,7 @@ ConstString ValueObjectMemory::GetDisplayTypeName() {
return m_compiler_type.GetDisplayTypeName();
}
-size_t ValueObjectMemory::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectMemory::CalculateNumChildren(uint32_t max) {
if (m_type_sp) {
auto child_count = m_type_sp->GetNumChildren(true);
return child_count <= max ? child_count : max;
diff --git a/lldb/source/Core/ValueObjectRegister.cpp b/lldb/source/Core/ValueObjectRegister.cpp
index c2b84c11347359..d4c144cc7edb9a 100644
--- a/lldb/source/Core/ValueObjectRegister.cpp
+++ b/lldb/source/Core/ValueObjectRegister.cpp
@@ -74,7 +74,7 @@ ConstString ValueObjectRegisterSet::GetQualifiedTypeName() {
return ConstString();
}
-size_t ValueObjectRegisterSet::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectRegisterSet::CalculateNumChildren(uint32_t max) {
const RegisterSet *reg_set = m_reg_ctx_sp->GetRegisterSet(m_reg_set_idx);
if (reg_set) {
auto reg_count = reg_set->num_registers;
@@ -220,7 +220,7 @@ ConstString ValueObjectRegister::GetTypeName() {
return m_type_name;
}
-size_t ValueObjectRegister::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectRegister::CalculateNumChildren(uint32_t max) {
ExecutionContext exe_ctx(GetExecutionContextRef());
auto children_count = GetCompilerType().GetNumChildren(true, &exe_ctx);
return children_count <= max ? children_count : max;
diff --git a/lldb/source/Core/ValueObjectSyntheticFilter.cpp b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
index e8b4b02d11a0bb..ae358fba4bd815 100644
--- a/lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -84,7 +84,7 @@ ConstString ValueObjectSynthetic::GetDisplayTypeName() {
return m_parent->GetDisplayTypeName();
}
-size_t ValueObjectSynthetic::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectSynthetic::CalculateNumChildren(uint32_t max) {
Log *log = GetLog(LLDBLog::DataFormatters);
UpdateValueIfNeeded();
diff --git a/lldb/source/Core/ValueObjectVTable.cpp b/lldb/source/Core/ValueObjectVTable.cpp
index 177ae4167a1d45..4d1cbb8d2f6fc2 100644
--- a/lldb/source/Core/ValueObjectVTable.cpp
+++ b/lldb/source/Core/ValueObjectVTable.cpp
@@ -33,7 +33,7 @@ class ValueObjectVTableChild : public ValueObject {
std::optional<uint64_t> GetByteSize() override { return m_addr_size; };
- size_t CalculateNumChildren(uint32_t max) override { return 0; };
+ uint32_t CalculateNumChildren(uint32_t max) override { return 0; };
ValueType GetValueType() const override { return eValueTypeVTableEntry; };
@@ -159,7 +159,7 @@ std::optional<uint64_t> ValueObjectVTable::GetByteSize() {
return std::nullopt;
}
-size_t ValueObjectVTable::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectVTable::CalculateNumChildren(uint32_t max) {
if (UpdateValueIfNeeded(false))
return m_num_vtable_entries <= max ? m_num_vtable_entries : max;
return 0;
diff --git a/lldb/source/Core/ValueObjectVariable.cpp b/lldb/source/Core/ValueObjectVariable.cpp
index 9f8df847f28a8e..dc62bb6358dc97 100644
--- a/lldb/source/Core/ValueObjectVariable.cpp
+++ b/lldb/source/Core/ValueObjectVariable.cpp
@@ -94,7 +94,7 @@ ConstString ValueObjectVariable::GetQualifiedTypeName() {
return ConstString();
}
-size_t ValueObjectVariable::CalculateNumChildren(uint32_t max) {
+uint32_t ValueObjectVariable::CalculateNumChildren(uint32_t max) {
CompilerType type(GetCompilerType());
if (!type.IsValid())
|
In the end this value comes from TypeSystem::GetNumChildren which returns a uint32_t, so ValueObject should be consistent with that.
…int32_t This way it is consistent with ValueObject and TypeSystem.
adrian-prantl
force-pushed
the
calculatenumchildren
branch
from
056c198 to
242da8f
Compare
You can test this locally with the following command:git-clang-format --diff 21be2fbd17f9ff6f3f04e0ababc91c9cdd5aed85 79642bf2a0ee0c12071f1f1b8b33683c4708ae3d -- lldb/include/lldb/Core/ValueObject.h lldb/include/lldb/Core/ValueObjectCast.h lldb/include/lldb/Core/ValueObjectChild.h lldb/include/lldb/Core/ValueObjectConstResult.h lldb/include/lldb/Core/ValueObjectDynamicValue.h lldb/include/lldb/Core/ValueObjectMemory.h lldb/include/lldb/Core/ValueObjectRegister.h lldb/include/lldb/Core/ValueObjectSyntheticFilter.h lldb/include/lldb/Core/ValueObjectVTable.h lldb/include/lldb/Core/ValueObjectVariable.h lldb/include/lldb/DataFormatters/TypeSynthetic.h lldb/include/lldb/DataFormatters/VectorIterator.h lldb/include/lldb/Target/StackFrameRecognizer.h lldb/source/Core/ValueObject.cpp lldb/source/Core/ValueObjectCast.cpp lldb/source/Core/ValueObjectChild.cpp lldb/source/Core/ValueObjectConstResult.cpp lldb/source/Core/ValueObjectDynamicValue.cpp lldb/source/Core/ValueObjectMemory.cpp lldb/source/Core/ValueObjectRegister.cpp lldb/source/Core/ValueObjectSyntheticFilter.cpp lldb/source/Core/ValueObjectVTable.cpp lldb/source/Core/ValueObjectVariable.cpp lldb/source/DataFormatters/TypeSynthetic.cpp lldb/source/DataFormatters/VectorType.cpp lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp lldb/source/Plugins/Language/CPlusPlus/Coroutines.h lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxx.h lldb/source/Plugins/Language/CPlusPlus/LibCxxAtomic.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxInitializerList.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxQueue.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxRangesRefView.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxTuple.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxUnorderedMap.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxValarray.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxVariant.cpp lldb/source/Plugins/Language/CPlusPlus/LibCxxVector.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcppTuple.cpp lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp lldb/source/Plugins/Language/ObjC/Cocoa.cpp lldb/source/Plugins/Language/ObjC/NSArray.cpp lldb/source/Plugins/Language/ObjC/NSDictionary.cpp lldb/source/Plugins/Language/ObjC/NSError.cpp lldb/source/Plugins/Language/ObjC/NSException.cpp lldb/source/Plugins/Language/ObjC/NSIndexPath.cpp lldb/source/Plugins/Language/ObjC/NSSet.cppView the diff from clang-format here.diff --git a/lldb/source/Core/ValueObjectSyntheticFilter.cpp b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
index 7f8a9a34cb..e33a649dde 100644
--- a/lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -31,7 +31,9 @@ public:
DummySyntheticFrontEnd(ValueObject &backend)
: SyntheticChildrenFrontEnd(backend) {}
- uint32_t CalculateNumChildren() override { return m_backend.GetNumChildren(); }
+ uint32_t CalculateNumChildren() override {
+ return m_backend.GetNumChildren();
+ }
lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override {
return m_backend.GetChildAtIndex(idx);
diff --git a/lldb/source/DataFormatters/TypeSynthetic.cpp b/lldb/source/DataFormatters/TypeSynthetic.cpp
index 0ae38c4d31..b9ab5b0897 100644
--- a/lldb/source/DataFormatters/TypeSynthetic.cpp
+++ b/lldb/source/DataFormatters/TypeSynthetic.cpp
@@ -184,7 +184,8 @@ uint32_t ScriptedSyntheticChildren::FrontEnd::CalculateNumChildren() {
return m_interpreter->CalculateNumChildren(m_wrapper_sp, UINT32_MAX);
}
-uint32_t ScriptedSyntheticChildren::FrontEnd::CalculateNumChildren(uint32_t max) {
+uint32_t
+ScriptedSyntheticChildren::FrontEnd::CalculateNumChildren(uint32_t max) {
if (!m_wrapper_sp || m_interpreter == nullptr)
return 0;
return m_interpreter->CalculateNumChildren(m_wrapper_sp, max);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 5abb3d5067..43c7dff2c3 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -371,7 +371,9 @@ LibStdcppSharedPtrSyntheticFrontEnd::LibStdcppSharedPtrSyntheticFrontEnd(
Update();
}
-uint32_t LibStdcppSharedPtrSyntheticFrontEnd::CalculateNumChildren() { return 1; }
+uint32_t LibStdcppSharedPtrSyntheticFrontEnd::CalculateNumChildren() {
+ return 1;
+}
lldb::ValueObjectSP
LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
diff --git a/lldb/source/Plugins/Language/ObjC/NSArray.cpp b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
index 7f060b2613..76d93d0f1e 100644
--- a/lldb/source/Plugins/Language/ObjC/NSArray.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSArray.cpp
@@ -634,9 +634,8 @@ lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<D32, D64, Inline>::
}
template <typename D32, typename D64, bool Inline>
-uint32_t
-lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<D32, D64, Inline>::
- CalculateNumChildren() {
+uint32_t lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<
+ D32, D64, Inline>::CalculateNumChildren() {
return m_data_32 ? m_data_32->used : m_data_64->used;
}
@@ -682,9 +681,8 @@ lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<D32, D64, Inline>::
}
template <typename D32, typename D64, bool Inline>
-lldb::ValueObjectSP
-lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<D32, D64, Inline>::
- GetChildAtIndex(uint32_t idx) {
+lldb::ValueObjectSP lldb_private::formatters::GenericNSArrayISyntheticFrontEnd<
+ D32, D64, Inline>::GetChildAtIndex(uint32_t idx) {
if (idx >= CalculateNumChildren())
return lldb::ValueObjectSP();
lldb::addr_t object_at_idx;
diff --git a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
index da94eda152..fc8b082a5e 100644
--- a/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSDictionary.cpp
@@ -262,9 +262,9 @@ namespace Foundation1100 {
NSDictionaryMSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp);
~NSDictionaryMSyntheticFrontEnd() override;
-
+
uint32_t CalculateNumChildren() override;
-
+
lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override;
lldb::ChildCacheState Update() override;
@@ -1087,8 +1087,8 @@ size_t lldb_private::formatters::GenericNSDictionaryMSyntheticFrontEnd<
}
template <typename D32, typename D64>
-uint32_t
-lldb_private::formatters::GenericNSDictionaryMSyntheticFrontEnd<D32,D64>::CalculateNumChildren() {
+uint32_t lldb_private::formatters::GenericNSDictionaryMSyntheticFrontEnd<
+ D32, D64>::CalculateNumChildren() {
if (!m_data_32 && !m_data_64)
return 0;
return (m_data_32 ? m_data_32->_used : m_data_64->_used);
@@ -1250,9 +1250,8 @@ lldb_private::formatters::Foundation1100::
return idx;
}
-uint32_t
-lldb_private::formatters::Foundation1100::
- NSDictionaryMSyntheticFrontEnd::CalculateNumChildren() {
+uint32_t lldb_private::formatters::Foundation1100::
+ NSDictionaryMSyntheticFrontEnd::CalculateNumChildren() {
if (!m_data_32 && !m_data_64)
return 0;
return (m_data_32 ? m_data_32->_used : m_data_64->_used);
@@ -1298,9 +1297,8 @@ lldb_private::formatters::Foundation1100::
return true;
}
-lldb::ValueObjectSP
-lldb_private::formatters::Foundation1100::
- NSDictionaryMSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
+lldb::ValueObjectSP lldb_private::formatters::Foundation1100::
+ NSDictionaryMSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
lldb::addr_t m_keys_ptr =
(m_data_32 ? m_data_32->_keys_addr : m_data_64->_keys_addr);
lldb::addr_t m_values_ptr =
diff --git a/lldb/source/Plugins/Language/ObjC/NSException.cpp b/lldb/source/Plugins/Language/ObjC/NSException.cpp
index 09d3a1b42b..8612c542c9 100644
--- a/lldb/source/Plugins/Language/ObjC/NSException.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSException.cpp
@@ -123,9 +123,7 @@ public:
~NSExceptionSyntheticFrontEnd() override = default;
- uint32_t CalculateNumChildren() override {
- return 4;
- }
+ uint32_t CalculateNumChildren() override { return 4; }
lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override {
switch (idx) {
diff --git a/lldb/source/Plugins/Language/ObjC/NSSet.cpp b/lldb/source/Plugins/Language/ObjC/NSSet.cpp
index c965a2a134..98754620ab 100644
--- a/lldb/source/Plugins/Language/ObjC/NSSet.cpp
+++ b/lldb/source/Plugins/Language/ObjC/NSSet.cpp
@@ -697,9 +697,8 @@ lldb_private::formatters::
}
template <typename D32, typename D64>
-uint32_t
-lldb_private::formatters::
- GenericNSSetMSyntheticFrontEnd<D32, D64>::CalculateNumChildren() {
+uint32_t lldb_private::formatters::GenericNSSetMSyntheticFrontEnd<
+ D32, D64>::CalculateNumChildren() {
if (!m_data_32 && !m_data_64)
return 0;
return (m_data_32 ? m_data_32->_used : m_data_64->_used);
@@ -747,9 +746,8 @@ lldb_private::formatters::
}
template <typename D32, typename D64>
-lldb::ValueObjectSP
-lldb_private::formatters::
- GenericNSSetMSyntheticFrontEnd<D32, D64>::GetChildAtIndex(uint32_t idx) {
+lldb::ValueObjectSP lldb_private::formatters::GenericNSSetMSyntheticFrontEnd<
+ D32, D64>::GetChildAtIndex(uint32_t idx) {
lldb::addr_t m_objs_addr =
(m_data_32 ? m_data_32->_objs_addr : m_data_64->_objs_addr);
|
|
Theoretically someone could make a synthetic type or an array with more than UINT32_MAX elements, and with this change we can't support displaying that. I did get one bug report that this didn't work a few years back, though this likely a very uncommon case. If it were easy to go the other way, and pipe num_children as a size_t meaningfully through clang and lldb, that would be formally better. |
|
Why not increase TypeSystem::GetNumChildren to return a size_t instead? |
We could, but even 2^32 seems like an outrageous amount of children to me. What would be a use-case for this? A ValueObject that has the entire address space as synthetic children? There is other code that assumes that we can store the result of GetNumChildren() in an int64 and use negative numbers as out-of-band signaling. We could replace that with std::optional of course. |
|
I got one bug report - which I can't currently find - from someone who had managed to make a thing this big and we wouldn't print elements out of it correctly. We didn't do anything about it at the time, it did seem silly.
But it WAS actually silly, at least once.
Jim
… On Mar 1, 2024, at 4:36 PM, Adrian Prantl ***@***.***> wrote:
Why not increase TypeSystem::GetNumChildren to return a size_t instead?
We could, but even 2^32 seems like an outrageous amount of children to me. What would be a use-case for this? A ValueObject that has the entire address space as synthetic children?
There is other code that assumes that we can store the result of GetNumChildren() in an int64 and use negative numbers as out-of-band signaling. We could replace that with std::optional of course.
—
Reply to this email directly, view it on GitHub <#83501 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVWZOEMBLVZPRVS4J7ODYWENH3AVCNFSM6AAAAABEAZQRFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZUGEZTIMZXGM>.
You are receiving this because you are on a team that was mentioned.
|
But what's there to gain by limiting it to 32 bits instead of 64? Saying it's unlikely or overkill isn't a very strong argument. I think a possibly better answer to Alex' question is probably: because |
There was a problem hiding this comment.
I would almost vote to change everything to uint64_t except for the public API since we can't change the API without breaking. Though I winder if we can actually change this one:
uint64_t SBValue::GetNumChildren();
Since the return value isn't mangled into the function name?
The reason I mention the uint64_t is lldb::SBValue and lldb_private::ValueObject* can represent any object that can be expanded. We could have a lldb::SBValue that represents all of memory in a process where each object can represent an area in memory as a specific format and size. So a 64 bit process could have a SBValue with UINT64_MAX children available if we wanted to have a SBValue that represented memory as uint8_t objects.
So if we are going to change stuff around, I would vote to use uint64_t instead of uint32_t
|
I would vote for |
Changing the type of a return value if it changes the size/layout is ABI breaking. The mangled name wouldn't change but it may silently break something later without us realizing. |
|
My main point was to unify it, but I have no problem with going for |
adrian-prantl
force-pushed
the
calculatenumchildren
branch
from
242da8f to
e9c706f
Compare
lldb/include/lldb/Core/ValueObject.h
Outdated
| @@ -958,7 +958,7 @@ class ValueObject { | |||
| int32_t synthetic_index); | |||
|
|
|||
| /// Should only be called by ValueObject::GetNumChildren(). | |||
| virtual size_t CalculateNumChildren(uint32_t max = UINT32_MAX) = 0; | |||
| virtual uint64_t CalculateNumChildren(uint64_t max = UINT32_MAX) = 0; | |||
There was a problem hiding this comment.
As long as we are changing things, do we want to pass in a std::optional<uint64_t> max? If not, then switch this to UINT64_MAX.
There was a problem hiding this comment.
Oh that's separately a good change to make apart from the return value I mentioned below.
The entire point of this patch series is to prepare for a final patch that will change the return value to |
I was suggesting this for the input parameters only. But we can do this later. |
|
Folks I'm talking this all back. Changing the return type from a uint32_t to a uint64_t is an all-or-nothing affair, because the UINT32_MAX sentinel value is permeating a bunch of other APIs that I have not intention of changing right now. For example, |
adrian-prantl
force-pushed
the
calculatenumchildren
branch
from
bbf8e4b to
79642bf
Compare
|
So I reverted back to the state where I convert everything to int32_t. This way I don't have to touch the UINT32_MAX sentinels in this patch, which should be done separately and very carefully. |
@bulbazord I discovered that the idea that these are 32-bit values is deeply baked into the code by use of UINT32_MAX sentinel values in multiple places. We can of course change that too at some point, but we're really not giving anything up with this patch that isn't already a limitation. |
If the goal is to migrate to 64-bit everywhere, yes. But we'd need to do this on a bunch more APIs. If you feel strongly we need to do this I can do it, but the patch will get much larger this way. |
There was a problem hiding this comment.
As long as the changes are planned for quick follow up patches I am good not changing it up front. And we can probably just switch to using std::optional<uint64_t> instead of switching things to use UINT32_MAX anyway with a follow up patch which is even better.
|
Merged in e710523 |
…xpected (#84219) Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected This is an NFC change that does not yet add any error handling or change any code to return any errors. This is the second big change in the patch series started with #83501 A follow-up PR will wire up error handling.
…xpected (#84219) Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected This is an NFC change that does not yet add any error handling or change any code to return any errors. This is the second big change in the patch series started with #83501 A follow-up PR will wire up error handling.
…xpected (llvm#84219) Change GetNumChildren()/CalculateNumChildren() methods return llvm::Expected This is an NFC change that does not yet add any error handling or change any code to return any errors. This is the second big change in the patch series started with llvm#83501 A follow-up PR will wire up error handling. (cherry picked from commit 624ea68)
This is a proof-of-concept patch that illustrates how to use the Expected return values to surface rich error messages all the way up to the ValueObjectPrinter. This is the final patch in the series that includes llvm#83501 and llvm#84219 (cherry picked from commit 6462ead)
In the end this value comes from TypeSystem::GetNumChildren which returns a uint32_t, so ValueObject should be consistent with that.