That's actually a better idea. Relanded it with the member removed and constructor deleted. Thanks!
- Raphael > On 23 Jul 2020, at 09:49, Eric Christopher <echri...@gmail.com> wrote: > > Hi Raphael, > > I've temporarily reverted this again with hopefully a better explanation here: > > echristo@athyra ~/s/llvm-project> git push > To github.com:llvm/llvm-project.git > 55c0f12a869..3a75466f41b master -> master > > One review thought: If you don't want people using the default constructor > for TypeMatcher perhaps just delete it? (i.e. = delete). If that's not where > you were going then perhaps we can come up with another way around this :) > > Thanks and sorry for the inconvenience! > > -eric > > On Wed, Jul 22, 2020 at 12:34 AM Raphael Isemann via lldb-commits > <lldb-commits@lists.llvm.org <mailto:lldb-commits@lists.llvm.org>> wrote: > > Author: Raphael Isemann > Date: 2020-07-22T09:32:28+02:00 > New Revision: 074b121642b286afb16adeebda5ec8236f7b8ea9 > > URL: > https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9 > > <https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9> > DIFF: > 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 > <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 ®ex = 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 ®ex = 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 ®ex = 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 ®ex, > + 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 <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)); > + } > +} > > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org <mailto:lldb-commits@lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > <https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits