Skip to content

Commit

Permalink
Temporarily Revert "Reland [lldb] Unify type name matching in Formatt…
Browse files Browse the repository at this point in the history
…ersContainer"

as it breaks bots with due to m_valid being an unused class member
except in assert builds.

This reverts commit 074b121.
  • Loading branch information
echristo committed Jul 23, 2020
1 parent 55c0f12 commit 3a75466
Show file tree
Hide file tree
Showing 10 changed files with 179 additions and 302 deletions.
6 changes: 3 additions & 3 deletions lldb/include/lldb/DataFormatters/DataVisualization.h
Expand Up @@ -69,9 +69,9 @@ class DataVisualization {

static void Clear();

static void ForEach(std::function<bool(const TypeMatcher &,
const lldb::TypeSummaryImplSP &)>
callback);
static void
ForEach(std::function<bool(ConstString, const lldb::TypeSummaryImplSP &)>
callback);

static uint32_t GetCount();
};
Expand Down
9 changes: 8 additions & 1 deletion lldb/include/lldb/DataFormatters/FormatManager.h
Expand Up @@ -34,7 +34,7 @@ namespace lldb_private {
// this file's objects directly

class FormatManager : public IFormatChangeListener {
typedef FormatMap<TypeSummaryImpl> NamedSummariesMap;
typedef FormatMap<ConstString, TypeSummaryImpl> NamedSummariesMap;
typedef TypeCategoryMap::MapType::iterator CategoryMapIterator;

public:
Expand Down Expand Up @@ -144,6 +144,13 @@ 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
Expand Down
241 changes: 132 additions & 109 deletions lldb/include/lldb/DataFormatters/FormattersContainer.h
Expand Up @@ -37,113 +37,57 @@ class IFormatChangeListener {
virtual uint32_t GetCurrentRevision() = 0;
};

/// 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());
}
// 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;

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);
}
std::string type_cstr(type.AsCString());
StringLexer type_lexer(type_cstr);

/// Returns the underlying match string for this TypeMatcher.
ConstString GetMatchString() const {
assert(m_valid && "Using invalid TypeMatcher");
type_lexer.AdvanceIf("class ");
type_lexer.AdvanceIf("enum ");
type_lexer.AdvanceIf("struct ");
type_lexer.AdvanceIf("union ");

if (m_is_regex)
return ConstString(m_type_name_regex.GetText());
return StripTypeName(m_type_name);
}
while (type_lexer.NextIf({' ', '\t', '\v', '\f'}).first)
;

/// 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();
}
};
return ConstString(type_lexer.GetUnlexed());
}

template <typename ValueType> class FormattersContainer;
template <typename KeyType, typename ValueType> class FormattersContainer;

