JDevlieghere updated this revision to Diff 314755. JDevlieghere marked an inline comment as done. JDevlieghere added a comment.
Use `iterator_facade_base` as suggested by @dblaikie. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94136/new/ https://reviews.llvm.org/D94136 Files: lldb/include/lldb/Core/ModuleList.h lldb/source/Core/ModuleList.cpp lldb/source/Target/Target.cpp lldb/unittests/Core/CMakeLists.txt lldb/unittests/Core/ModuleListTest.cpp
Index: lldb/unittests/Core/ModuleListTest.cpp =================================================================== --- /dev/null +++ lldb/unittests/Core/ModuleListTest.cpp @@ -0,0 +1,77 @@ +//===-- ModuleListTest.cpp ------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "TestingSupport/SubsystemRAII.h" +#include "TestingSupport/TestUtilities.h" + +#include "lldb/Core/Module.h" +#include "lldb/Core/ModuleList.h" +#include "lldb/Utility/DataBuffer.h" + +#include "Plugins/ObjectFile/ELF/ObjectFileELF.h" +#include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h" +#include "Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h" + +#include "gtest/gtest.h" + +using namespace lldb; +using namespace lldb_private; + +namespace { +class ModuleListTest : public testing::Test { +public: + static void SetUpTestCase() { FileSystem::Initialize(); } + static void TearDownTestCase() { FileSystem::Terminate(); } +}; +} // namespace + +TEST_F(ModuleListTest, Iterators) { + auto AS = ArchSpec(); + ModuleSP module_foo = std::make_shared<Module>(FileSpec("foo"), AS); + ModuleSP module_bar = std::make_shared<Module>(FileSpec("bar"), AS); + ModuleSP module_baz = std::make_shared<Module>(FileSpec("baz"), AS); + + ModuleList module_list; + module_list.Append(module_foo); + module_list.Append(module_bar); + module_list.Append(module_baz); + + ASSERT_EQ(module_list.GetModuleAtIndex(0), module_foo); + ASSERT_EQ(module_list.GetModuleAtIndex(1), module_bar); + ASSERT_EQ(module_list.GetModuleAtIndex(2), module_baz); + + auto begin = module_list.begin(); + auto end = module_list.end(); + auto it = begin; + + ASSERT_NE(it, end); + ASSERT_EQ(*it, module_foo); + + ++it; + ASSERT_NE(it, end); + ASSERT_EQ(*it, module_bar); + + ++it; + ASSERT_NE(it, end); + ASSERT_EQ(*it, module_baz); + + ++it; + ASSERT_EQ(it, end); + + --it; + ASSERT_NE(it, end); + ASSERT_EQ(*it, module_baz); + + --it; + ASSERT_NE(it, end); + ASSERT_EQ(*it, module_bar); + + --it; + ASSERT_EQ(*it, module_foo); + ASSERT_EQ(it, begin); +} Index: lldb/unittests/Core/CMakeLists.txt =================================================================== --- lldb/unittests/Core/CMakeLists.txt +++ lldb/unittests/Core/CMakeLists.txt @@ -2,6 +2,7 @@ CommunicationTest.cpp MangledTest.cpp ModuleSpecTest.cpp + ModuleListTest.cpp RichManglingContextTest.cpp SourceManagerTest.cpp StreamCallbackTest.cpp Index: lldb/source/Target/Target.cpp =================================================================== --- lldb/source/Target/Target.cpp +++ lldb/source/Target/Target.cpp @@ -1656,17 +1656,15 @@ bool Target::ModuleIsExcludedForUnconstrainedSearches( const FileSpec &module_file_spec) { if (GetBreakpointsConsultPlatformAvoidList()) { - ModuleList matchingModules; + ModuleList matching_modules; ModuleSpec module_spec(module_file_spec); - GetImages().FindModules(module_spec, matchingModules); - size_t num_modules = matchingModules.GetSize(); + GetImages().FindModules(module_spec, matching_modules); // If there is more than one module for this file spec, only // return true if ALL the modules are on the black list. - if (num_modules > 0) { - for (size_t i = 0; i < num_modules; i++) { - if (!ModuleIsExcludedForUnconstrainedSearches( - matchingModules.GetModuleAtIndex(i))) + if (matching_modules.GetSize() > 0) { + for (ModuleSP m : matching_modules) { + if (!ModuleIsExcludedForUnconstrainedSearches(m)) return false; } return true; @@ -2471,9 +2469,7 @@ } const ModuleList &modules = GetImages(); - const size_t num_images = modules.GetSize(); - for (size_t idx = 0; idx < num_images; ++idx) { - ModuleSP module_sp(modules.GetModuleAtIndex(idx)); + for (ModuleSP module_sp : modules) { if (!module_sp || !module_sp->GetObjectFile()) continue; @@ -2846,11 +2842,8 @@ size_t Target::UnloadModuleSections(const ModuleList &module_list) { size_t section_unload_count = 0; - size_t num_modules = module_list.GetSize(); - for (size_t i = 0; i < num_modules; ++i) { - section_unload_count += - UnloadModuleSections(module_list.GetModuleAtIndex(i)); - } + for (ModuleSP module_sp : module_list) + section_unload_count += UnloadModuleSections(module_sp); return section_unload_count; } Index: lldb/source/Core/ModuleList.cpp =================================================================== --- lldb/source/Core/ModuleList.cpp +++ lldb/source/Core/ModuleList.cpp @@ -132,6 +132,16 @@ return m_symlink_paths; } +ModuleIterator::ModuleIterator(const ModuleList &module_list, size_t idx) + : m_module_list(module_list), m_index(idx) {} + +ModuleIterator::ModuleIterator(const ModuleList &module_list) + : m_module_list(module_list), m_index(module_list.GetSize()) {} + +const lldb::ModuleSP ModuleIterator::operator*() const { + return m_module_list.GetModuleAtIndex(m_index); +} + ModuleList::ModuleList() : m_modules(), m_modules_mutex(), m_notifier(nullptr) {} Index: lldb/include/lldb/Core/ModuleList.h =================================================================== --- lldb/include/lldb/Core/ModuleList.h +++ lldb/include/lldb/Core/ModuleList.h @@ -18,11 +18,12 @@ #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" - #include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/iterator.h" #include "llvm/Support/RWMutex.h" #include <functional> +#include <limits> #include <list> #include <mutex> #include <vector> @@ -63,6 +64,41 @@ PathMappingList GetSymlinkMappings() const; }; +class ModuleList; + +class ModuleIterator + : public llvm::iterator_facade_base< + ModuleIterator, std::bidirectional_iterator_tag, lldb::ModuleSP> { +public: + explicit ModuleIterator(const ModuleList &module_list, size_t idx); + ModuleIterator(const ModuleList &module_list); + + const lldb::ModuleSP operator*() const; + + ModuleIterator &operator++() { + ++m_index; + return *this; + } + + ModuleIterator &operator--() { + --m_index; + return *this; + } + + friend bool operator==(const ModuleIterator &lhs, const ModuleIterator &rhs) { + return &lhs.m_module_list == &rhs.m_module_list && + lhs.m_index == rhs.m_index; + } + + friend bool operator!=(const ModuleIterator &lhs, const ModuleIterator &rhs) { + return !(lhs == rhs); + } + +private: + const ModuleList &m_module_list; + size_t m_index; +}; + /// \class ModuleList ModuleList.h "lldb/Core/ModuleList.h" /// A collection class for Module objects. /// @@ -467,6 +503,12 @@ void ForEach(std::function<bool(const lldb::ModuleSP &module_sp)> const &callback) const; + typedef ModuleIterator iterator; + typedef ModuleIterator const_iterator; + + ModuleIterator begin() const { return ModuleIterator(*this, 0); } + ModuleIterator end() const { return ModuleIterator(*this); } + protected: // Class typedefs. typedef std::vector<lldb::ModuleSP>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
