[Lldb-commits] [lldb] 8f63cf5 - [lldb][NFC] Cleanup ValueObject construction code
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 23 00:39:43 PST 2021
Author: Raphael Isemann
Date: 2021-02-23T09:39:18+01:00
New Revision: 8f63cf5da3c098f5f16a1055d6e13d4bcf399a27
URL: https://github.com/llvm/llvm-project/commit/8f63cf5da3c098f5f16a1055d6e13d4bcf399a27
DIFF: https://github.com/llvm/llvm-project/commit/8f63cf5da3c098f5f16a1055d6e13d4bcf399a27.diff
LOG: [lldb][NFC] Cleanup ValueObject construction code
Just code cleanup for ValueObject constructors:
* Use default member initializers where possible.
* Doxygenify the comments for membersa nd constructors where needed.
* Delete the default constructor which isn't defined.
* Initialize the bitfields via a utility struct instead of doing this in the
different constructors.
Reviewed By: JDevlieghere
Differential Revision: https://reviews.llvm.org/D97199
Added:
Modified:
lldb/include/lldb/Core/ValueObject.h
lldb/source/Core/ValueObject.cpp
lldb/source/Core/ValueObjectSyntheticFilter.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Core/ValueObject.h b/lldb/include/lldb/Core/ValueObject.h
index a665e7afa0ca..e8c7defa330c 100644
--- a/lldb/include/lldb/Core/ValueObject.h
+++ b/lldb/include/lldb/Core/ValueObject.h
@@ -420,7 +420,9 @@ class ValueObject : public UserID {
return (GetBitfieldBitSize() != 0) || (GetBitfieldBitOffset() != 0);
}
- virtual bool IsArrayItemForPointer() { return m_is_array_item_for_pointer; }
+ virtual bool IsArrayItemForPointer() {
+ return m_flags.m_is_array_item_for_pointer;
+ }
virtual const char *GetValueAsCString();
@@ -744,7 +746,9 @@ class ValueObject : public UserID {
AddressType GetAddressTypeOfChildren();
- void SetHasCompleteType() { m_did_calculate_complete_objc_class_type = true; }
+ void SetHasCompleteType() {
+ m_flags.m_did_calculate_complete_objc_class_type = true;
+ }
/// Find out if a ValueObject might have children.
///
@@ -815,76 +819,95 @@ class ValueObject : public UserID {
};
// Classes that inherit from ValueObject can see and modify these
- ValueObject
- *m_parent; // The parent value object, or nullptr if this has no parent
- ValueObject *m_root; // The root of the hierarchy for this ValueObject (or
- // nullptr if never calculated)
- EvaluationPoint m_update_point; // Stores both the stop id and the full
- // context at which this value was last
- // updated. When we are asked to update the value object, we check whether
- // the context & stop id are the same before updating.
- ConstString m_name; // The name of this object
- DataExtractor
- m_data; // A data extractor that can be used to extract the value.
+
+ /// The parent value object, or nullptr if this has no parent.
+ ValueObject *m_parent = nullptr;
+ /// The root of the hierarchy for this ValueObject (or nullptr if never
+ /// calculated).
+ ValueObject *m_root = nullptr;
+ /// Stores both the stop id and the full context at which this value was last
+ /// updated. When we are asked to update the value object, we check whether
+ /// the context & stop id are the same before updating.
+ EvaluationPoint m_update_point;
+ /// The name of this object.
+ ConstString m_name;
+ /// A data extractor that can be used to extract the value.
+ DataExtractor m_data;
Value m_value;
- Status
- m_error; // An error object that can describe any errors that occur when
- // updating values.
- std::string m_value_str; // Cached value string that will get cleared if/when
- // the value is updated.
- std::string m_old_value_str; // Cached old value string from the last time the
- // value was gotten
- std::string m_location_str; // Cached location string that will get cleared
- // if/when the value is updated.
- std::string m_summary_str; // Cached summary string that will get cleared
- // if/when the value is updated.
- std::string m_object_desc_str; // Cached result of the "object printer". This
- //
diff ers from the summary
- // in that the summary is consed up by us, the object_desc_string is builtin.
-
- CompilerType m_override_type; // If the type of the value object should be
- // overridden, the type to impose.
-
- ValueObjectManager *m_manager; // This object is managed by the root object
- // (any ValueObject that gets created
- // without a parent.) The manager gets passed through all the generations of
- // dependent objects, and will keep the whole cluster of objects alive as
- // long as a shared pointer to any of them has been handed out. Shared
- // pointers to value objects must always be made with the GetSP method.
+ /// An error object that can describe any errors that occur when updating
+ /// values.
+ Status m_error;
+ /// Cached value string that will get cleared if/when the value is updated.
+ std::string m_value_str;
+ /// Cached old value string from the last time the value was gotten
+ std::string m_old_value_str;
+ /// Cached location string that will get cleared if/when the value is updated.
+ std::string m_location_str;
+ /// Cached summary string that will get cleared if/when the value is updated.
+ std::string m_summary_str;
+ /// Cached result of the "object printer". This
diff ers from the summary
+ /// in that the summary is consed up by us, the object_desc_string is builtin.
+ std::string m_object_desc_str;
+ /// If the type of the value object should be overridden, the type to impose.
+ CompilerType m_override_type;
+
+ /// This object is managed by the root object (any ValueObject that gets
+ /// created without a parent.) The manager gets passed through all the
+ /// generations of dependent objects, and will keep the whole cluster of
+ /// objects alive as long as a shared pointer to any of them has been handed
+ /// out. Shared pointers to value objects must always be made with the GetSP
+ /// method.
+ ValueObjectManager *m_manager = nullptr;
ChildrenManager m_children;
std::map<ConstString, ValueObject *> m_synthetic_children;
- ValueObject *m_dynamic_value;
- ValueObject *m_synthetic_value;
- ValueObject *m_deref_valobj;
+ ValueObject *m_dynamic_value = nullptr;
+ ValueObject *m_synthetic_value = nullptr;
+ ValueObject *m_deref_valobj = nullptr;
- lldb::ValueObjectSP m_addr_of_valobj_sp; // We have to hold onto a shared
- // pointer to this one because it is
- // created
- // as an independent ValueObjectConstResult, which isn't managed by us.
+ /// We have to hold onto a shared pointer to this one because it is created
+ /// as an independent ValueObjectConstResult, which isn't managed by us.
+ lldb::ValueObjectSP m_addr_of_valobj_sp;
- lldb::Format m_format;
- lldb::Format m_last_format;
- uint32_t m_last_format_mgr_revision;
+ lldb::Format m_format = lldb::eFormatDefault;
+ lldb::Format m_last_format = lldb::eFormatDefault;
+ uint32_t m_last_format_mgr_revision = 0;
lldb::TypeSummaryImplSP m_type_summary_sp;
lldb::TypeFormatImplSP m_type_format_sp;
lldb::SyntheticChildrenSP m_synthetic_children_sp;
ProcessModID m_user_id_of_forced_summary;
- AddressType m_address_type_of_ptr_or_ref_children;
+ AddressType m_address_type_of_ptr_or_ref_children = eAddressTypeInvalid;
llvm::SmallVector<uint8_t, 16> m_value_checksum;
- lldb::LanguageType m_preferred_display_language;
-
- uint64_t m_language_flags;
-
- bool m_value_is_valid : 1, m_value_did_change : 1, m_children_count_valid : 1,
- m_old_value_valid : 1, m_is_deref_of_parent : 1,
- m_is_array_item_for_pointer : 1, m_is_bitfield_for_scalar : 1,
- m_is_child_at_offset : 1, m_is_getting_summary : 1,
- m_did_calculate_complete_objc_class_type : 1,
- m_is_synthetic_children_generated : 1;
+ lldb::LanguageType m_preferred_display_language = lldb::eLanguageTypeUnknown;
+
+ uint64_t m_language_flags = 0;
+
+ // Utility class for initializing all bitfields in ValueObject's constructors.
+ // FIXME: This could be done via default initializers once we have C++20.
+ struct Bitflags {
+ bool m_value_is_valid : 1, m_value_did_change : 1,
+ m_children_count_valid : 1, m_old_value_valid : 1,
+ m_is_deref_of_parent : 1, m_is_array_item_for_pointer : 1,
+ m_is_bitfield_for_scalar : 1, m_is_child_at_offset : 1,
+ m_is_getting_summary : 1, m_did_calculate_complete_objc_class_type : 1,
+ m_is_synthetic_children_generated : 1;
+ Bitflags() {
+ m_value_is_valid = false;
+ m_value_did_change = false;
+ m_children_count_valid = false;
+ m_old_value_valid = false;
+ m_is_deref_of_parent = false;
+ m_is_array_item_for_pointer = false;
+ m_is_bitfield_for_scalar = false;
+ m_is_child_at_offset = false;
+ m_is_getting_summary = false;
+ m_did_calculate_complete_objc_class_type = false;
+ m_is_synthetic_children_generated = false;
+ }
+ } m_flags;
friend class ValueObjectChild;
friend class ExpressionVariable; // For SetName
@@ -892,22 +915,13 @@ class ValueObject : public UserID {
friend class ValueObjectConstResultImpl;
friend class ValueObjectSynthetic; // For ClearUserVisibleData
- // Constructors and Destructors
-
- // Use the no-argument constructor to make a constant variable object (with
- // no ExecutionContextScope.)
-
- ValueObject();
-
- // Use this constructor to create a "root variable object". The ValueObject
- // will be locked to this context through-out its lifespan.
-
+ /// Use this constructor to create a "root variable object". The ValueObject
+ /// will be locked to this context through-out its lifespan.
ValueObject(ExecutionContextScope *exe_scope, ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type = eAddressTypeLoad);
- // Use this constructor to create a ValueObject owned by another ValueObject.
- // It will inherit the ExecutionContext of its parent.
-
+ /// Use this constructor to create a ValueObject owned by another ValueObject.
+ /// It will inherit the ExecutionContext of its parent.
ValueObject(ValueObject &parent);
ValueObjectManager *GetManager() { return m_manager; }
diff --git a/lldb/source/Core/ValueObject.cpp b/lldb/source/Core/ValueObject.cpp
index 93fb5009f95b..8f1d4e8cc7e5 100644
--- a/lldb/source/Core/ValueObject.cpp
+++ b/lldb/source/Core/ValueObject.cpp
@@ -77,26 +77,10 @@ static user_id_t g_value_obj_uid = 0;
// ValueObject constructor
ValueObject::ValueObject(ValueObject &parent)
: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(&parent), m_root(nullptr),
- m_update_point(parent.GetUpdatePoint()), m_name(), m_data(), m_value(),
- m_error(), m_value_str(), m_old_value_str(), m_location_str(),
- m_summary_str(), m_object_desc_str(), m_manager(parent.GetManager()),
- m_children(), m_synthetic_children(), m_dynamic_value(nullptr),
- m_synthetic_value(nullptr), m_deref_valobj(nullptr),
- m_format(eFormatDefault), m_last_format(eFormatDefault),
- m_last_format_mgr_revision(0), m_type_summary_sp(), m_type_format_sp(),
- m_synthetic_children_sp(), m_user_id_of_forced_summary(),
- m_address_type_of_ptr_or_ref_children(eAddressTypeInvalid),
- m_value_checksum(),
- m_preferred_display_language(lldb::eLanguageTypeUnknown),
- m_language_flags(0), m_value_is_valid(false), m_value_did_change(false),
- m_children_count_valid(false), m_old_value_valid(false),
- m_is_deref_of_parent(false), m_is_array_item_for_pointer(false),
- m_is_bitfield_for_scalar(false), m_is_child_at_offset(false),
- m_is_getting_summary(false),
- m_did_calculate_complete_objc_class_type(false),
- m_is_synthetic_children_generated(
- parent.m_is_synthetic_children_generated) {
+ m_parent(&parent), m_update_point(parent.GetUpdatePoint()),
+ m_manager(parent.GetManager()) {
+ m_flags.m_is_synthetic_children_generated =
+ parent.m_flags.m_is_synthetic_children_generated;
m_data.SetByteOrder(parent.GetDataExtractor().GetByteOrder());
m_data.SetAddressByteSize(parent.GetDataExtractor().GetAddressByteSize());
m_manager->ManageObject(this);
@@ -107,25 +91,8 @@ ValueObject::ValueObject(ExecutionContextScope *exe_scope,
ValueObjectManager &manager,
AddressType child_ptr_or_ref_addr_type)
: UserID(++g_value_obj_uid), // Unique identifier for every value object
- m_parent(nullptr), m_root(nullptr), m_update_point(exe_scope), m_name(),
- m_data(), m_value(), m_error(), m_value_str(), m_old_value_str(),
- m_location_str(), m_summary_str(), m_object_desc_str(),
- m_manager(&manager), m_children(), m_synthetic_children(),
- m_dynamic_value(nullptr), m_synthetic_value(nullptr),
- m_deref_valobj(nullptr), m_format(eFormatDefault),
- m_last_format(eFormatDefault), m_last_format_mgr_revision(0),
- m_type_summary_sp(), m_type_format_sp(), m_synthetic_children_sp(),
- m_user_id_of_forced_summary(),
- m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type),
- m_value_checksum(),
- m_preferred_display_language(lldb::eLanguageTypeUnknown),
- m_language_flags(0), m_value_is_valid(false), m_value_did_change(false),
- m_children_count_valid(false), m_old_value_valid(false),
- m_is_deref_of_parent(false), m_is_array_item_for_pointer(false),
- m_is_bitfield_for_scalar(false), m_is_child_at_offset(false),
- m_is_getting_summary(false),
- m_did_calculate_complete_objc_class_type(false),
- m_is_synthetic_children_generated(false) {
+ m_update_point(exe_scope), m_manager(&manager),
+ m_address_type_of_ptr_or_ref_children(child_ptr_or_ref_addr_type) {
if (exe_scope) {
TargetSP target_sp(exe_scope->CalculateTarget());
if (target_sp) {
@@ -170,9 +137,9 @@ bool ValueObject::UpdateValueIfNeeded(bool update_format) {
// Save the old value using swap to avoid a string copy which also will
// clear our m_value_str
if (m_value_str.empty()) {
- m_old_value_valid = false;
+ m_flags.m_old_value_valid = false;
} else {
- m_old_value_valid = true;
+ m_flags.m_old_value_valid = true;
m_old_value_str.swap(m_value_str);
ClearUserVisibleData(eClearUserVisibleDataItemsValue);
}
@@ -215,7 +182,7 @@ bool ValueObject::UpdateValueIfNeeded(bool update_format) {
if (first_update)
SetValueDidChange(false);
- else if (!m_value_did_change && !success) {
+ else if (!m_flags.m_value_did_change && !success) {
// The value wasn't gotten successfully, so we mark this as changed if
// the value used to be valid and now isn't
SetValueDidChange(value_was_valid);
@@ -266,8 +233,8 @@ void ValueObject::SetNeedsUpdate() {
}
void ValueObject::ClearDynamicTypeInformation() {
- m_children_count_valid = false;
- m_did_calculate_complete_objc_class_type = false;
+ m_flags.m_children_count_valid = false;
+ m_flags.m_did_calculate_complete_objc_class_type = false;
m_last_format_mgr_revision = 0;
m_override_type = CompilerType();
SetValueFormat(lldb::TypeFormatImplSP());
@@ -278,14 +245,14 @@ void ValueObject::ClearDynamicTypeInformation() {
CompilerType ValueObject::MaybeCalculateCompleteType() {
CompilerType compiler_type(GetCompilerTypeImpl());
- if (m_did_calculate_complete_objc_class_type) {
+ if (m_flags.m_did_calculate_complete_objc_class_type) {
if (m_override_type.IsValid())
return m_override_type;
else
return compiler_type;
}
- m_did_calculate_complete_objc_class_type = true;
+ m_flags.m_did_calculate_complete_objc_class_type = true;
ProcessSP process_sp(
GetUpdatePoint().GetExecutionContextRef().GetProcessSP());
@@ -418,14 +385,14 @@ bool ValueObject::IsLogicalTrue(Status &error) {
return ret;
}
-bool ValueObject::GetValueIsValid() const { return m_value_is_valid; }
+bool ValueObject::GetValueIsValid() const { return m_flags.m_value_is_valid; }
-void ValueObject::SetValueIsValid(bool b) { m_value_is_valid = b; }
+void ValueObject::SetValueIsValid(bool b) { m_flags.m_value_is_valid = b; }
-bool ValueObject::GetValueDidChange() { return m_value_did_change; }
+bool ValueObject::GetValueDidChange() { return m_flags.m_value_did_change; }
void ValueObject::SetValueDidChange(bool value_changed) {
- m_value_did_change = value_changed;
+ m_flags.m_value_did_change = value_changed;
}
ValueObjectSP ValueObject::GetChildAtIndex(size_t idx, bool can_create) {
@@ -553,14 +520,14 @@ size_t ValueObject::GetNumChildren(uint32_t max) {
UpdateValueIfNeeded();
if (max < UINT32_MAX) {
- if (m_children_count_valid) {
+ if (m_flags.m_children_count_valid) {
size_t children_count = m_children.GetChildrenCount();
return children_count <= max ? children_count : max;
} else
return CalculateNumChildren(max);
}
- if (!m_children_count_valid) {
+ if (!m_flags.m_children_count_valid) {
SetNumChildren(CalculateNumChildren());
}
return m_children.GetChildrenCount();
@@ -580,7 +547,7 @@ bool ValueObject::MightHaveChildren() {
// Should only be called by ValueObject::GetNumChildren()
void ValueObject::SetNumChildren(size_t num_children) {
- m_children_count_valid = true;
+ m_flags.m_children_count_valid = true;
m_children.SetChildrenCount(num_children);
}
@@ -654,10 +621,10 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
// ideally we would like to bail out if passing NULL, but if we do so we end
// up not providing the summary for function pointers anymore
- if (/*summary_ptr == NULL ||*/ m_is_getting_summary)
+ if (/*summary_ptr == NULL ||*/ m_flags.m_is_getting_summary)
return false;
- m_is_getting_summary = true;
+ m_flags.m_is_getting_summary = true;
TypeSummaryOptions actual_options(options);
@@ -680,7 +647,7 @@ bool ValueObject::GetSummaryAsCString(TypeSummaryImpl *summary_ptr,
// up-to-date (e.g. ${svar%#})
summary_ptr->FormatObject(this, destination, actual_options);
}
- m_is_getting_summary = false;
+ m_flags.m_is_getting_summary = false;
return !destination.empty();
}
@@ -1110,7 +1077,7 @@ const char *ValueObject::GetValueAsCString() {
if (m_type_format_sp)
format_sp = m_type_format_sp;
else {
- if (m_is_bitfield_for_scalar)
+ if (m_flags.m_is_bitfield_for_scalar)
my_format = eFormatUnsigned;
else {
if (m_value.GetContextType() == Value::ContextType::RegisterInfo) {
@@ -1128,7 +1095,7 @@ const char *ValueObject::GetValueAsCString() {
if (!format_sp)
format_sp = std::make_shared<TypeFormatImpl_Format>(my_format);
if (GetValueAsCString(*format_sp.get(), m_value_str)) {
- if (!m_value_did_change && m_old_value_valid) {
+ if (!m_flags.m_value_did_change && m_flags.m_old_value_valid) {
// The value was gotten successfully, so we consider the value as
// changed if the value string
diff ers
SetValueDidChange(m_old_value_str != m_value_str);
@@ -1725,7 +1692,7 @@ ValueObjectSP ValueObject::GetSyntheticArrayMember(size_t index,
AddSyntheticChild(index_const_str, synthetic_child);
synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(ConstString(index_str));
- synthetic_child_sp->m_is_array_item_for_pointer = true;
+ synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true;
}
}
}
@@ -1759,7 +1726,7 @@ ValueObjectSP ValueObject::GetSyntheticBitFieldChild(uint32_t from, uint32_t to,
AddSyntheticChild(index_const_str, synthetic_child);
synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(ConstString(index_str));
- synthetic_child_sp->m_is_bitfield_for_scalar = true;
+ synthetic_child_sp->m_flags.m_is_bitfield_for_scalar = true;
}
}
}
@@ -1798,7 +1765,7 @@ ValueObjectSP ValueObject::GetSyntheticChildAtOffset(
AddSyntheticChild(name_const_str, synthetic_child);
synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(name_const_str);
- synthetic_child_sp->m_is_child_at_offset = true;
+ synthetic_child_sp->m_flags.m_is_child_at_offset = true;
}
return synthetic_child_sp;
}
@@ -1990,7 +1957,7 @@ void ValueObject::GetExpressionPath(Stream &s,
// sometimes they are consed up in ways that don't make sense from an
// underlying language/API standpoint. So, use a special code path here to
// return something that can hopefully be used in expression
- if (m_is_synthetic_children_generated) {
+ if (m_flags.m_is_synthetic_children_generated) {
UpdateValueIfNeeded();
if (m_value.GetValueType() == Value::ValueType::LoadAddress) {
@@ -2038,7 +2005,7 @@ void ValueObject::GetExpressionPath(Stream &s,
// if we are a deref_of_parent just because we are synthetic array members
// made up to allow ptr[%d] syntax to work in variable printing, then add our
// name ([%d]) to the expression path
- if (m_is_array_item_for_pointer &&
+ if (m_flags.m_is_array_item_for_pointer &&
epformat == eGetExpressionPathFormatHonorPointers)
s.PutCString(m_name.GetStringRef());
@@ -3253,11 +3220,11 @@ ValueObjectSP ValueObject::Persist() {
}
bool ValueObject::IsSyntheticChildrenGenerated() {
- return m_is_synthetic_children_generated;
+ return m_flags.m_is_synthetic_children_generated;
}
void ValueObject::SetSyntheticChildrenGenerated(bool b) {
- m_is_synthetic_children_generated = b;
+ m_flags.m_is_synthetic_children_generated = b;
}
uint64_t ValueObject::GetLanguageFlags() { return m_language_flags; }
diff --git a/lldb/source/Core/ValueObjectSyntheticFilter.cpp b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
index cebf7abfe523..aa55e3965706 100644
--- a/lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ b/lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -190,7 +190,7 @@ bool ValueObjectSynthetic::UpdateValue() {
// children count for a synthetic VO that might indeed happen, so we need
// to tell the upper echelons that they need to come back to us asking for
// children
- m_children_count_valid = false;
+ m_flags.m_children_count_valid = false;
{
std::lock_guard<std::mutex> guard(m_child_mutex);
m_synthetic_children_cache.clear();
More information about the lldb-commits
mailing list