jankratochvil created this revision. jankratochvil added reviewers: aprantl, clayborg. jankratochvil added a project: LLDB. Herald added a subscriber: JDevlieghere.
As @aprantl suggested <https://reviews.llvm.org/D53255#1265520> I have merged `DWARFDebugInfoEntry`'s `m_empty_children` into `m_has_children`. `m_empty_children` was implemented by https://reviews.llvm.org/rL144983. I do not see why @clayborg made it a separate flag, from some point of view it is sure cleaner but technically I do not see its purpose. I have checked all calls of `HasChildren()` that it should not matter to them. The code even wants to know if there are any children - it does not matter how the children presence is coded in the binary. The only problematic may be `operator==` (and `!=`) although that one is currently only used by my `lldbassert()` in my bugfixed https://reviews.llvm.org/D53255. This patch sure has no regressions on Fedora 28 x86_64. I do not need/request this patch, just that @aprantl asked about it. Repository: rLLDB LLDB https://reviews.llvm.org/D53321 Files: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp @@ -223,7 +223,7 @@ // the list (saves up to 25% in C++ code), we need a way to let the // DIE know that it actually doesn't have children. if (!m_die_array.empty()) - m_die_array.back().SetEmptyChildren(true); + m_die_array.back().SetHasChildren(false); } } else { die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]); @@ -263,7 +263,7 @@ if (!m_die_array.empty()) { if (m_first_die) { // Only needed for the assertion. - m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren()); + m_first_die.SetHasChildren(m_die_array.front().HasChildren()); lldbassert(m_first_die == m_die_array.front()); } m_first_die = m_die_array.front(); Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h +++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h @@ -57,8 +57,7 @@ DWARFDebugInfoEntry() : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0), - m_empty_children(false), m_abbr_idx(0), m_has_children(false), - m_tag(0) {} + m_abbr_idx(0), m_has_children(false), m_tag(0) {} explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; } bool operator==(const DWARFDebugInfoEntry &rhs) const; @@ -227,10 +226,10 @@ // we don't need to store our child pointer, if we have a child it will // be the next entry in the list... DWARFDebugInfoEntry *GetFirstChild() { - return (HasChildren() && !m_empty_children) ? this + 1 : NULL; + return HasChildren() ? this + 1 : NULL; } const DWARFDebugInfoEntry *GetFirstChild() const { - return (HasChildren() && !m_empty_children) ? this + 1 : NULL; + return HasChildren() ? this + 1 : NULL; } void GetDeclContextDIEs(DWARFUnit *cu, @@ -271,10 +270,6 @@ void SetParentIndex(uint32_t idx) { m_parent_idx = idx; } - bool GetEmptyChildren() const { return m_empty_children; } - - void SetEmptyChildren(bool b) { m_empty_children = b; } - static void DumpDIECollection(lldb_private::Stream &strm, DWARFDebugInfoEntry::collection &die_collection); @@ -284,11 +279,12 @@ m_offset; // Offset within the .debug_info of the start of this entry uint32_t m_parent_idx; // How many to subtract from "this" to get the parent. // If zero this die has no parent - uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling. - m_empty_children : 1; // If a DIE says it had children, yet it just - // contained a NULL tag, this will be set. + uint32_t m_sibling_idx; // How many to add to "this" to get the sibling. uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE, m_has_children : 1, // Set to 1 if this DIE has children + // It is zero if a DIE says it had + // children, yet it just contained + // a NULL tag. m_tag : 16; // A copy of the DW_TAG value so we don't // have to go through the compile unit // abbrev table Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp @@ -40,7 +40,6 @@ m_offset = *offset_ptr; m_parent_idx = 0; m_sibling_idx = 0; - m_empty_children = false; const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr); assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE)); m_abbr_idx = abbr_idx; @@ -1836,7 +1835,6 @@ bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const { return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx && m_sibling_idx == rhs.m_sibling_idx && - m_empty_children == rhs.m_empty_children && m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children && m_tag == rhs.m_tag; }
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp @@ -223,7 +223,7 @@ // the list (saves up to 25% in C++ code), we need a way to let the // DIE know that it actually doesn't have children. if (!m_die_array.empty()) - m_die_array.back().SetEmptyChildren(true); + m_die_array.back().SetHasChildren(false); } } else { die.SetParentIndex(m_die_array.size() - die_index_stack[depth - 1]); @@ -263,7 +263,7 @@ if (!m_die_array.empty()) { if (m_first_die) { // Only needed for the assertion. - m_first_die.SetEmptyChildren(m_die_array.front().GetEmptyChildren()); + m_first_die.SetHasChildren(m_die_array.front().HasChildren()); lldbassert(m_first_die == m_die_array.front()); } m_first_die = m_die_array.front(); Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h +++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h @@ -57,8 +57,7 @@ DWARFDebugInfoEntry() : m_offset(DW_INVALID_OFFSET), m_parent_idx(0), m_sibling_idx(0), - m_empty_children(false), m_abbr_idx(0), m_has_children(false), - m_tag(0) {} + m_abbr_idx(0), m_has_children(false), m_tag(0) {} explicit operator bool() const { return m_offset != DW_INVALID_OFFSET; } bool operator==(const DWARFDebugInfoEntry &rhs) const; @@ -227,10 +226,10 @@ // we don't need to store our child pointer, if we have a child it will // be the next entry in the list... DWARFDebugInfoEntry *GetFirstChild() { - return (HasChildren() && !m_empty_children) ? this + 1 : NULL; + return HasChildren() ? this + 1 : NULL; } const DWARFDebugInfoEntry *GetFirstChild() const { - return (HasChildren() && !m_empty_children) ? this + 1 : NULL; + return HasChildren() ? this + 1 : NULL; } void GetDeclContextDIEs(DWARFUnit *cu, @@ -271,10 +270,6 @@ void SetParentIndex(uint32_t idx) { m_parent_idx = idx; } - bool GetEmptyChildren() const { return m_empty_children; } - - void SetEmptyChildren(bool b) { m_empty_children = b; } - static void DumpDIECollection(lldb_private::Stream &strm, DWARFDebugInfoEntry::collection &die_collection); @@ -284,11 +279,12 @@ m_offset; // Offset within the .debug_info of the start of this entry uint32_t m_parent_idx; // How many to subtract from "this" to get the parent. // If zero this die has no parent - uint32_t m_sibling_idx : 31, // How many to add to "this" to get the sibling. - m_empty_children : 1; // If a DIE says it had children, yet it just - // contained a NULL tag, this will be set. + uint32_t m_sibling_idx; // How many to add to "this" to get the sibling. uint32_t m_abbr_idx : DIE_ABBR_IDX_BITSIZE, m_has_children : 1, // Set to 1 if this DIE has children + // It is zero if a DIE says it had + // children, yet it just contained + // a NULL tag. m_tag : 16; // A copy of the DW_TAG value so we don't // have to go through the compile unit // abbrev table Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp +++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp @@ -40,7 +40,6 @@ m_offset = *offset_ptr; m_parent_idx = 0; m_sibling_idx = 0; - m_empty_children = false; const uint64_t abbr_idx = debug_info_data.GetULEB128(offset_ptr); assert(abbr_idx < (1 << DIE_ABBR_IDX_BITSIZE)); m_abbr_idx = abbr_idx; @@ -1836,7 +1835,6 @@ bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const { return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx && m_sibling_idx == rhs.m_sibling_idx && - m_empty_children == rhs.m_empty_children && m_abbr_idx == rhs.m_abbr_idx && m_has_children == rhs.m_has_children && m_tag == rhs.m_tag; }
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits