[Lldb-commits] [lldb] 074b121 - Reland [lldb] Unify type name matching in FormattersContainer

Raphael Isemann via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 22 00:34:33 PDT 2020


Author: Raphael Isemann
Date: 2020-07-22T09:32:28+02:00
New Revision: 074b121642b286afb16adeebda5ec8236f7b8ea9

URL: https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9
DIFF: https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9.diff

LOG: Reland [lldb] Unify type name matching in FormattersContainer

This was originally reverted because the Linux bots were red after this landed,
but it seems that was actually caused by a different commit. I double checked
that this works on Linux, so let's reland this on Linux.

Summary:

FormattersContainer stores LLDB's formatters. It's implemented as a templated
map-like data structures that supports any kind of value type and only allows
ConstString and RegularExpression as the key types. The keys are used for
matching type names (e.g., the ConstString key `std::vector` matches the type
with the same name while RegularExpression keys match any type where the
RegularExpression instance matches).

The fact that a single FormattersContainer can only match either by string
comparison or regex matching (depending on the KeyType) causes us to always have
two FormatterContainer instances in all the formatting code. This also leads to
us having every type name matching logic in LLDB twice. For example,
TypeCategory has to implement every method twice (one string matching one, one
regex matching one).

This patch changes FormattersContainer to instead have a single `TypeMatcher`
key that wraps the logic for string-based and regex-based type matching and is
now the only possible KeyType for the FormattersContainer. This means that a
single FormattersContainer can now match types with both regex and string
comparison.

To summarize the changes in this patch:
* Remove all the `*_Impl` methods from `FormattersContainer`
* Instead call the FormatMap functions from `FormattersContainer` with a
  `TypeMatcher` type that does the respective matching.
* Replace `ConstString` with `TypeMatcher` in the few places that directly
  interact with `FormattersContainer`.

I'm working on some follow up patches that I split up because they deserve their
own review:

* Unify FormatMap and FormattersContainer (they are nearly identical now).
* Delete the duplicated half of all the type matching code that can now use one
  interface.
* Propagate TypeMatcher through all the formatter code interfaces instead of
  always offering two functions for everything.

