Author: Michael Buch Date: 2024-07-03T19:54:20+02:00 New Revision: a0176533766201eca58b20a11e42ab30c73d1b1b
URL: https://github.com/llvm/llvm-project/commit/a0176533766201eca58b20a11e42ab30c73d1b1b DIFF: https://github.com/llvm/llvm-project/commit/a0176533766201eca58b20a11e42ab30c73d1b1b.diff LOG: [lldb][DataFormatter][NFC] Factor out MapIterator logic into separate helper (#97544) This patch factors all the logic for advancing the `MapIterator` out of `GetChildAtIndex`. This, in my opinion, helps readability, and will be useful for upcoming cleanups in this area. While here, some drive-by changes: * added a couple of clarification comments * fixed a variable name typo * turned the `return lldb::ValueObjectSP()` into `return nullptr` * added an assertion to make sure we keep the iterator cache in a valid state Added: Modified: lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp Removed: ################################################################################ diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp index 44fe294ced722..6c2bc1a34137a 100644 --- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp +++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp @@ -17,6 +17,7 @@ #include "lldb/Utility/Endian.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/Stream.h" +#include "lldb/lldb-forward.h" using namespace lldb; using namespace lldb_private; @@ -184,6 +185,22 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd { void GetValueOffset(const lldb::ValueObjectSP &node); + /// Returns the ValueObject for the __tree_node type that + /// holds the key/value pair of the node at index \ref idx. + /// + /// \param[in] idx The child index that we're looking to get + /// the key/value pair for. + /// + /// \param[in] max_depth The maximum search depth after which + /// we stop trying to find the key/value + /// pair for. + /// + /// \returns On success, returns the ValueObjectSP corresponding + /// to the __tree_node's __value_ member (which holds + /// the key/value pair the formatter wants to display). + /// On failure, will return nullptr. + ValueObjectSP GetKeyValuePair(size_t idx, size_t max_depth); + ValueObject *m_tree = nullptr; ValueObject *m_root_node = nullptr; CompilerType m_element_type; @@ -267,7 +284,7 @@ void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset( uint64_t bit_offset; if (node_type.GetIndexOfFieldWithName("__value_", nullptr, &bit_offset) != UINT32_MAX) { - // Old layout (pre 089a7cc5dea) + // Old layout (pre d05b10ab4fc65) m_skip_size = bit_offset / 8u; } else { auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>(); @@ -299,75 +316,88 @@ void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset( } } -lldb::ValueObjectSP -lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex( - uint32_t idx) { - static ConstString g_cc_("__cc_"), g_cc("__cc"); - static ConstString g_nc("__nc"); - uint32_t num_children = CalculateNumChildrenIgnoringErrors(); - if (idx >= num_children) - return lldb::ValueObjectSP(); - if (m_tree == nullptr || m_root_node == nullptr) - return lldb::ValueObjectSP(); - - MapIterator iterator(m_root_node, num_children); +ValueObjectSP +lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair( + size_t idx, size_t max_depth) { + MapIterator iterator(m_root_node, max_depth); const bool need_to_skip = (idx > 0); - size_t actual_advancde = idx; + size_t actual_advance = idx; if (need_to_skip) { + // If we have already created the iterator for the previous + // index, we can start from there and advance by 1. auto cached_iterator = m_iterators.find(idx - 1); if (cached_iterator != m_iterators.end()) { iterator = cached_iterator->second; - actual_advancde = 1; + actual_advance = 1; } } - ValueObjectSP iterated_sp(iterator.advance(actual_advancde)); - if (!iterated_sp) { + ValueObjectSP iterated_sp(iterator.advance(actual_advance)); + if (!iterated_sp) // this tree is garbage - stop - m_tree = - nullptr; // this will stop all future searches until an Update() happens - return iterated_sp; - } + return nullptr; - if (!GetDataType()) { - m_tree = nullptr; - return lldb::ValueObjectSP(); - } + if (!GetDataType()) + return nullptr; if (!need_to_skip) { Status error; iterated_sp = iterated_sp->Dereference(error); - if (!iterated_sp || error.Fail()) { - m_tree = nullptr; - return lldb::ValueObjectSP(); - } + if (!iterated_sp || error.Fail()) + return nullptr; + GetValueOffset(iterated_sp); auto child_sp = iterated_sp->GetChildMemberWithName("__value_"); - if (child_sp) + if (child_sp) { + // Old layout (pre 089a7cc5dea) iterated_sp = child_sp; - else + } else { iterated_sp = iterated_sp->GetSyntheticChildAtOffset( m_skip_size, m_element_type, true); - if (!iterated_sp) { - m_tree = nullptr; - return lldb::ValueObjectSP(); } + + if (!iterated_sp) + return nullptr; } else { // because of the way our debug info is made, we need to read item 0 // first so that we can cache information used to generate other elements if (m_skip_size == UINT32_MAX) GetChildAtIndex(0); - if (m_skip_size == UINT32_MAX) { - m_tree = nullptr; - return lldb::ValueObjectSP(); - } + + if (m_skip_size == UINT32_MAX) + return nullptr; + iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size, m_element_type, true); - if (!iterated_sp) { - m_tree = nullptr; - return lldb::ValueObjectSP(); - } + if (!iterated_sp) + return nullptr; + } + + m_iterators[idx] = iterator; + assert(iterated_sp != nullptr && + "Cached MapIterator for invalid ValueObject"); + + return iterated_sp; +} + +lldb::ValueObjectSP +lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex( + uint32_t idx) { + static ConstString g_cc_("__cc_"), g_cc("__cc"); + static ConstString g_nc("__nc"); + uint32_t num_children = CalculateNumChildrenIgnoringErrors(); + if (idx >= num_children) + return nullptr; + + if (m_tree == nullptr || m_root_node == nullptr) + return nullptr; + + ValueObjectSP key_val_sp = GetKeyValuePair(idx, /*max_depth=*/num_children); + if (!key_val_sp) { + // this will stop all future searches until an Update() happens + m_tree = nullptr; + return nullptr; } // at this point we have a valid @@ -375,7 +405,7 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex( // all items named __value_ StreamString name; name.Printf("[%" PRIu64 "]", (uint64_t)idx); - auto potential_child_sp = iterated_sp->Clone(ConstString(name.GetString())); + auto potential_child_sp = key_val_sp->Clone(ConstString(name.GetString())); if (potential_child_sp) { switch (potential_child_sp->GetNumChildrenIgnoringErrors()) { case 1: { @@ -396,7 +426,6 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetChildAtIndex( } } } - m_iterators[idx] = iterator; return potential_child_sp; } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