template <typename ValueType> class FormatMap {
template <typename KeyType, typename ValueType> class FormatMap {
public:
typedef typename ValueType::SharedPointer ValueSP;
typedef std::vector<std::pair<TypeMatcher, ValueSP>> MapType;
typedef std::vector<std::pair<KeyType, ValueSP>> MapType;
typedef typename MapType::iterator MapIterator;
typedef std::function<bool(const TypeMatcher &, const ValueSP &)>
ForEachCallback;
typedef std::function<bool(const KeyType &, const ValueSP &)> ForEachCallback;

FormatMap(IFormatChangeListener *lst)
: m_map(), m_map_mutex(), listener(lst) {}

void Add(TypeMatcher matcher, const ValueSP &entry) {
void Add(KeyType name, const ValueSP &entry) {
if (listener)
entry->GetRevision() = listener->GetCurrentRevision();
else
entry->GetRevision() = 0;

std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
Delete(matcher);
m_map.emplace_back(std::move(matcher), std::move(entry));
Delete(name);
m_map.emplace_back(std::move(name), std::move(entry));
if (listener)
listener->Changed();
}

bool Delete(const TypeMatcher &matcher) {
bool Delete(const KeyType &name) {
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
for (MapIterator iter = m_map.begin(); iter != m_map.end(); ++iter)
if (iter->first.CreatedBySameMatchString(matcher)) {
if (iter->first == name) {
m_map.erase(iter);
if (listener)
listener->Changed();
Expand All @@ -159,10 +103,10 @@ template <typename ValueType> class FormatMap {
listener->Changed();
}

bool Get(const TypeMatcher &matcher, ValueSP &entry) {
bool Get(const KeyType &name, ValueSP &entry) {
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
for (const auto &pos : m_map)
if (pos.first.CreatedBySameMatchString(matcher)) {
if (pos.first == name) {
entry = pos.second;
return true;
}
Expand All @@ -173,7 +117,7 @@ template <typename ValueType> class FormatMap {
if (callback) {
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
for (const auto &pos : m_map) {
const TypeMatcher &type = pos.first;
const KeyType &type = pos.first;
if (!callback(type, pos.second))
break;
}
Expand All @@ -190,10 +134,10 @@ template <typename ValueType> class FormatMap {
}

// If caller holds the mutex we could return a reference without copy ctor.
llvm::Optional<TypeMatcher> GetKeyAtIndex(size_t index) {
KeyType GetKeyAtIndex(size_t index) {
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
if (index >= m_map.size())
return llvm::None;
return {};
return m_map[index].first;
}

Expand All @@ -206,56 +150,50 @@ template <typename ValueType> class FormatMap {

std::recursive_mutex &mutex() { return m_map_mutex; }

friend class FormattersContainer<ValueType>;
friend class FormattersContainer<KeyType, ValueType>;
friend class FormatManager;
};

template <typename ValueType> class FormattersContainer {
template <typename KeyType, typename ValueType> class FormattersContainer {
protected:
typedef FormatMap<ValueType> BackEndType;
typedef FormatMap<KeyType, 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<ValueType>>
typedef typename std::shared_ptr<FormattersContainer<KeyType, ValueType>>
SharedPointer;

friend class TypeCategoryImpl;

FormattersContainer(IFormatChangeListener *lst) : m_format_map(lst) {}

void Add(TypeMatcher type, const MapValueType &entry) {
m_format_map.Add(std::move(type), entry);
void Add(MapKeyType type, const MapValueType &entry) {
Add_Impl(std::move(type), entry, static_cast<KeyType *>(nullptr));
}

bool Delete(TypeMatcher type) { return m_format_map.Delete(type); }
bool Delete(ConstString type) {
return Delete_Impl(type, static_cast<KeyType *>(nullptr));
}

bool Get(ConstString type, MapValueType &entry) {
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;
return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
}

bool GetExact(ConstString type, MapValueType &entry) {
return m_format_map.Get(type, entry);
return GetExact_Impl(type, entry, static_cast<KeyType *>(nullptr));
}

MapValueType GetAtIndex(size_t index) {
return m_format_map.GetValueAtIndex(index);
}

lldb::TypeNameSpecifierImplSP GetTypeNameSpecifierAtIndex(size_t index) {
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));
return GetTypeNameSpecifierAtIndex_Impl(index,
static_cast<KeyType *>(nullptr));
}

void Clear() { m_format_map.Clear(); }
Expand All @@ -270,6 +208,91 @@ template <typename ValueType> class FormattersContainer {
FormattersContainer(const FormattersContainer &) = delete;
const FormattersContainer &operator=(const FormattersContainer &) = delete;

void Add_Impl(MapKeyType type, const MapValueType &entry,
RegularExpression *dummy) {
m_format_map.Add(std::move(type), entry);
}

void Add_Impl(ConstString type, const MapValueType &entry,
ConstString *dummy) {
m_format_map.Add(GetValidTypeName_Impl(type), entry);
}

bool Delete_Impl(ConstString type, ConstString *dummy) {
return m_format_map.Delete(type);
}

bool Delete_Impl(ConstString type, RegularExpression *dummy) {
std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
MapIterator pos, end = m_format_map.map().end();
for (pos = m_format_map.map().begin(); pos != end; pos++) {
const RegularExpression &regex = pos->first;
if (type.GetStringRef() == regex.GetText()) {
m_format_map.map().erase(pos);
if (m_format_map.listener)
m_format_map.listener->Changed();
return true;
}
}
return false;
}

bool Get_Impl(ConstString type, MapValueType &entry, ConstString *dummy) {
return m_format_map.Get(type, entry);
}

bool GetExact_Impl(ConstString type, MapValueType &entry,
ConstString *dummy) {
return Get_Impl(type, entry, static_cast<KeyType *>(nullptr));
}

lldb::TypeNameSpecifierImplSP
GetTypeNameSpecifierAtIndex_Impl(size_t index, ConstString *dummy) {
ConstString key = m_format_map.GetKeyAtIndex(index);
if (key)
return lldb::TypeNameSpecifierImplSP(
new TypeNameSpecifierImpl(key.GetStringRef(), false));
else
return lldb::TypeNameSpecifierImplSP();
}

lldb::TypeNameSpecifierImplSP
GetTypeNameSpecifierAtIndex_Impl(size_t index, RegularExpression *dummy) {
RegularExpression regex = m_format_map.GetKeyAtIndex(index);
if (regex == RegularExpression())
return lldb::TypeNameSpecifierImplSP();
return lldb::TypeNameSpecifierImplSP(
new TypeNameSpecifierImpl(regex.GetText().str().c_str(), true));
}

bool Get_Impl(ConstString key, MapValueType &value,
RegularExpression *dummy) {
llvm::StringRef key_str = key.GetStringRef();
std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
// Patterns are matched in reverse-chronological order.
for (const auto &pos : llvm::reverse(m_format_map.map())) {
const RegularExpression &regex = pos.first;
if (regex.Execute(key_str)) {
value = pos.second;
return true;
}
}
return false;
}

bool GetExact_Impl(ConstString key, MapValueType &value,
RegularExpression *dummy) {
std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
for (const auto &pos : m_format_map.map()) {
const RegularExpression &regex = pos.first;
if (regex.GetText() == key.GetStringRef()) {
value = pos.second;
return true;
}
}
return false;
}

bool Get(const FormattersMatchVector &candidates, MapValueType &entry) {
for (const FormattersMatchCandidate &candidate : candidates) {
if (Get(candidate.GetTypeName(), entry)) {
Expand Down

0 comments on commit 3a75466

Please sign in to comment.