[Lldb-commits] [lldb] 77ae06b - [lldb][NFC] Remove FormatMap
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 23 09:35:20 PDT 2020
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(
More information about the lldb-commits
mailing list