[Lldb-commits] [PATCH] D66654: 2/2: Process formatters in reverse-chronological order

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.

I think this looks great. Some operations may get less efficient with this, but OTOH, some will get actually faster. And it is definitely the most understandable solution out of everything we had so far.



================
Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:83
+    Delete(name);
+    m_map.push_back(std::make_pair(std::move(name), std::move(entry)));
     if (listener)
----------------
m_map.emplace_back, maybe?


================
Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:109
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
-    MapIterator iter = m_map.find(name);
-    if (iter == m_map.end())
-      return false;
-    entry = iter->second;
-    return true;
+    for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter)
+      if (iter->first == name) {
----------------
range-based for


================
Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:130-134
-    MapIterator iter = m_map.begin();
-    MapIterator end = m_map.end();
-    while (index > 0) {
-      iter++;
-      index--;
----------------
lol


================
Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:168
+  typedef KeyType MapKeyType;
+  typedef std::shared_ptr<ValueType> MapValueType;
   typedef typename BackEndType::ForEachCallback ForEachCallback;
----------------
Is this still needed, or is that a leftover from the MapVector attempt?


================
Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:291-292
+    // Patterns are matched in reverse-chronological order.
+    MapReverseIterator pos, end = m_format_map.map().rend();
+    for (pos = m_format_map.map().rbegin(); pos != end; pos++) {
       const RegularExpression &regex = pos->first;
----------------
consider: `for(const auto &entry: llvm::reverse(m_format_map))`


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66654/new/

https://reviews.llvm.org/D66654