Author: Raphael Isemann Date: 2020-07-23T18:34:59+02:00 New Revision: 77ae06b8c6c7425c0376dbd526390ba1f48b3db5
URL: https://github.com/llvm/llvm-project/commit/77ae06b8c6c7425c0376dbd526390ba1f48b3db5 DIFF: https://github.com/llvm/llvm-project/commit/77ae06b8c6c7425c0376dbd526390ba1f48b3db5.diff LOG: [lldb][NFC] Remove FormatMap Summary: FormattersContainer.h has two containers: FormatMap and FormattersContainer itself. FormatMap is essentially just a SetVector with a listener interface that is aspiring to be thread-safe as most of its functions lock its member mutex. FormattersContainer is for the most part just calling the matching functions of internal FormatMap instance and essentially acts as a wrapper class with some minor formatter search functionality on top. The only difference is that the FormattersContainer's public `Get` function is actually searching formatters in the list of formatters (and for example doing regex-matching) while FormatMap's `Get` function is just looking up a a format by the type matcher string. This patch deletes `FormatMap` by just renaming it to `FormattersContainer` and pulling in the two `Get` functions from the original `FormattersContainer` class. The only other user of `FormatMap` was the `NamedSummariesMap` in the `FormatManager` which I migrated by just making it also a `FormattersContainer` and replaced the only call to the `Get` function (which now has new semantics) with `GetExact` (which is FormattersContainer's function that has the semantics of FormatMap's `Get`). As `NamedSummariesMap` only stores non-regex-based formatters, both `Get` and `GetExact` would have worked, so this was mostly to clarify that this is supposed to be NFC. I also added the missing mutex lock in the `GetCount` function which was previously missing in the `FormatMap` implementation. Technically not "NFC" but I anyway had to change the function... Reviewers: labath, mib Reviewed By: labath Subscribers: abidh, JDevlieghere Differential Revision: https://reviews.llvm.org/D84296 Added: Modified: lldb/include/lldb/DataFormatters/FormatManager.h lldb/include/lldb/DataFormatters/FormattersContainer.h lldb/include/lldb/DataFormatters/TypeCategory.h lldb/source/DataFormatters/DataVisualization.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h index 98c5b132c203..978ad148d6c4 100644 --- a/lldb/include/lldb/DataFormatters/FormatManager.h +++ b/lldb/include/lldb/DataFormatters/FormatManager.h @@ -34,7 +34,7 @@ namespace lldb_private { // this file's objects directly class FormatManager : public IFormatChangeListener { - typedef FormatMap<TypeSummaryImpl> NamedSummariesMap; + typedef FormattersContainer<TypeSummaryImpl> NamedSummariesMap; typedef TypeCategoryMap::MapType::iterator CategoryMapIterator; public: diff --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h b/lldb/include/lldb/DataFormatters/FormattersContainer.h index d7c531e9618a..a30ad920ef45 100644 --- a/lldb/include/lldb/DataFormatters/FormattersContainer.h +++ b/lldb/include/lldb/DataFormatters/FormattersContainer.h @@ -104,18 +104,18 @@ class TypeMatcher { } }; -template <typename ValueType> class FormattersContainer; - -template <typename ValueType> class FormatMap { +template <typename ValueType> class FormattersContainer { public: - typedef typename ValueType::SharedPointer ValueSP; + typedef typename std::shared_ptr<ValueType> ValueSP; typedef std::vector<std::pair<TypeMatcher, ValueSP>> MapType; - typedef typename MapType::iterator MapIterator; typedef std::function<bool(const TypeMatcher &, const ValueSP &)> ForEachCallback; + typedef typename std::shared_ptr<FormattersContainer<ValueType>> + SharedPointer; + + friend class TypeCategoryImpl; - FormatMap(IFormatChangeListener *lst) - : m_map(), m_map_mutex(), listener(lst) {} + FormattersContainer(IFormatChangeListener *lst) : listener(lst) {} void Add(TypeMatcher matcher, const ValueSP &entry) { if (listener) @@ -130,9 +130,9 @@ template <typename ValueType> class FormatMap { listener->Changed(); } - bool Delete(const TypeMatcher &matcher) { + bool Delete(TypeMatcher matcher) { std::lock_guard<std::recursive_mutex> guard(m_map_mutex); - for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter) + for (auto iter = m_map.begin(); iter != m_map.end(); ++iter) if (iter->first.CreatedBySameMatchString(matcher)) { m_map.erase(iter); if (listener) @@ -142,14 +142,18 @@ template <typename ValueType> class FormatMap { return false; } - void Clear() { + bool Get(ConstString type, ValueSP &entry) { std::lock_guard<std::recursive_mutex> guard(m_map_mutex); - m_map.clear(); - if (listener) - listener->Changed(); + for (auto &formatter : llvm::reverse(m_map)) { + if (formatter.first.Matches(type)) { + entry = formatter.second; + return true; + } + } + return false; } - bool Get(const TypeMatcher &matcher, ValueSP &entry) { + bool GetExact(TypeMatcher matcher, ValueSP &entry) { std::lock_guard<std::recursive_mutex> guard(m_map_mutex); for (const auto &pos : m_map) if (pos.first.CreatedBySameMatchString(matcher)) { @@ -159,108 +163,50 @@ template <typename ValueType> class FormatMap { return false; } - void ForEach(ForEachCallback callback) { - if (callback) { - std::lock_guard<std::recursive_mutex> guard(m_map_mutex); - for (const auto &pos : m_map) { - const TypeMatcher &type = pos.first; - if (!callback(type, pos.second)) - break; - } - } - } - - uint32_t GetCount() { return m_map.size(); } - - ValueSP GetValueAtIndex(size_t index) { + ValueSP GetAtIndex(size_t index) { std::lock_guard<std::recursive_mutex> guard(m_map_mutex); if (index >= m_map.size()) return ValueSP(); return m_map[index].second; } - // If caller holds the mutex we could return a reference without copy ctor. - llvm::Optional<TypeMatcher> GetKeyAtIndex(size_t index) { + lldb::TypeNameSpecifierImplSP GetTypeNameSpecifierAtIndex(size_t index) { std::lock_guard<std::recursive_mutex> guard(m_map_mutex); if (index >= m_map.size()) - return llvm::None; - return m_map[index].first; + return lldb::TypeNameSpecifierImplSP(); + TypeMatcher type_matcher = m_map[index].first; + return std::make_shared<TypeNameSpecifierImpl>( + type_matcher.GetMatchString().GetStringRef(), true); } -protected: - MapType m_map; - std::recursive_mutex m_map_mutex; - IFormatChangeListener *listener; - - MapType &map() { return m_map; } - - std::recursive_mutex &mutex() { return m_map_mutex; } - - friend class FormattersContainer<ValueType>; - friend class FormatManager; -}; - -template <typename ValueType> class FormattersContainer { -protected: - typedef FormatMap<ValueType> BackEndType; - -public: - typedef std::shared_ptr<ValueType> MapValueType; - typedef typename BackEndType::ForEachCallback ForEachCallback; - typedef typename std::shared_ptr<FormattersContainer<ValueType>> - SharedPointer; - - friend class TypeCategoryImpl; - - FormattersContainer(IFormatChangeListener *lst) : m_format_map(lst) {} - - void Add(TypeMatcher type, const MapValueType &entry) { - m_format_map.Add(std::move(type), entry); + void Clear() { + std::lock_guard<std::recursive_mutex> guard(m_map_mutex); + m_map.clear(); + if (listener) + listener->Changed(); } - bool Delete(TypeMatcher type) { return m_format_map.Delete(type); } - - bool Get(ConstString type, MapValueType &entry) { - std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex()); - for (auto &formatter : llvm::reverse(m_format_map.map())) { - if (formatter.first.Matches(type)) { - entry = formatter.second; - return true; + void ForEach(ForEachCallback callback) { + if (callback) { + std::lock_guard<std::recursive_mutex> guard(m_map_mutex); + for (const auto &pos : m_map) { + const TypeMatcher &type = pos.first; + if (!callback(type, pos.second)) + break; } } - return false; } - bool GetExact(ConstString type, MapValueType &entry) { - return m_format_map.Get(type, entry); - } - - MapValueType GetAtIndex(size_t index) { - return m_format_map.GetValueAtIndex(index); - } - - lldb::TypeNameSpecifierImplSP GetTypeNameSpecifierAtIndex(size_t index) { - llvm::Optional<TypeMatcher> type_matcher = - m_format_map.GetKeyAtIndex(index); - if (!type_matcher) - return lldb::TypeNameSpecifierImplSP(); - return lldb::TypeNameSpecifierImplSP(new TypeNameSpecifierImpl( - type_matcher->GetMatchString().GetStringRef(), true)); + uint32_t GetCount() { + std::lock_guard<std::recursive_mutex> guard(m_map_mutex); + return m_map.size(); } - void Clear() { m_format_map.Clear(); } - - void ForEach(ForEachCallback callback) { m_format_map.ForEach(callback); } - - uint32_t GetCount() { return m_format_map.GetCount(); } - protected: - BackEndType m_format_map; - FormattersContainer(const FormattersContainer &) = delete; const FormattersContainer &operator=(const FormattersContainer &) = delete; - bool Get(const FormattersMatchVector &candidates, MapValueType &entry) { + bool Get(const FormattersMatchVector &candidates, ValueSP &entry) { for (const FormattersMatchCandidate &candidate : candidates) { if (Get(candidate.GetTypeName(), entry)) { if (candidate.IsMatch(entry) == false) { @@ -273,6 +219,10 @@ template <typename ValueType> class FormattersContainer { } return false; } + + MapType m_map; + std::recursive_mutex m_map_mutex; + IFormatChangeListener *listener; }; } // namespace lldb_private diff --git a/lldb/include/lldb/DataFormatters/TypeCategory.h b/lldb/include/lldb/DataFormatters/TypeCategory.h index 4c8a7e14be12..2662ffc5aeac 100644 --- a/lldb/include/lldb/DataFormatters/TypeCategory.h +++ b/lldb/include/lldb/DataFormatters/TypeCategory.h @@ -31,7 +31,7 @@ template <typename FormatterImpl> class FormatterContainerPair { typedef TypeMatcher ExactMatchMap; typedef TypeMatcher RegexMatchMap; - typedef typename ExactMatchContainer::MapValueType MapValueType; + typedef typename ExactMatchContainer::ValueSP MapValueType; typedef typename ExactMatchContainer::SharedPointer ExactMatchContainerSP; typedef typename RegexMatchContainer::SharedPointer RegexMatchContainerSP; diff --git a/lldb/source/DataFormatters/DataVisualization.cpp b/lldb/source/DataFormatters/DataVisualization.cpp index 82248bb64285..ded8bbd90391 100644 --- a/lldb/source/DataFormatters/DataVisualization.cpp +++ b/lldb/source/DataFormatters/DataVisualization.cpp @@ -169,7 +169,7 @@ DataVisualization::Categories::GetCategoryAtIndex(size_t index) { bool DataVisualization::NamedSummaryFormats::GetSummaryFormat( ConstString type, lldb::TypeSummaryImplSP &entry) { - return GetFormatManager().GetNamedSummaryContainer().Get(type, entry); + return GetFormatManager().GetNamedSummaryContainer().GetExact(type, entry); } void DataVisualization::NamedSummaryFormats::Add( _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
