[Lldb-commits] [lldb] 074b121 - Reland [lldb] Unify type name matching in FormattersContainer
Raphael “Teemperor” Isemann via lldb-commits
lldb-commits at lists.llvm.org
Thu Jul 23 09:29:52 PDT 2020
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 <echristo at gmail.com> wrote:
>
> Hi Raphael,
>
> I've temporarily reverted this again with hopefully a better explanation here:
>
> echristo at 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 at lists.llvm.org <mailto:lldb-commits at 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 at lists.llvm.org <mailto:lldb-commits at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits <https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200723/6df6c793/attachment-0001.html>
More information about the lldb-commits
mailing list