[lldb][NFC] Remove FormatMap

[teemperor]

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...

[labath]

I do recall this being fairly messy, but I never took the time to figure out the reasons for it. Seems like a nice cleanup though.

lldb/include/lldb/DataFormatters/FormattersContainer.h

178

make_shared ?

lldb/include/lldb/DataFormatters/FormatManager.h

 
 // this file (and its. cpp) contain the low-level implementation of LLDB Data
 // Visualization class DataVisualization is the high-level front-end of this
 // feature clients should refer to that class as the entry-point into the data
 // formatters unless they have a good reason to bypass it and prefer to use
 // this file's objects directly
 
 class FormatManager : public IFormatChangeListener {
-  typedef FormatMap<TypeSummaryImpl> NamedSummariesMap;
+  typedef FormattersContainer<TypeSummaryImpl> NamedSummariesMap;
   typedef TypeCategoryMap::MapType::iterator CategoryMapIterator;
 
 public:
   typedef std::map<lldb::LanguageType, LanguageCategory::UniquePointer>
       LanguageCategories;
 
   FormatManager();
 

lldb/include/lldb/DataFormatters/FormattersContainer.h

public:
   /// that originally created them:
   /// (lldb) type summary add --summary-string \"A\" -x TypeName
   /// (lldb) type summary delete TypeName
   bool CreatedBySameMatchString(TypeMatcher other) const {
     return GetMatchString() == other.GetMatchString();
   }
 };
 
-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)
       entry->GetRevision() = listener->GetCurrentRevision();
     else
       entry->GetRevision() = 0;
 
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     Delete(matcher);
     m_map.emplace_back(std::move(matcher), std::move(entry));
     if (listener)
       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)
           listener->Changed();
         return true;
       }
     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)) {
         entry = pos.second;
         return true;
       }
     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;
-      }
-    }
-    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;
       }
-
-  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) {
           entry.reset();
           continue;
         } else {
           return true;
         }
       }
     }
     return false;
   }
+
+  MapType m_map;
+  std::recursive_mutex m_map_mutex;
+  IFormatChangeListener *listener;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_DATAFORMATTERS_FORMATTERSCONTAINER_H

lldb/include/lldb/DataFormatters/TypeCategory.h

 template <typename FormatterImpl> class FormatterContainerPair {
 public:
   typedef FormattersContainer<FormatterImpl> ExactMatchContainer;
   typedef FormattersContainer<FormatterImpl> RegexMatchContainer;
 
   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;
 
   typedef
       typename ExactMatchContainer::ForEachCallback ExactMatchForEachCallback;
   typedef
       typename RegexMatchContainer::ForEachCallback RegexMatchForEachCallback;

lldb/source/DataFormatters/DataVisualization.cpp

 
 lldb::TypeCategoryImplSP
 DataVisualization::Categories::GetCategoryAtIndex(size_t index) {
   return GetFormatManager().GetCategoryAtIndex(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(
     ConstString type, const lldb::TypeSummaryImplSP &entry) {
   GetFormatManager().GetNamedSummaryContainer().Add(type, entry);
 }
 
 bool DataVisualization::NamedSummaryFormats::Delete(ConstString type) {