<div dir="ltr">Hi Raphael,<div><br></div><div>I've temporarily reverted this again with hopefully a better explanation here:</div><div><br></div><div>echristo@athyra ~/s/llvm-project> git push<br>To github.com:llvm/llvm-project.git<br> 55c0f12a869..3a75466f41b master -> master<br></div><div><br></div><div>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 :)</div><div><br></div><div>Thanks and sorry for the inconvenience!</div><div><br></div><div>-eric</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 22, 2020 at 12:34 AM Raphael Isemann via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Raphael Isemann<br>
Date: 2020-07-22T09:32:28+02:00<br>
New Revision: 074b121642b286afb16adeebda5ec8236f7b8ea9<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/074b121642b286afb16adeebda5ec8236f7b8ea9.diff</a><br>
<br>
LOG: Reland [lldb] Unify type name matching in FormattersContainer<br>
<br>
This was originally reverted because the Linux bots were red after this landed,<br>
but it seems that was actually caused by a different commit. I double checked<br>
that this works on Linux, so let's reland this on Linux.<br>
<br>
Summary:<br>
<br>
FormattersContainer stores LLDB's formatters. It's implemented as a templated<br>
map-like data structures that supports any kind of value type and only allows<br>
ConstString and RegularExpression as the key types. The keys are used for<br>
matching type names (e.g., the ConstString key `std::vector` matches the type<br>
with the same name while RegularExpression keys match any type where the<br>
RegularExpression instance matches).<br>
<br>
The fact that a single FormattersContainer can only match either by string<br>
comparison or regex matching (depending on the KeyType) causes us to always have<br>
two FormatterContainer instances in all the formatting code. This also leads to<br>
us having every type name matching logic in LLDB twice. For example,<br>
TypeCategory has to implement every method twice (one string matching one, one<br>
regex matching one).<br>
<br>
This patch changes FormattersContainer to instead have a single `TypeMatcher`<br>
key that wraps the logic for string-based and regex-based type matching and is<br>
now the only possible KeyType for the FormattersContainer. This means that a<br>
single FormattersContainer can now match types with both regex and string<br>
comparison.<br>
<br>
To summarize the changes in this patch:<br>
* Remove all the `*_Impl` methods from `FormattersContainer`<br>
* Instead call the FormatMap functions from `FormattersContainer` with a<br>
`TypeMatcher` type that does the respective matching.<br>
* Replace `ConstString` with `TypeMatcher` in the few places that directly<br>
interact with `FormattersContainer`.<br>
<br>
I'm working on some follow up patches that I split up because they deserve their<br>
own review:<br>
<br>
* Unify FormatMap and FormattersContainer (they are nearly identical now).<br>
* Delete the duplicated half of all the type matching code that can now use one<br>
interface.<br>
* Propagate TypeMatcher through all the formatter code interfaces instead of<br>
always offering two functions for everything.<br>
<br>
There is one ugly design part that I couldn't get rid of yet and that is that we<br>
have to support getting back the string used to construct a `TypeMatcher` later<br>
on. The reason for this is that LLDB only supports referencing existing type<br>
matchers by just typing their respective input string again (without even<br>
supplying if it's a regex or not).<br>
<br>
Reviewers: davide, mib<br>
<br>
Reviewed By: mib<br>
<br>
Subscribers: mgorny, JDevlieghere<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D84151" rel="noreferrer" target="_blank">https://reviews.llvm.org/D84151</a><br>
<br>
Added: <br>
lldb/unittests/DataFormatter/FormattersContainerTest.cpp<br>
<br>
Modified: <br>
lldb/include/lldb/DataFormatters/DataVisualization.h<br>
lldb/include/lldb/DataFormatters/FormatManager.h<br>
lldb/include/lldb/DataFormatters/FormattersContainer.h<br>
lldb/include/lldb/DataFormatters/TypeCategory.h<br>
lldb/include/lldb/DataFormatters/TypeCategoryMap.h<br>
lldb/source/Commands/CommandObjectType.cpp<br>
lldb/source/DataFormatters/DataVisualization.cpp<br>
lldb/source/DataFormatters/FormatManager.cpp<br>
lldb/unittests/DataFormatter/CMakeLists.txt<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/lldb/include/lldb/DataFormatters/DataVisualization.h b/lldb/include/lldb/DataFormatters/DataVisualization.h<br>
index b053aa074d9e..7be07d65acdd 100644<br>
--- a/lldb/include/lldb/DataFormatters/DataVisualization.h<br>
+++ b/lldb/include/lldb/DataFormatters/DataVisualization.h<br>
@@ -69,9 +69,9 @@ class DataVisualization {<br>
<br>
static void Clear();<br>
<br>
- static void<br>
- ForEach(std::function<bool(ConstString, const lldb::TypeSummaryImplSP &)><br>
- callback);<br>
+ static void ForEach(std::function<bool(const TypeMatcher &,<br>
+ const lldb::TypeSummaryImplSP &)><br>
+ callback);<br>
<br>
static uint32_t GetCount();<br>
};<br>
<br>
diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h<br>
index 56a0303f9b02..98c5b132c203 100644<br>
--- a/lldb/include/lldb/DataFormatters/FormatManager.h<br>
+++ b/lldb/include/lldb/DataFormatters/FormatManager.h<br>
@@ -34,7 +34,7 @@ namespace lldb_private {<br>
// this file's objects directly<br>
<br>
class FormatManager : public IFormatChangeListener {<br>
- typedef FormatMap<ConstString, TypeSummaryImpl> NamedSummariesMap;<br>
+ typedef FormatMap<TypeSummaryImpl> NamedSummariesMap;<br>
typedef TypeCategoryMap::MapType::iterator CategoryMapIterator;<br>
<br>
public:<br>
@@ -144,13 +144,6 @@ class FormatManager : public IFormatChangeListener {<br>
<br>
static const char *GetFormatAsCString(lldb::Format format);<br>
<br>
- // if the user tries to add formatters for, say, "struct Foo" those will not<br>
- // match any type because of the way we strip qualifiers from typenames this<br>
- // method looks for the case where the user is adding a<br>
- // "class","struct","enum" or "union" Foo and strips the unnecessary<br>
- // qualifier<br>
- static ConstString GetValidTypeName(ConstString type);<br>
-<br>
// when DataExtractor dumps a vectorOfT, it uses a predefined format for each<br>
// item this method returns it, or eFormatInvalid if vector_format is not a<br>
// vectorOf<br>
<br>
diff --git a/lldb/include/lldb/DataFormatters/FormattersContainer.h b/lldb/include/lldb/DataFormatters/FormattersContainer.h<br>
index a22cf494bf8a..69dd1ecf1752 100644<br>
--- a/lldb/include/lldb/DataFormatters/FormattersContainer.h<br>
+++ b/lldb/include/lldb/DataFormatters/FormattersContainer.h<br>
@@ -37,57 +37,113 @@ class IFormatChangeListener {<br>
virtual uint32_t GetCurrentRevision() = 0;<br>
};<br>
<br>
-// if the user tries to add formatters for, say, "struct Foo" those will not<br>
-// match any type because of the way we strip qualifiers from typenames this<br>
-// method looks for the case where the user is adding a "class","struct","enum"<br>
-// or "union" Foo and strips the unnecessary qualifier<br>
-static inline ConstString GetValidTypeName_Impl(ConstString type) {<br>
- if (type.IsEmpty())<br>
- return type;<br>
+/// Class for matching type names.<br>
+class TypeMatcher {<br>
+ RegularExpression m_type_name_regex;<br>
+ ConstString m_type_name;<br>
+ /// False if m_type_name_regex should be used for matching. False if this is<br>
+ /// just matching by comparing with m_type_name string.<br>
+ bool m_is_regex;<br>
+ /// True iff this TypeMatcher is invalid and shouldn't be used for any<br>
+ /// type matching logic.<br>
+ bool m_valid = true;<br>
+<br>
+ // if the user tries to add formatters for, say, "struct Foo" those will not<br>
+ // match any type because of the way we strip qualifiers from typenames this<br>
+ // method looks for the case where the user is adding a<br>
+ // "class","struct","enum" or "union" Foo and strips the unnecessary qualifier<br>
+ static ConstString StripTypeName(ConstString type) {<br>
+ if (type.IsEmpty())<br>
+ return type;<br>
+<br>
+ std::string type_cstr(type.AsCString());<br>
+ StringLexer type_lexer(type_cstr);<br>
+<br>
+ type_lexer.AdvanceIf("class ");<br>
+ type_lexer.AdvanceIf("enum ");<br>
+ type_lexer.AdvanceIf("struct ");<br>
+ type_lexer.AdvanceIf("union ");<br>
+<br>
+ while (type_lexer.NextIf({' ', '\t', '\v', '\f'}).first)<br>
+ ;<br>
+<br>
+ return ConstString(type_lexer.GetUnlexed());<br>
+ }<br>
<br>
- std::string type_cstr(type.AsCString());<br>
- StringLexer type_lexer(type_cstr);<br>
+public:<br>
+ /// Creates an invalid matcher that should not be used for any type matching.<br>
+ TypeMatcher() : m_valid(false) {}<br>
+ /// Creates a matcher that accepts any type with exactly the given type name.<br>
+ TypeMatcher(ConstString type_name)<br>
+ : m_type_name(type_name), m_is_regex(false) {}<br>
+ /// Creates a matcher that accepts any type matching the given regex.<br>
+ TypeMatcher(RegularExpression regex)<br>
+ : m_type_name_regex(regex), m_is_regex(true) {}<br>
+<br>
+ /// True iff this matches the given type name.<br>
+ bool Matches(ConstString type_name) const {<br>
+ assert(m_valid && "Using invalid TypeMatcher");<br>
+<br>
+ if (m_is_regex)<br>
+ return m_type_name_regex.Execute(type_name.GetStringRef());<br>
+ return m_type_name == type_name ||<br>
+ StripTypeName(m_type_name) == StripTypeName(type_name);<br>
+ }<br>
<br>
- type_lexer.AdvanceIf("class ");<br>
- type_lexer.AdvanceIf("enum ");<br>
- type_lexer.AdvanceIf("struct ");<br>
- type_lexer.AdvanceIf("union ");<br>
+ /// Returns the underlying match string for this TypeMatcher.<br>
+ ConstString GetMatchString() const {<br>
+ assert(m_valid && "Using invalid TypeMatcher");<br>
<br>
- while (type_lexer.NextIf({' ', '\t', '\v', '\f'}).first)<br>
- ;<br>
+ if (m_is_regex)<br>
+ return ConstString(m_type_name_regex.GetText());<br>
+ return StripTypeName(m_type_name);<br>
+ }<br>
<br>
- return ConstString(type_lexer.GetUnlexed());<br>
-}<br>
+ /// Returns true if this TypeMatcher and the given one were most created by<br>
+ /// the same match string.<br>
+ /// The main purpose of this function is to find existing TypeMatcher<br>
+ /// instances by the user input that created them. This is necessary as LLDB<br>
+ /// allows referencing existing TypeMatchers in commands by the user input<br>
+ /// that originally created them:<br>
+ /// (lldb) type summary add --summary-string \"A\" -x TypeName<br>
+ /// (lldb) type summary delete TypeName<br>
+ bool CreatedBySameMatchString(TypeMatcher other) const {<br>
+ assert(m_valid && "Using invalid TypeMatcher");<br>
+<br>
+ return GetMatchString() == other.GetMatchString();<br>
+ }<br>
+};<br>
<br>
-template <typename KeyType, typename ValueType> class FormattersContainer;<br>
+template <typename ValueType> class FormattersContainer;<br>
<br>
-template <typename KeyType, typename ValueType> class FormatMap {<br>
+template <typename ValueType> class FormatMap {<br>
public:<br>
typedef typename ValueType::SharedPointer ValueSP;<br>
- typedef std::vector<std::pair<KeyType, ValueSP>> MapType;<br>
+ typedef std::vector<std::pair<TypeMatcher, ValueSP>> MapType;<br>
typedef typename MapType::iterator MapIterator;<br>
- typedef std::function<bool(const KeyType &, const ValueSP &)> ForEachCallback;<br>
+ typedef std::function<bool(const TypeMatcher &, const ValueSP &)><br>
+ ForEachCallback;<br>
<br>
FormatMap(IFormatChangeListener *lst)<br>
: m_map(), m_map_mutex(), listener(lst) {}<br>
<br>
- void Add(KeyType name, const ValueSP &entry) {<br>
+ void Add(TypeMatcher matcher, const ValueSP &entry) {<br>
if (listener)<br>
entry->GetRevision() = listener->GetCurrentRevision();<br>
else<br>
entry->GetRevision() = 0;<br>
<br>
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);<br>
- Delete(name);<br>
- m_map.emplace_back(std::move(name), std::move(entry));<br>
+ Delete(matcher);<br>
+ m_map.emplace_back(std::move(matcher), std::move(entry));<br>
if (listener)<br>
listener->Changed();<br>
}<br>
<br>
- bool Delete(const KeyType &name) {<br>
+ bool Delete(const TypeMatcher &matcher) {<br>
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);<br>
for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter)<br>
- if (iter->first == name) {<br>
+ if (iter->first.CreatedBySameMatchString(matcher)) {<br>
m_map.erase(iter);<br>
if (listener)<br>
listener->Changed();<br>
@@ -103,10 +159,10 @@ template <typename KeyType, typename ValueType> class FormatMap {<br>
listener->Changed();<br>
}<br>
<br>
- bool Get(const KeyType &name, ValueSP &entry) {<br>
+ bool Get(const TypeMatcher &matcher, ValueSP &entry) {<br>
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);<br>
for (const auto &pos : m_map)<br>
- if (pos.first == name) {<br>
+ if (pos.first.CreatedBySameMatchString(matcher)) {<br>
entry = pos.second;<br>
return true;<br>
}<br>
@@ -117,7 +173,7 @@ template <typename KeyType, typename ValueType> class FormatMap {<br>
if (callback) {<br>
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);<br>
for (const auto &pos : m_map) {<br>
- const KeyType &type = pos.first;<br>
+ const TypeMatcher &type = pos.first;<br>
if (!callback(type, pos.second))<br>
break;<br>
}<br>
@@ -134,10 +190,10 @@ template <typename KeyType, typename ValueType> class FormatMap {<br>
}<br>
<br>
// If caller holds the mutex we could return a reference without copy ctor.<br>
- KeyType GetKeyAtIndex(size_t index) {<br>
+ llvm::Optional<TypeMatcher> GetKeyAtIndex(size_t index) {<br>
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);<br>
if (index >= m_map.size())<br>
- return {};<br>
+ return llvm::None;<br>
return m_map[index].first;<br>
}<br>
<br>
@@ -150,41 +206,43 @@ template <typename KeyType, typename ValueType> class FormatMap {<br>
<br>
std::recursive_mutex &mutex() { return m_map_mutex; }<br>
<br>
- friend class FormattersContainer<KeyType, ValueType>;<br>
+ friend class FormattersContainer<ValueType>;<br>
friend class FormatManager;<br>
};<br>
<br>
-template <typename KeyType, typename ValueType> class FormattersContainer {<br>
+template <typename ValueType> class FormattersContainer {<br>
protected:<br>
- typedef FormatMap<KeyType, ValueType> BackEndType;<br>
+ typedef FormatMap<ValueType> BackEndType;<br>
<br>
public:<br>
- typedef typename BackEndType::MapType MapType;<br>
- typedef typename MapType::iterator MapIterator;<br>
- typedef KeyType MapKeyType;<br>
typedef std::shared_ptr<ValueType> MapValueType;<br>
typedef typename BackEndType::ForEachCallback ForEachCallback;<br>
- typedef typename std::shared_ptr<FormattersContainer<KeyType, ValueType>><br>
+ typedef typename std::shared_ptr<FormattersContainer<ValueType>><br>
SharedPointer;<br>
<br>
friend class TypeCategoryImpl;<br>
<br>
FormattersContainer(IFormatChangeListener *lst) : m_format_map(lst) {}<br>
<br>
- void Add(MapKeyType type, const MapValueType &entry) {<br>
- Add_Impl(std::move(type), entry, static_cast<KeyType *>(nullptr));<br>
+ void Add(TypeMatcher type, const MapValueType &entry) {<br>
+ m_format_map.Add(std::move(type), entry);<br>
}<br>
<br>
- bool Delete(ConstString type) {<br>
- return Delete_Impl(type, static_cast<KeyType *>(nullptr));<br>
- }<br>
+ bool Delete(TypeMatcher type) { return m_format_map.Delete(type); }<br>
<br>
bool Get(ConstString type, MapValueType &entry) {<br>
- return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));<br>
+ std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());<br>
+ for (auto &formatter : llvm::reverse(m_format_map.map())) {<br>
+ if (formatter.first.Matches(type)) {<br>
+ entry = formatter.second;<br>
+ return true;<br>
+ }<br>
+ }<br>
+ return false;<br>
}<br>
<br>
bool GetExact(ConstString type, MapValueType &entry) {<br>
- return GetExact_Impl(type, entry, static_cast<KeyType *>(nullptr));<br>
+ return m_format_map.Get(type, entry);<br>
}<br>
<br>
MapValueType GetAtIndex(size_t index) {<br>
@@ -192,8 +250,12 @@ template <typename KeyType, typename ValueType> class FormattersContainer {<br>
}<br>
<br>
lldb::TypeNameSpecifierImplSP GetTypeNameSpecifierAtIndex(size_t index) {<br>
- return GetTypeNameSpecifierAtIndex_Impl(index,<br>
- static_cast<KeyType *>(nullptr));<br>
+ llvm::Optional<TypeMatcher> type_matcher =<br>
+ m_format_map.GetKeyAtIndex(index);<br>
+ if (!type_matcher)<br>
+ return lldb::TypeNameSpecifierImplSP();<br>
+ return lldb::TypeNameSpecifierImplSP(new TypeNameSpecifierImpl(<br>
+ type_matcher->GetMatchString().GetStringRef(), true));<br>
}<br>
<br>
void Clear() { m_format_map.Clear(); }<br>
@@ -208,91 +270,6 @@ template <typename KeyType, typename ValueType> class FormattersContainer {<br>
FormattersContainer(const FormattersContainer &) = delete;<br>
const FormattersContainer &operator=(const FormattersContainer &) = delete;<br>
<br>
- void Add_Impl(MapKeyType type, const MapValueType &entry,<br>
- RegularExpression *dummy) {<br>
- m_format_map.Add(std::move(type), entry);<br>
- }<br>
-<br>
- void Add_Impl(ConstString type, const MapValueType &entry,<br>
- ConstString *dummy) {<br>
- m_format_map.Add(GetValidTypeName_Impl(type), entry);<br>
- }<br>
-<br>
- bool Delete_Impl(ConstString type, ConstString *dummy) {<br>
- return m_format_map.Delete(type);<br>
- }<br>
-<br>
- bool Delete_Impl(ConstString type, RegularExpression *dummy) {<br>
- std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());<br>
- MapIterator pos, end = m_format_map.map().end();<br>
- for (pos = m_format_map.map().begin(); pos != end; pos++) {<br>
- const RegularExpression ®ex = pos->first;<br>
- if (type.GetStringRef() == regex.GetText()) {<br>
- m_format_map.map().erase(pos);<br>
- if (m_format_map.listener)<br>
- m_format_map.listener->Changed();<br>
- return true;<br>
- }<br>
- }<br>
- return false;<br>
- }<br>
-<br>
- bool Get_Impl(ConstString type, MapValueType &entry, ConstString *dummy) {<br>
- return m_format_map.Get(type, entry);<br>
- }<br>
-<br>
- bool GetExact_Impl(ConstString type, MapValueType &entry,<br>
- ConstString *dummy) {<br>
- return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));<br>
- }<br>
-<br>
- lldb::TypeNameSpecifierImplSP<br>
- GetTypeNameSpecifierAtIndex_Impl(size_t index, ConstString *dummy) {<br>
- ConstString key = m_format_map.GetKeyAtIndex(index);<br>
- if (key)<br>
- return lldb::TypeNameSpecifierImplSP(<br>
- new TypeNameSpecifierImpl(key.GetStringRef(), false));<br>
- else<br>
- return lldb::TypeNameSpecifierImplSP();<br>
- }<br>
-<br>
- lldb::TypeNameSpecifierImplSP<br>
- GetTypeNameSpecifierAtIndex_Impl(size_t index, RegularExpression *dummy) {<br>
- RegularExpression regex = m_format_map.GetKeyAtIndex(index);<br>
- if (regex == RegularExpression())<br>
- return lldb::TypeNameSpecifierImplSP();<br>
- return lldb::TypeNameSpecifierImplSP(<br>
- new TypeNameSpecifierImpl(regex.GetText().str().c_str(), true));<br>
- }<br>
-<br>
- bool Get_Impl(ConstString key, MapValueType &value,<br>
- RegularExpression *dummy) {<br>
- llvm::StringRef key_str = key.GetStringRef();<br>
- std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());<br>
- // Patterns are matched in reverse-chronological order.<br>
- for (const auto &pos : llvm::reverse(m_format_map.map())) {<br>
- const RegularExpression ®ex = pos.first;<br>
- if (regex.Execute(key_str)) {<br>
- value = pos.second;<br>
- return true;<br>
- }<br>
- }<br>
- return false;<br>
- }<br>
-<br>
- bool GetExact_Impl(ConstString key, MapValueType &value,<br>
- RegularExpression *dummy) {<br>
- std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());<br>
- for (const auto &pos : m_format_map.map()) {<br>
- const RegularExpression ®ex = pos.first;<br>
- if (regex.GetText() == key.GetStringRef()) {<br>
- value = pos.second;<br>
- return true;<br>
- }<br>
- }<br>
- return false;<br>
- }<br>
-<br>
bool Get(const FormattersMatchVector &candidates, MapValueType &entry) {<br>
for (const FormattersMatchCandidate &candidate : candidates) {<br>
if (Get(candidate.GetTypeName(), entry)) {<br>
<br>
diff --git a/lldb/include/lldb/DataFormatters/TypeCategory.h b/lldb/include/lldb/DataFormatters/TypeCategory.h<br>
index 11614fc67cd2..4c8a7e14be12 100644<br>
--- a/lldb/include/lldb/DataFormatters/TypeCategory.h<br>
+++ b/lldb/include/lldb/DataFormatters/TypeCategory.h<br>
@@ -25,12 +25,11 @@ namespace lldb_private {<br>
<br>
template <typename FormatterImpl> class FormatterContainerPair {<br>
public:<br>
- typedef FormattersContainer<ConstString, FormatterImpl> ExactMatchContainer;<br>
- typedef FormattersContainer<RegularExpression, FormatterImpl><br>
- RegexMatchContainer;<br>
+ typedef FormattersContainer<FormatterImpl> ExactMatchContainer;<br>
+ typedef FormattersContainer<FormatterImpl> RegexMatchContainer;<br>
<br>
- typedef typename ExactMatchContainer::MapType ExactMatchMap;<br>
- typedef typename RegexMatchContainer::MapType RegexMatchMap;<br>
+ typedef TypeMatcher ExactMatchMap;<br>
+ typedef TypeMatcher RegexMatchMap;<br>
<br>
typedef typename ExactMatchContainer::MapValueType MapValueType;<br>
<br>
@@ -348,19 +347,13 @@ class TypeCategoryImpl {<br>
friend class LanguageCategory;<br>
friend class TypeCategoryMap;<br>
<br>
- friend class FormattersContainer<ConstString, TypeFormatImpl>;<br>
- friend class FormattersContainer<lldb::RegularExpressionSP, TypeFormatImpl>;<br>
+ friend class FormattersContainer<TypeFormatImpl>;<br>
<br>
- friend class FormattersContainer<ConstString, TypeSummaryImpl>;<br>
- friend class FormattersContainer<lldb::RegularExpressionSP, TypeSummaryImpl>;<br>
+ friend class FormattersContainer<TypeSummaryImpl>;<br>
<br>
- friend class FormattersContainer<ConstString, TypeFilterImpl>;<br>
- friend class FormattersContainer<lldb::RegularExpressionSP, TypeFilterImpl>;<br>
-<br>
- friend class FormattersContainer<ConstString, ScriptedSyntheticChildren>;<br>
- friend class FormattersContainer<lldb::RegularExpressionSP,<br>
- ScriptedSyntheticChildren>;<br>
+ friend class FormattersContainer<TypeFilterImpl>;<br>
<br>
+ friend class FormattersContainer<ScriptedSyntheticChildren>;<br>
};<br>
<br>
} // namespace lldb_private<br>
<br>
diff --git a/lldb/include/lldb/DataFormatters/TypeCategoryMap.h b/lldb/include/lldb/DataFormatters/TypeCategoryMap.h<br>
index 832652f7d745..6cd773786386 100644<br>
--- a/lldb/include/lldb/DataFormatters/TypeCategoryMap.h<br>
+++ b/lldb/include/lldb/DataFormatters/TypeCategoryMap.h<br>
@@ -103,7 +103,7 @@ class TypeCategoryMap {<br>
<br>
std::recursive_mutex &mutex() { return m_map_mutex; }<br>
<br>
- friend class FormattersContainer<KeyType, ValueType>;<br>
+ friend class FormattersContainer<ValueType>;<br>
friend class FormatManager;<br>
};<br>
} // namespace lldb_private<br>
<br>
diff --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp<br>
index b2020f26621f..b23f91de0ce6 100644<br>
--- a/lldb/source/Commands/CommandObjectType.cpp<br>
+++ b/lldb/source/Commands/CommandObjectType.cpp<br>
@@ -1066,13 +1066,15 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {<br>
TypeCategoryImpl::ForEachCallbacks<FormatterType> foreach;<br>
foreach<br>
.SetExact([&result, &formatter_regex, &any_printed](<br>
- ConstString name,<br>
+ const TypeMatcher &type_matcher,<br>
const FormatterSharedPointer &format_sp) -> bool {<br>
if (formatter_regex) {<br>
bool escape = true;<br>
- if (name.GetStringRef() == formatter_regex->GetText()) {<br>
+ if (type_matcher.CreatedBySameMatchString(<br>
+ ConstString(formatter_regex->GetText()))) {<br>
escape = false;<br>
- } else if (formatter_regex->Execute(name.GetStringRef())) {<br>
+ } else if (formatter_regex->Execute(<br>
+ type_matcher.GetMatchString().GetStringRef())) {<br>
escape = false;<br>
}<br>
<br>
@@ -1081,20 +1083,23 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {<br>
}<br>
<br>
any_printed = true;<br>
- result.GetOutputStream().Printf("%s: %s\n", name.AsCString(),<br>
- format_sp->GetDescription().c_str());<br>
+ result.GetOutputStream().Printf(<br>
+ "%s: %s\n", type_matcher.GetMatchString().GetCString(),<br>
+ format_sp->GetDescription().c_str());<br>
return true;<br>
});<br>
<br>
foreach<br>
.SetWithRegex([&result, &formatter_regex, &any_printed](<br>
- const RegularExpression ®ex,<br>
+ const TypeMatcher &type_matcher,<br>
const FormatterSharedPointer &format_sp) -> bool {<br>
if (formatter_regex) {<br>
bool escape = true;<br>
- if (regex.GetText() == formatter_regex->GetText()) {<br>
+ if (type_matcher.CreatedBySameMatchString(<br>
+ ConstString(formatter_regex->GetText()))) {<br>
escape = false;<br>
- } else if (formatter_regex->Execute(regex.GetText())) {<br>
+ } else if (formatter_regex->Execute(<br>
+ type_matcher.GetMatchString().GetStringRef())) {<br>
escape = false;<br>
}<br>
<br>
@@ -1103,9 +1108,9 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {<br>
}<br>
<br>
any_printed = true;<br>
- result.GetOutputStream().Printf("%s: %s\n",<br>
- regex.GetText().str().c_str(),<br>
- format_sp->GetDescription().c_str());<br>
+ result.GetOutputStream().Printf(<br>
+ "%s: %s\n", type_matcher.GetMatchString().GetCString(),<br>
+ format_sp->GetDescription().c_str());<br>
return true;<br>
});<br>
<br>
@@ -1681,10 +1686,10 @@ class CommandObjectTypeSummaryList<br>
if (DataVisualization::NamedSummaryFormats::GetCount() > 0) {<br>
result.GetOutputStream().Printf("Named summaries:\n");<br>
DataVisualization::NamedSummaryFormats::ForEach(<br>
- [&result](ConstString name,<br>
+ [&result](const TypeMatcher &type_matcher,<br>
const TypeSummaryImplSP &summary_sp) -> bool {<br>
result.GetOutputStream().Printf(<br>
- "%s: %s\n", name.AsCString(),<br>
+ "%s: %s\n", type_matcher.GetMatchString().GetCString(),<br>
summary_sp->GetDescription().c_str());<br>
return true;<br>
});<br>
<br>
diff --git a/lldb/source/DataFormatters/DataVisualization.cpp b/lldb/source/DataFormatters/DataVisualization.cpp<br>
index 450a5cbc3ef3..82248bb64285 100644<br>
--- a/lldb/source/DataFormatters/DataVisualization.cpp<br>
+++ b/lldb/source/DataFormatters/DataVisualization.cpp<br>
@@ -174,8 +174,7 @@ bool DataVisualization::NamedSummaryFormats::GetSummaryFormat(<br>
<br>
void DataVisualization::NamedSummaryFormats::Add(<br>
ConstString type, const lldb::TypeSummaryImplSP &entry) {<br>
- GetFormatManager().GetNamedSummaryContainer().Add(<br>
- FormatManager::GetValidTypeName(type), entry);<br>
+ GetFormatManager().GetNamedSummaryContainer().Add(type, entry);<br>
}<br>
<br>
bool DataVisualization::NamedSummaryFormats::Delete(ConstString type) {<br>
@@ -187,7 +186,7 @@ void DataVisualization::NamedSummaryFormats::Clear() {<br>
}<br>
<br>
void DataVisualization::NamedSummaryFormats::ForEach(<br>
- std::function<bool(ConstString, const lldb::TypeSummaryImplSP &)><br>
+ std::function<bool(const TypeMatcher &, const lldb::TypeSummaryImplSP &)><br>
callback) {<br>
GetFormatManager().GetNamedSummaryContainer().ForEach(callback);<br>
}<br>
<br>
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp<br>
index ad02d37360b8..af6df3ae6b47 100644<br>
--- a/lldb/source/DataFormatters/FormatManager.cpp<br>
+++ b/lldb/source/DataFormatters/FormatManager.cpp<br>
@@ -551,10 +551,6 @@ bool FormatManager::ShouldPrintAsOneLiner(ValueObject &valobj) {<br>
return true;<br>
}<br>
<br>
-ConstString FormatManager::GetValidTypeName(ConstString type) {<br>
- return ::GetValidTypeName_Impl(type);<br>
-}<br>
-<br>
ConstString FormatManager::GetTypeForCache(ValueObject &valobj,<br>
lldb::DynamicValueType use_dynamic) {<br>
ValueObjectSP valobj_sp = valobj.GetQualifiedRepresentationIfAvailable(<br>
<br>
diff --git a/lldb/unittests/DataFormatter/CMakeLists.txt b/lldb/unittests/DataFormatter/CMakeLists.txt<br>
index 45011c56b0b0..9d967a72bfd1 100644<br>
--- a/lldb/unittests/DataFormatter/CMakeLists.txt<br>
+++ b/lldb/unittests/DataFormatter/CMakeLists.txt<br>
@@ -1,5 +1,6 @@<br>
add_lldb_unittest(LLDBFormatterTests<br>
FormatManagerTests.cpp<br>
+ FormattersContainerTest.cpp<br>
StringPrinterTests.cpp<br>
<br>
LINK_LIBS<br>
<br>
diff --git a/lldb/unittests/DataFormatter/FormattersContainerTest.cpp b/lldb/unittests/DataFormatter/FormattersContainerTest.cpp<br>
new file mode 100644<br>
index 000000000000..a28212391eae<br>
--- /dev/null<br>
+++ b/lldb/unittests/DataFormatter/FormattersContainerTest.cpp<br>
@@ -0,0 +1,159 @@<br>
+//===-- FormattersContainerTests.cpp --------------------------------------===//<br>
+//<br>
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.<br>
+// See <a href="https://llvm.org/LICENSE.txt" rel="noreferrer" target="_blank">https://llvm.org/LICENSE.txt</a> for license information.<br>
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "lldb/DataFormatters/FormattersContainer.h"<br>
+<br>
+#include "gtest/gtest.h"<br>
+<br>
+using namespace lldb;<br>
+using namespace lldb_private;<br>
+<br>
+// All the prefixes that the exact name matching will strip from the type.<br>
+static const std::vector<std::string> exact_name_prefixes = {<br>
+ "", // no prefix.<br>
+ "class ", "struct ", "union ", "enum ",<br>
+};<br>
+<br>
+// TypeMatcher that uses a exact type name string that needs to be matched.<br>
+TEST(TypeMatcherTests, ExactName) {<br>
+ for (const std::string &prefix : exact_name_prefixes) {<br>
+ SCOPED_TRACE("Prefix: " + prefix);<br>
+<br>
+ TypeMatcher matcher(ConstString(prefix + "Name"));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("class Name")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("struct Name")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("union Name")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("enum Name")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("Name")));<br>
+<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("Name ")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("ame")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("Nam")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("am")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("a")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString(" ")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("class N")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("class ")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("class")));<br>
+ }<br>
+}<br>
+<br>
+// TypeMatcher that uses a regex to match a type name.<br>
+TEST(TypeMatcherTests, RegexName) {<br>
+ TypeMatcher matcher(RegularExpression("^a[a-z]c$"));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("abc")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("azc")));<br>
+<br>
+ // FIXME: This isn't consistent with the 'exact' type name matches above.<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("class abc")));<br>
+<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("abbc")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString(" abc")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("abc ")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString(" abc ")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("XabcX")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("ac")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("a[a-z]c")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("aAc")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("ABC")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("")));<br>
+}<br>
+<br>
+// TypeMatcher that only searches the type name.<br>
+TEST(TypeMatcherTests, RegexMatchPart) {<br>
+ TypeMatcher matcher(RegularExpression("a[a-z]c"));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("class abc")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("abc")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString(" abc ")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("azc")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("abc ")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString(" abc ")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString(" abc")));<br>
+ EXPECT_TRUE(matcher.Matches(ConstString("XabcX")));<br>
+<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("abbc")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("ac")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("a[a-z]c")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("aAc")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("ABC")));<br>
+ EXPECT_FALSE(matcher.Matches(ConstString("")));<br>
+}<br>
+<br>
+// GetMatchString for exact type name matchers.<br>
+TEST(TypeMatcherTests, GetMatchStringExactName) {<br>
+ EXPECT_EQ(TypeMatcher(ConstString("aa")).GetMatchString(), "aa");<br>
+ EXPECT_EQ(TypeMatcher(ConstString("")).GetMatchString(), "");<br>
+ EXPECT_EQ(TypeMatcher(ConstString("[a]")).GetMatchString(), "[a]");<br>
+}<br>
+<br>
+// GetMatchString for regex matchers.<br>
+TEST(TypeMatcherTests, GetMatchStringRegex) {<br>
+ EXPECT_EQ(TypeMatcher(RegularExpression("aa")).GetMatchString(), "aa");<br>
+ EXPECT_EQ(TypeMatcher(RegularExpression("")).GetMatchString(), "");<br>
+ EXPECT_EQ(TypeMatcher(RegularExpression("[a]")).GetMatchString(), "[a]");<br>
+}<br>
+<br>
+// GetMatchString for regex matchers.<br>
+TEST(TypeMatcherTests, CreatedBySameMatchString) {<br>
+ TypeMatcher empty_str(ConstString(""));<br>
+ TypeMatcher empty_regex(RegularExpression(""));<br>
+ EXPECT_TRUE(empty_str.CreatedBySameMatchString(empty_str));<br>
+ EXPECT_TRUE(empty_str.CreatedBySameMatchString(empty_regex));<br>
+<br>
+ TypeMatcher a_str(ConstString("a"));<br>
+ TypeMatcher a_regex(RegularExpression("a"));<br>
+ EXPECT_TRUE(a_str.CreatedBySameMatchString(a_str));<br>
+ EXPECT_TRUE(a_str.CreatedBySameMatchString(a_regex));<br>
+<br>
+ TypeMatcher digit_str(ConstString("[0-9]"));<br>
+ TypeMatcher digit_regex(RegularExpression("[0-9]"));<br>
+ EXPECT_TRUE(digit_str.CreatedBySameMatchString(digit_str));<br>
+ EXPECT_TRUE(digit_str.CreatedBySameMatchString(digit_regex));<br>
+<br>
+ EXPECT_FALSE(empty_str.CreatedBySameMatchString(a_str));<br>
+ EXPECT_FALSE(empty_str.CreatedBySameMatchString(a_regex));<br>
+ EXPECT_FALSE(empty_str.CreatedBySameMatchString(digit_str));<br>
+ EXPECT_FALSE(empty_str.CreatedBySameMatchString(digit_regex));<br>
+<br>
+ EXPECT_FALSE(empty_regex.CreatedBySameMatchString(a_str));<br>
+ EXPECT_FALSE(empty_regex.CreatedBySameMatchString(a_regex));<br>
+ EXPECT_FALSE(empty_regex.CreatedBySameMatchString(digit_str));<br>
+ EXPECT_FALSE(empty_regex.CreatedBySameMatchString(digit_regex));<br>
+<br>
+ EXPECT_FALSE(a_str.CreatedBySameMatchString(empty_str));<br>
+ EXPECT_FALSE(a_str.CreatedBySameMatchString(empty_regex));<br>
+ EXPECT_FALSE(a_str.CreatedBySameMatchString(digit_str));<br>
+ EXPECT_FALSE(a_str.CreatedBySameMatchString(digit_regex));<br>
+<br>
+ EXPECT_FALSE(a_regex.CreatedBySameMatchString(empty_str));<br>
+ EXPECT_FALSE(a_regex.CreatedBySameMatchString(empty_regex));<br>
+ EXPECT_FALSE(a_regex.CreatedBySameMatchString(digit_str));<br>
+ EXPECT_FALSE(a_regex.CreatedBySameMatchString(digit_regex));<br>
+<br>
+ EXPECT_FALSE(digit_str.CreatedBySameMatchString(empty_str));<br>
+ EXPECT_FALSE(digit_str.CreatedBySameMatchString(empty_regex));<br>
+ EXPECT_FALSE(digit_str.CreatedBySameMatchString(a_str));<br>
+ EXPECT_FALSE(digit_str.CreatedBySameMatchString(a_regex));<br>
+<br>
+ EXPECT_FALSE(digit_regex.CreatedBySameMatchString(empty_str));<br>
+ EXPECT_FALSE(digit_regex.CreatedBySameMatchString(empty_regex));<br>
+ EXPECT_FALSE(digit_regex.CreatedBySameMatchString(a_str));<br>
+ EXPECT_FALSE(digit_regex.CreatedBySameMatchString(a_regex));<br>
+}<br>
+<br>
+// Test CreatedBySameMatchString with stripped exact name prefixes.<br>
+TEST(TypeMatcherTests, CreatedBySameMatchStringExactNamePrefixes) {<br>
+ for (const std::string &prefix : exact_name_prefixes) {<br>
+ SCOPED_TRACE("Prefix: " + prefix);<br>
+ TypeMatcher with_prefix(ConstString(prefix + "Name"));<br>
+ TypeMatcher without_prefix(RegularExpression(""));<br>
+<br>
+ EXPECT_TRUE(with_prefix.CreatedBySameMatchString(with_prefix));<br>
+ EXPECT_TRUE(without_prefix.CreatedBySameMatchString(without_prefix));<br>
+ }<br>
+}<br>
<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
</blockquote></div>