There is one ugly design part that I couldn't get rid of yet and that is that we
have to support getting back the string used to construct a `TypeMatcher` later
on. The reason for this is that LLDB only supports referencing existing type
matchers by just typing their respective input string again (without even
supplying if it's a regex or not).

Reviewers: davide, mib

Reviewed By: mib

Subscribers: mgorny, JDevlieghere

Differential Revision: https://reviews.llvm.org/D84151

Added: 
    lldb/unittests/DataFormatter/FormattersContainerTest.cpp

Modified: 
    lldb/include/lldb/DataFormatters/DataVisualization.h
    lldb/include/lldb/DataFormatters/FormatManager.h
    lldb/include/lldb/DataFormatters/FormattersContainer.h
    lldb/include/lldb/DataFormatters/TypeCategory.h
    lldb/include/lldb/DataFormatters/TypeCategoryMap.h
    lldb/source/Commands/CommandObjectType.cpp
    lldb/source/DataFormatters/DataVisualization.cpp
    lldb/source/DataFormatters/FormatManager.cpp
    lldb/unittests/DataFormatter/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/DataFormatters/DataVisualization.h b/lldb/include/lldb/DataFormatters/DataVisualization.h
index b053aa074d9e..7be07d65acdd 100644
--- a/lldb/include/lldb/DataFormatters/DataVisualization.h
+++ b/lldb/include/lldb/DataFormatters/DataVisualization.h
@@ -69,9 +69,9 @@ class DataVisualization {
 
     static void Clear();
 
-    static void
-    ForEach(std::function<bool(ConstString, const lldb::TypeSummaryImplSP &)>
-                callback);
+    static void ForEach(std::function<bool(const TypeMatcher &,
+                                           const lldb::TypeSummaryImplSP &)>
+                            callback);
 
     static uint32_t GetCount();
   };

diff  --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h
index 56a0303f9b02..98c5b132c203 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<ConstString, TypeSummaryImpl> NamedSummariesMap;
+  typedef FormatMap<TypeSummaryImpl> NamedSummariesMap;
   typedef TypeCategoryMap::MapType::iterator CategoryMapIterator;
 
 public:
@@ -144,13 +144,6 @@ class FormatManager : public IFormatChangeListener {
 
   static const char *GetFormatAsCString(lldb::Format format);
 
-  // if the user tries to add formatters for, say, "struct Foo" those will not
-  // match any type because of the way we strip qualifiers from typenames this
-  // method looks for the case where the user is adding a
-  // "class","struct","enum" or "union" Foo and strips the unnecessary
-  // qualifier
-  static ConstString GetValidTypeName(ConstString type);
-
   // when DataExtractor dumps a vectorOfT, it uses a predefined format for each
   // item this method returns it, or eFormatInvalid if vector_format is not a
   // vectorOf

diff  --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h b/lldb/include/lldb/DataFormatters/FormattersContainer.h
index a22cf494bf8a..69dd1ecf1752 100644
--- a/lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ b/lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -37,57 +37,113 @@ class IFormatChangeListener {
   virtual uint32_t GetCurrentRevision() = 0;
 };
 
-// if the user tries to add formatters for, say, "struct Foo" those will not
-// match any type because of the way we strip qualifiers from typenames this
-// method looks for the case where the user is adding a "class","struct","enum"
-// or "union" Foo and strips the unnecessary qualifier
-static inline ConstString GetValidTypeName_Impl(ConstString type) {
-  if (type.IsEmpty())
-    return type;
+/// Class for matching type names.
+class TypeMatcher {
+  RegularExpression m_type_name_regex;
+  ConstString m_type_name;
+  /// False if m_type_name_regex should be used for matching. False if this is
+  /// just matching by comparing with m_type_name string.
+  bool m_is_regex;
+  /// True iff this TypeMatcher is invalid and shouldn't be used for any
+  /// type matching logic.
+  bool m_valid = true;
+
+  // if the user tries to add formatters for, say, "struct Foo" those will not
+  // match any type because of the way we strip qualifiers from typenames this
+  // method looks for the case where the user is adding a
+  // "class","struct","enum" or "union" Foo and strips the unnecessary qualifier
+  static ConstString StripTypeName(ConstString type) {
+    if (type.IsEmpty())
+      return type;
+
+    std::string type_cstr(type.AsCString());
+    StringLexer type_lexer(type_cstr);
+
+    type_lexer.AdvanceIf("class ");
+    type_lexer.AdvanceIf("enum ");
+    type_lexer.AdvanceIf("struct ");
+    type_lexer.AdvanceIf("union ");
+
+    while (type_lexer.NextIf({' ', '\t', '\v', '\f'}).first)
+      ;
+
+    return ConstString(type_lexer.GetUnlexed());
+  }
 
-  std::string type_cstr(type.AsCString());
-  StringLexer type_lexer(type_cstr);
+public:
+  /// Creates an invalid matcher that should not be used for any type matching.
+  TypeMatcher() : m_valid(false) {}
+  /// Creates a matcher that accepts any type with exactly the given type name.
+  TypeMatcher(ConstString type_name)
+      : m_type_name(type_name), m_is_regex(false) {}
+  /// Creates a matcher that accepts any type matching the given regex.
+  TypeMatcher(RegularExpression regex)
+      : m_type_name_regex(regex), m_is_regex(true) {}
+
+  /// True iff this matches the given type name.
+  bool Matches(ConstString type_name) const {
+    assert(m_valid && "Using invalid TypeMatcher");
+
+    if (m_is_regex)
+      return m_type_name_regex.Execute(type_name.GetStringRef());
+    return m_type_name == type_name ||
+           StripTypeName(m_type_name) == StripTypeName(type_name);
+  }
 
-  type_lexer.AdvanceIf("class ");
-  type_lexer.AdvanceIf("enum ");
-  type_lexer.AdvanceIf("struct ");
-  type_lexer.AdvanceIf("union ");
+  /// Returns the underlying match string for this TypeMatcher.
+  ConstString GetMatchString() const {
+    assert(m_valid && "Using invalid TypeMatcher");
 
-  while (type_lexer.NextIf({' ', '\t', '\v', '\f'}).first)
-    ;
+    if (m_is_regex)
+      return ConstString(m_type_name_regex.GetText());
+    return StripTypeName(m_type_name);
+  }
 
-  return ConstString(type_lexer.GetUnlexed());
-}
+  /// Returns true if this TypeMatcher and the given one were most created by
+  /// the same match string.
+  /// The main purpose of this function is to find existing TypeMatcher
+  /// instances by the user input that created them. This is necessary as LLDB
+  /// allows referencing existing TypeMatchers in commands by the user input
+  /// that originally created them:
+  /// (lldb) type summary add --summary-string \"A\" -x TypeName
+  /// (lldb) type summary delete TypeName
+  bool CreatedBySameMatchString(TypeMatcher other) const {
+    assert(m_valid && "Using invalid TypeMatcher");
+
+    return GetMatchString() == other.GetMatchString();
+  }
+};
 
-template <typename KeyType, typename ValueType> class FormattersContainer;
+template <typename ValueType> class FormattersContainer;
 
-template <typename KeyType, typename ValueType> class FormatMap {
+template <typename ValueType> class FormatMap {
 public:
   typedef typename ValueType::SharedPointer ValueSP;
-  typedef std::vector<std::pair<KeyType, ValueSP>> MapType;
+  typedef std::vector<std::pair<TypeMatcher, ValueSP>> MapType;
   typedef typename MapType::iterator MapIterator;
-  typedef std::function<bool(const KeyType &, const ValueSP &)> ForEachCallback;
+  typedef std::function<bool(const TypeMatcher &, const ValueSP &)>
+      ForEachCallback;
 
   FormatMap(IFormatChangeListener *lst)
       : m_map(), m_map_mutex(), listener(lst) {}
 
-  void Add(KeyType name, const ValueSP &entry) {
+  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(name);
-    m_map.emplace_back(std::move(name), std::move(entry));
+    Delete(matcher);
+    m_map.emplace_back(std::move(matcher), std::move(entry));
     if (listener)
       listener->Changed();
   }
 
-  bool Delete(const KeyType &name) {
+  bool Delete(const TypeMatcher &matcher) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter)
-      if (iter->first == name) {
+      if (iter->first.CreatedBySameMatchString(matcher)) {
         m_map.erase(iter);
         if (listener)
           listener->Changed();
@@ -103,10 +159,10 @@ template <typename KeyType, typename ValueType> class FormatMap {
       listener->Changed();
   }
 
-  bool Get(const KeyType &name, ValueSP &entry) {
+  bool Get(const TypeMatcher &matcher, ValueSP &entry) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     for (const auto &pos : m_map)
-      if (pos.first == name) {
+      if (pos.first.CreatedBySameMatchString(matcher)) {
         entry = pos.second;
         return true;
       }
@@ -117,7 +173,7 @@ template <typename KeyType, typename ValueType> class FormatMap {
     if (callback) {
       std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
       for (const auto &pos : m_map) {
-        const KeyType &type = pos.first;
+        const TypeMatcher &type = pos.first;
         if (!callback(type, pos.second))
           break;
       }
@@ -134,10 +190,10 @@ template <typename KeyType, typename ValueType> class FormatMap {
   }
 
   // If caller holds the mutex we could return a reference without copy ctor.
-  KeyType GetKeyAtIndex(size_t index) {
+  llvm::Optional<TypeMatcher> GetKeyAtIndex(size_t index) {
     std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
     if (index >= m_map.size())
-      return {};
+      return llvm::None;
     return m_map[index].first;
   }
 
@@ -150,41 +206,43 @@ template <typename KeyType, typename ValueType> class FormatMap {
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
 
-  friend class FormattersContainer<KeyType, ValueType>;
+  friend class FormattersContainer<ValueType>;
   friend class FormatManager;
 };
 
-template <typename KeyType, typename ValueType> class FormattersContainer {
+template <typename ValueType> class FormattersContainer {
 protected:
-  typedef FormatMap<KeyType, ValueType> BackEndType;
+  typedef FormatMap<ValueType> BackEndType;
 
 public:
-  typedef typename BackEndType::MapType MapType;
-  typedef typename MapType::iterator MapIterator;
-  typedef KeyType MapKeyType;
   typedef std::shared_ptr<ValueType> MapValueType;
   typedef typename BackEndType::ForEachCallback ForEachCallback;
-  typedef typename std::shared_ptr<FormattersContainer<KeyType, ValueType>>
+  typedef typename std::shared_ptr<FormattersContainer<ValueType>>
       SharedPointer;
 
   friend class TypeCategoryImpl;
 
   FormattersContainer(IFormatChangeListener *lst) : m_format_map(lst) {}
 
-  void Add(MapKeyType type, const MapValueType &entry) {
-    Add_Impl(std::move(type), entry, static_cast<KeyType *>(nullptr));
+  void Add(TypeMatcher type, const MapValueType &entry) {
+    m_format_map.Add(std::move(type), entry);
   }
 
-  bool Delete(ConstString type) {
-    return Delete_Impl(type, static_cast<KeyType *>(nullptr));
-  }
+  bool Delete(TypeMatcher type) { return m_format_map.Delete(type); }
 
   bool Get(ConstString type, MapValueType &entry) {
-    return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
+    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;
   }
 
   bool GetExact(ConstString type, MapValueType &entry) {
-    return GetExact_Impl(type, entry, static_cast<KeyType *>(nullptr));
+    return m_format_map.Get(type, entry);
   }
 
   MapValueType GetAtIndex(size_t index) {
@@ -192,8 +250,12 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
   }
 
   lldb::TypeNameSpecifierImplSP GetTypeNameSpecifierAtIndex(size_t index) {
-    return GetTypeNameSpecifierAtIndex_Impl(index,
-                                            static_cast<KeyType *>(nullptr));
+    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));
   }
 
   void Clear() { m_format_map.Clear(); }
@@ -208,91 +270,6 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
   FormattersContainer(const FormattersContainer &) = delete;
   const FormattersContainer &operator=(const FormattersContainer &) = delete;
 
-  void Add_Impl(MapKeyType type, const MapValueType &entry,
-                RegularExpression *dummy) {
-    m_format_map.Add(std::move(type), entry);
-  }
-
-  void Add_Impl(ConstString type, const MapValueType &entry,
-                ConstString *dummy) {
-    m_format_map.Add(GetValidTypeName_Impl(type), entry);
-  }
-
-  bool Delete_Impl(ConstString type, ConstString *dummy) {
-    return m_format_map.Delete(type);
-  }
-
-  bool Delete_Impl(ConstString type, RegularExpression *dummy) {
-    std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
-    MapIterator pos, end = m_format_map.map().end();
-    for (pos = m_format_map.map().begin(); pos != end; pos++) {
-      const RegularExpression &regex = pos->first;
-      if (type.GetStringRef() == regex.GetText()) {
-        m_format_map.map().erase(pos);
-        if (m_format_map.listener)
-          m_format_map.listener->Changed();
-        return true;
-      }
-    }
-    return false;
-  }
-
-  bool Get_Impl(ConstString type, MapValueType &entry, ConstString *dummy) {
-    return m_format_map.Get(type, entry);
-  }
-
-  bool GetExact_Impl(ConstString type, MapValueType &entry,
-                     ConstString *dummy) {
-    return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
-  }
-
-  lldb::TypeNameSpecifierImplSP
-  GetTypeNameSpecifierAtIndex_Impl(size_t index, ConstString *dummy) {
-    ConstString key = m_format_map.GetKeyAtIndex(index);
-    if (key)
-      return lldb::TypeNameSpecifierImplSP(
-          new TypeNameSpecifierImpl(key.GetStringRef(), false));
-    else
-      return lldb::TypeNameSpecifierImplSP();
-  }
-
-  lldb::TypeNameSpecifierImplSP
-  GetTypeNameSpecifierAtIndex_Impl(size_t index, RegularExpression *dummy) {
-    RegularExpression regex = m_format_map.GetKeyAtIndex(index);
-    if (regex == RegularExpression())
-      return lldb::TypeNameSpecifierImplSP();
-    return lldb::TypeNameSpecifierImplSP(
-        new TypeNameSpecifierImpl(regex.GetText().str().c_str(), true));
-  }
-
-  bool Get_Impl(ConstString key, MapValueType &value,
-                RegularExpression *dummy) {
-    llvm::StringRef key_str = key.GetStringRef();
-    std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
-    // Patterns are matched in reverse-chronological order.
-    for (const auto &pos : llvm::reverse(m_format_map.map())) {
-      const RegularExpression &regex = pos.first;
-      if (regex.Execute(key_str)) {
-        value = pos.second;
-        return true;
-      }
-    }
-    return false;
-  }
-
-  bool GetExact_Impl(ConstString key, MapValueType &value,
-                     RegularExpression *dummy) {
-    std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
-    for (const auto &pos : m_format_map.map()) {
-      const RegularExpression &regex = pos.first;
-      if (regex.GetText() == key.GetStringRef()) {
-        value = pos.second;
-        return true;
-      }
-    }
-    return false;
-  }
-
   bool Get(const FormattersMatchVector &candidates, MapValueType &entry) {
     for (const FormattersMatchCandidate &candidate : candidates) {
       if (Get(candidate.GetTypeName(), entry)) {

diff  --git a/lldb/include/lldb/DataFormatters/TypeCategory.h b/lldb/include/lldb/DataFormatters/TypeCategory.h
index 11614fc67cd2..4c8a7e14be12 100644
--- a/lldb/include/lldb/DataFormatters/TypeCategory.h
+++ b/lldb/include/lldb/DataFormatters/TypeCategory.h
@@ -25,12 +25,11 @@ namespace lldb_private {
 
 template <typename FormatterImpl> class FormatterContainerPair {
 public:
-  typedef FormattersContainer<ConstString, FormatterImpl> ExactMatchContainer;
-  typedef FormattersContainer<RegularExpression, FormatterImpl>
-      RegexMatchContainer;
+  typedef FormattersContainer<FormatterImpl> ExactMatchContainer;
+  typedef FormattersContainer<FormatterImpl> RegexMatchContainer;
 
-  typedef typename ExactMatchContainer::MapType ExactMatchMap;
-  typedef typename RegexMatchContainer::MapType RegexMatchMap;
+  typedef TypeMatcher ExactMatchMap;
+  typedef TypeMatcher RegexMatchMap;
 
   typedef typename ExactMatchContainer::MapValueType MapValueType;
 
@@ -348,19 +347,13 @@ class TypeCategoryImpl {
   friend class LanguageCategory;
   friend class TypeCategoryMap;
 
-  friend class FormattersContainer<ConstString, TypeFormatImpl>;
-  friend class FormattersContainer<lldb::RegularExpressionSP, TypeFormatImpl>;
+  friend class FormattersContainer<TypeFormatImpl>;
 
-  friend class FormattersContainer<ConstString, TypeSummaryImpl>;
-  friend class FormattersContainer<lldb::RegularExpressionSP, TypeSummaryImpl>;
+  friend class FormattersContainer<TypeSummaryImpl>;
 
-  friend class FormattersContainer<ConstString, TypeFilterImpl>;
-  friend class FormattersContainer<lldb::RegularExpressionSP, TypeFilterImpl>;
-
-  friend class FormattersContainer<ConstString, ScriptedSyntheticChildren>;
-  friend class FormattersContainer<lldb::RegularExpressionSP,
-                                   ScriptedSyntheticChildren>;
+  friend class FormattersContainer<TypeFilterImpl>;
 
+  friend class FormattersContainer<ScriptedSyntheticChildren>;
 };
 
 } // namespace lldb_private

diff  --git a/lldb/include/lldb/DataFormatters/TypeCategoryMap.h b/lldb/include/lldb/DataFormatters/TypeCategoryMap.h
index 832652f7d745..6cd773786386 100644
--- a/lldb/include/lldb/DataFormatters/TypeCategoryMap.h
+++ b/lldb/include/lldb/DataFormatters/TypeCategoryMap.h
@@ -103,7 +103,7 @@ class TypeCategoryMap {
 
   std::recursive_mutex &mutex() { return m_map_mutex; }
 
-  friend class FormattersContainer<KeyType, ValueType>;
+  friend class FormattersContainer<ValueType>;
   friend class FormatManager;
 };
 } // namespace lldb_private

diff  --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index b2020f26621f..b23f91de0ce6 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -1066,13 +1066,15 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
       TypeCategoryImpl::ForEachCallbacks<FormatterType> foreach;
       foreach
         .SetExact([&result, &formatter_regex, &any_printed](
-                      ConstString name,
+                      const TypeMatcher &type_matcher,
                       const FormatterSharedPointer &format_sp) -> bool {
           if (formatter_regex) {
             bool escape = true;
-            if (name.GetStringRef() == formatter_regex->GetText()) {
+            if (type_matcher.CreatedBySameMatchString(
+                    ConstString(formatter_regex->GetText()))) {
               escape = false;
-            } else if (formatter_regex->Execute(name.GetStringRef())) {
+            } else if (formatter_regex->Execute(
+                           type_matcher.GetMatchString().GetStringRef())) {
               escape = false;
             }
 
@@ -1081,20 +1083,23 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
           }
 
           any_printed = true;
-          result.GetOutputStream().Printf("%s: %s\n", name.AsCString(),
-                                          format_sp->GetDescription().c_str());
+          result.GetOutputStream().Printf(
+              "%s: %s\n", type_matcher.GetMatchString().GetCString(),
+              format_sp->GetDescription().c_str());
           return true;
         });
 
       foreach
         .SetWithRegex([&result, &formatter_regex, &any_printed](
-                          const RegularExpression &regex,
+                          const TypeMatcher &type_matcher,
                           const FormatterSharedPointer &format_sp) -> bool {
           if (formatter_regex) {
             bool escape = true;
-            if (regex.GetText() == formatter_regex->GetText()) {
+            if (type_matcher.CreatedBySameMatchString(
+                    ConstString(formatter_regex->GetText()))) {
               escape = false;
-            } else if (formatter_regex->Execute(regex.GetText())) {
+            } else if (formatter_regex->Execute(
+                           type_matcher.GetMatchString().GetStringRef())) {
               escape = false;
             }
 
@@ -1103,9 +1108,9 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
           }
 
           any_printed = true;
-          result.GetOutputStream().Printf("%s: %s\n",
-                                          regex.GetText().str().c_str(),
-                                          format_sp->GetDescription().c_str());
+          result.GetOutputStream().Printf(
+              "%s: %s\n", type_matcher.GetMatchString().GetCString(),
+              format_sp->GetDescription().c_str());
           return true;
         });
 
@@ -1681,10 +1686,10 @@ class CommandObjectTypeSummaryList
     if (DataVisualization::NamedSummaryFormats::GetCount() > 0) {
       result.GetOutputStream().Printf("Named summaries:\n");
       DataVisualization::NamedSummaryFormats::ForEach(
-          [&result](ConstString name,
+          [&result](const TypeMatcher &type_matcher,
                     const TypeSummaryImplSP &summary_sp) -> bool {
             result.GetOutputStream().Printf(
-                "%s: %s\n", name.AsCString(),
+                "%s: %s\n", type_matcher.GetMatchString().GetCString(),
                 summary_sp->GetDescription().c_str());
             return true;
           });

diff  --git a/lldb/source/DataFormatters/DataVisualization.cpp b/lldb/source/DataFormatters/DataVisualization.cpp
index 450a5cbc3ef3..82248bb64285 100644
--- a/lldb/source/DataFormatters/DataVisualization.cpp
+++ b/lldb/source/DataFormatters/DataVisualization.cpp
@@ -174,8 +174,7 @@ bool DataVisualization::NamedSummaryFormats::GetSummaryFormat(
 
 void DataVisualization::NamedSummaryFormats::Add(
     ConstString type, const lldb::TypeSummaryImplSP &entry) {
-  GetFormatManager().GetNamedSummaryContainer().Add(
-      FormatManager::GetValidTypeName(type), entry);
+  GetFormatManager().GetNamedSummaryContainer().Add(type, entry);
 }
 
 bool DataVisualization::NamedSummaryFormats::Delete(ConstString type) {
@@ -187,7 +186,7 @@ void DataVisualization::NamedSummaryFormats::Clear() {
 }
 
 void DataVisualization::NamedSummaryFormats::ForEach(
-    std::function<bool(ConstString, const lldb::TypeSummaryImplSP &)>
+    std::function<bool(const TypeMatcher &, const lldb::TypeSummaryImplSP &)>
         callback) {
   GetFormatManager().GetNamedSummaryContainer().ForEach(callback);
 }

diff  --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index ad02d37360b8..af6df3ae6b47 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -551,10 +551,6 @@ bool FormatManager::ShouldPrintAsOneLiner(ValueObject &valobj) {
   return true;
 }
 
-ConstString FormatManager::GetValidTypeName(ConstString type) {
-  return ::GetValidTypeName_Impl(type);
-}
-
 ConstString FormatManager::GetTypeForCache(ValueObject &valobj,
                                            lldb::DynamicValueType use_dynamic) {
   ValueObjectSP valobj_sp = valobj.GetQualifiedRepresentationIfAvailable(

diff  --git a/lldb/unittests/DataFormatter/CMakeLists.txt b/lldb/unittests/DataFormatter/CMakeLists.txt
index 45011c56b0b0..9d967a72bfd1 100644
--- a/lldb/unittests/DataFormatter/CMakeLists.txt
+++ b/lldb/unittests/DataFormatter/CMakeLists.txt
@@ -1,5 +1,6 @@
 add_lldb_unittest(LLDBFormatterTests
   FormatManagerTests.cpp
+  FormattersContainerTest.cpp
   StringPrinterTests.cpp
 
   LINK_LIBS

diff  --git a/lldb/unittests/DataFormatter/FormattersContainerTest.cpp b/lldb/unittests/DataFormatter/FormattersContainerTest.cpp
new file mode 100644
index 000000000000..a28212391eae
--- /dev/null
+++ b/lldb/unittests/DataFormatter/FormattersContainerTest.cpp
@@ -0,0 +1,159 @@
+//===-- FormattersContainerTests.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 "lldb/DataFormatters/FormattersContainer.h"
+
+#include "gtest/gtest.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+// All the prefixes that the exact name matching will strip from the type.
+static const std::vector<std::string> exact_name_prefixes = {
+    "", // no prefix.
+    "class ", "struct ", "union ", "enum ",
+};
+
+// TypeMatcher that uses a exact type name string that needs to be matched.
+TEST(TypeMatcherTests, ExactName) {
+  for (const std::string &prefix : exact_name_prefixes) {
+    SCOPED_TRACE("Prefix: " + prefix);
+
+    TypeMatcher matcher(ConstString(prefix + "Name"));
+    EXPECT_TRUE(matcher.Matches(ConstString("class Name")));
+    EXPECT_TRUE(matcher.Matches(ConstString("struct Name")));
+    EXPECT_TRUE(matcher.Matches(ConstString("union Name")));
+    EXPECT_TRUE(matcher.Matches(ConstString("enum Name")));
+    EXPECT_TRUE(matcher.Matches(ConstString("Name")));
+
+    EXPECT_FALSE(matcher.Matches(ConstString("Name ")));
+    EXPECT_FALSE(matcher.Matches(ConstString("ame")));
+    EXPECT_FALSE(matcher.Matches(ConstString("Nam")));
+    EXPECT_FALSE(matcher.Matches(ConstString("am")));
+    EXPECT_FALSE(matcher.Matches(ConstString("a")));
+    EXPECT_FALSE(matcher.Matches(ConstString(" ")));
+    EXPECT_FALSE(matcher.Matches(ConstString("class N")));
+    EXPECT_FALSE(matcher.Matches(ConstString("class ")));
+    EXPECT_FALSE(matcher.Matches(ConstString("class")));
+  }
+}
+
+// TypeMatcher that uses a regex to match a type name.
+TEST(TypeMatcherTests, RegexName) {
+  TypeMatcher matcher(RegularExpression("^a[a-z]c$"));
+  EXPECT_TRUE(matcher.Matches(ConstString("abc")));
+  EXPECT_TRUE(matcher.Matches(ConstString("azc")));
+
+  // FIXME: This isn't consistent with the 'exact' type name matches above.
+  EXPECT_FALSE(matcher.Matches(ConstString("class abc")));
+
+  EXPECT_FALSE(matcher.Matches(ConstString("abbc")));
+  EXPECT_FALSE(matcher.Matches(ConstString(" abc")));
+  EXPECT_FALSE(matcher.Matches(ConstString("abc ")));
+  EXPECT_FALSE(matcher.Matches(ConstString(" abc ")));
+  EXPECT_FALSE(matcher.Matches(ConstString("XabcX")));
+  EXPECT_FALSE(matcher.Matches(ConstString("ac")));
+  EXPECT_FALSE(matcher.Matches(ConstString("a[a-z]c")));
+  EXPECT_FALSE(matcher.Matches(ConstString("aAc")));
+  EXPECT_FALSE(matcher.Matches(ConstString("ABC")));
+  EXPECT_FALSE(matcher.Matches(ConstString("")));
+}
+
+// TypeMatcher that only searches the type name.
+TEST(TypeMatcherTests, RegexMatchPart) {
+  TypeMatcher matcher(RegularExpression("a[a-z]c"));
+  EXPECT_TRUE(matcher.Matches(ConstString("class abc")));
+  EXPECT_TRUE(matcher.Matches(ConstString("abc")));
+  EXPECT_TRUE(matcher.Matches(ConstString(" abc ")));
+  EXPECT_TRUE(matcher.Matches(ConstString("azc")));
+  EXPECT_TRUE(matcher.Matches(ConstString("abc ")));
+  EXPECT_TRUE(matcher.Matches(ConstString(" abc ")));
+  EXPECT_TRUE(matcher.Matches(ConstString(" abc")));
+  EXPECT_TRUE(matcher.Matches(ConstString("XabcX")));
+
+  EXPECT_FALSE(matcher.Matches(ConstString("abbc")));
+  EXPECT_FALSE(matcher.Matches(ConstString("ac")));
+  EXPECT_FALSE(matcher.Matches(ConstString("a[a-z]c")));
+  EXPECT_FALSE(matcher.Matches(ConstString("aAc")));
+  EXPECT_FALSE(matcher.Matches(ConstString("ABC")));
+  EXPECT_FALSE(matcher.Matches(ConstString("")));
+}
+
+// GetMatchString for exact type name matchers.
+TEST(TypeMatcherTests, GetMatchStringExactName) {
+  EXPECT_EQ(TypeMatcher(ConstString("aa")).GetMatchString(), "aa");
+  EXPECT_EQ(TypeMatcher(ConstString("")).GetMatchString(), "");
+  EXPECT_EQ(TypeMatcher(ConstString("[a]")).GetMatchString(), "[a]");
+}
+
+// GetMatchString for regex matchers.
+TEST(TypeMatcherTests, GetMatchStringRegex) {
+  EXPECT_EQ(TypeMatcher(RegularExpression("aa")).GetMatchString(), "aa");
+  EXPECT_EQ(TypeMatcher(RegularExpression("")).GetMatchString(), "");
+  EXPECT_EQ(TypeMatcher(RegularExpression("[a]")).GetMatchString(), "[a]");
+}
+
+// GetMatchString for regex matchers.
+TEST(TypeMatcherTests, CreatedBySameMatchString) {
+  TypeMatcher empty_str(ConstString(""));
+  TypeMatcher empty_regex(RegularExpression(""));
+  EXPECT_TRUE(empty_str.CreatedBySameMatchString(empty_str));
+  EXPECT_TRUE(empty_str.CreatedBySameMatchString(empty_regex));
+
+  TypeMatcher a_str(ConstString("a"));
+  TypeMatcher a_regex(RegularExpression("a"));
+  EXPECT_TRUE(a_str.CreatedBySameMatchString(a_str));
+  EXPECT_TRUE(a_str.CreatedBySameMatchString(a_regex));
+
+  TypeMatcher digit_str(ConstString("[0-9]"));
+  TypeMatcher digit_regex(RegularExpression("[0-9]"));
+  EXPECT_TRUE(digit_str.CreatedBySameMatchString(digit_str));
+  EXPECT_TRUE(digit_str.CreatedBySameMatchString(digit_regex));
+
+  EXPECT_FALSE(empty_str.CreatedBySameMatchString(a_str));
+  EXPECT_FALSE(empty_str.CreatedBySameMatchString(a_regex));
+  EXPECT_FALSE(empty_str.CreatedBySameMatchString(digit_str));
+  EXPECT_FALSE(empty_str.CreatedBySameMatchString(digit_regex));
+
+  EXPECT_FALSE(empty_regex.CreatedBySameMatchString(a_str));
+  EXPECT_FALSE(empty_regex.CreatedBySameMatchString(a_regex));
+  EXPECT_FALSE(empty_regex.CreatedBySameMatchString(digit_str));
+  EXPECT_FALSE(empty_regex.CreatedBySameMatchString(digit_regex));
+
+  EXPECT_FALSE(a_str.CreatedBySameMatchString(empty_str));
+  EXPECT_FALSE(a_str.CreatedBySameMatchString(empty_regex));
+  EXPECT_FALSE(a_str.CreatedBySameMatchString(digit_str));
+  EXPECT_FALSE(a_str.CreatedBySameMatchString(digit_regex));
+
+  EXPECT_FALSE(a_regex.CreatedBySameMatchString(empty_str));
+  EXPECT_FALSE(a_regex.CreatedBySameMatchString(empty_regex));
+  EXPECT_FALSE(a_regex.CreatedBySameMatchString(digit_str));
+  EXPECT_FALSE(a_regex.CreatedBySameMatchString(digit_regex));
+
+  EXPECT_FALSE(digit_str.CreatedBySameMatchString(empty_str));
+  EXPECT_FALSE(digit_str.CreatedBySameMatchString(empty_regex));
+  EXPECT_FALSE(digit_str.CreatedBySameMatchString(a_str));
+  EXPECT_FALSE(digit_str.CreatedBySameMatchString(a_regex));
+
+  EXPECT_FALSE(digit_regex.CreatedBySameMatchString(empty_str));
+  EXPECT_FALSE(digit_regex.CreatedBySameMatchString(empty_regex));
+  EXPECT_FALSE(digit_regex.CreatedBySameMatchString(a_str));
+  EXPECT_FALSE(digit_regex.CreatedBySameMatchString(a_regex));
+}
+
+// Test CreatedBySameMatchString with stripped exact name prefixes.
+TEST(TypeMatcherTests, CreatedBySameMatchStringExactNamePrefixes) {
+  for (const std::string &prefix : exact_name_prefixes) {
+    SCOPED_TRACE("Prefix: " + prefix);
+    TypeMatcher with_prefix(ConstString(prefix + "Name"));
+    TypeMatcher without_prefix(RegularExpression(""));
+
+    EXPECT_TRUE(with_prefix.CreatedBySameMatchString(with_prefix));
+    EXPECT_TRUE(without_prefix.CreatedBySameMatchString(without_prefix));
+  }
+}


        


More information about the lldb-commits mailing list