[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGef3fa232b338: [Formatters][NFCI] Replace
is_regex arguments with an enum. (authored by jgorbe).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133240/new/
https://reviews.llvm.org/D133240
Files:
lldb/bindings/interface/SBTypeNameSpecifier.i
lldb/include/lldb/API/SBTypeNameSpecifier.h
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SBTypeNameSpecifier.cpp
lldb/source/Core/FormatEntity.cpp
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
target->GetBitfieldBitSize());
auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
if (val_obj_display ==
ValueObject::eValueObjectRepresentationStyleSummary &&
!DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+ LLDB_INSTRUMENT_VA(this, name, match_type);
if (name == nullptr || (*name) == 0)
m_opaque_sp.reset();
@@ -72,13 +79,20 @@
return SBType();
}
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+ LLDB_INSTRUMENT_VA(this);
+ if (!IsValid())
+return eFormatterMatchExact;
+ return m_opaque_sp->GetMatchType();
+}
+
bool SBTypeNameSpecifier::IsRegex() {
LLDB_INSTRUMENT_VA(this);
if (!IsValid())
return false;
- return m_opaque_sp->IsRegex();
+ return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
}
bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
if (!IsValid())
return !rhs.IsValid();
- if (IsRegex() != rhs.IsRegex())
+ if (GetMatchType() != rhs.GetMatchType())
return false;
if (GetName() == nullptr || rhs.GetName() == nullptr)
return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
eTemplateArgumentKindNullPtr,
};
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+ eFormatterMatchExact,
+ eFormatterMatchRegex,
+
+ eLastFormatterMatchType = eFormatterMatchRegex,
+};
+
/// Options that can be set for a formatter to alter its behavior. Not
/// all of these are applicable to all formatter types.
FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -41,9 +41,11 @@
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;
+ /// Indicates what kind of matching strategy should be used:
+ /// - eFormatterMatchExact: match the exact type name in m_type_name.
+ /// - eFormatterMatchRegex: match using the RegularExpression object
+ /// `m_type_name_regex` instead.
+ lldb::FormatterMatchType m_match_type;
// 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
@@ -71,22 +73,25 @@
TypeMatcher() = delete;
/// 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) {}
+ : m_type_name(type_name), m_match_type(lldb::eFormatterMatchExact)
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe updated this revision to Diff 459845.
jgorbe added a comment.
Replaced `kNumFormatterMatchTypes` with `eLastFormatterMatchType` to avoid
"unhandled enum value" warnings, as suggested by Pavel.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133240/new/
https://reviews.llvm.org/D133240
Files:
lldb/bindings/interface/SBTypeNameSpecifier.i
lldb/include/lldb/API/SBTypeNameSpecifier.h
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SBTypeNameSpecifier.cpp
lldb/source/Core/FormatEntity.cpp
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
target->GetBitfieldBitSize());
auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
if (val_obj_display ==
ValueObject::eValueObjectRepresentationStyleSummary &&
!DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+ LLDB_INSTRUMENT_VA(this, name, match_type);
if (name == nullptr || (*name) == 0)
m_opaque_sp.reset();
@@ -72,13 +79,20 @@
return SBType();
}
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+ LLDB_INSTRUMENT_VA(this);
+ if (!IsValid())
+return eFormatterMatchExact;
+ return m_opaque_sp->GetMatchType();
+}
+
bool SBTypeNameSpecifier::IsRegex() {
LLDB_INSTRUMENT_VA(this);
if (!IsValid())
return false;
- return m_opaque_sp->IsRegex();
+ return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
}
bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
if (!IsValid())
return !rhs.IsValid();
- if (IsRegex() != rhs.IsRegex())
+ if (GetMatchType() != rhs.GetMatchType())
return false;
if (GetName() == nullptr || rhs.GetName() == nullptr)
return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
eTemplateArgumentKindNullPtr,
};
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+ eFormatterMatchExact,
+ eFormatterMatchRegex,
+
+ eLastFormatterMatchType = eFormatterMatchRegex,
+};
+
/// Options that can be set for a formatter to alter its behavior. Not
/// all of these are applicable to all formatter types.
FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -41,9 +41,11 @@
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;
+ /// Indicates what kind of matching strategy should be used:
+ /// - eFormatterMatchExact: match the exact type name in m_type_name.
+ /// - eFormatterMatchRegex: match using the RegularExpression object
+ /// `m_type_name_regex` instead.
+ lldb::FormatterMatchType m_match_type;
// 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
@@ -71,22 +73,25 @@
TypeMatcher() = delete;
/// 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) {}
+ : m_type_name(type_name), m_match_type(lldb::eFormatterMatchExact) {}
/// Creates a matcher that accepts any type matching the given regex.
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe added inline comments. Comment at: lldb/include/lldb/lldb-enumerations.h:841 + + kNumFormatterMatchTypes, +}; labath wrote: > I see the enums in this file use wildly inconsistent styles for the "largest > value" enumerator. However, you've picked the one I like the least, because > this tends to produce `unhandled switch case "kNumFormatterMatchTypes"` > warnings. If you feel like you need to have a sentinel, I'd recommend going > with the `eLastFormatterMatchType = ` pattern (used e.g. in > the StateType enum). I //do// want a sentinel because the plan is to replace some hardcoded instances of "check for an exact match and if nothing is found then check for a regex match" with a class that has an array of size `kNumFormatterMatchTypes` formatter containers, and knows how to do a lookup in priority order. So then I can add another "priority tier" for callback matchers and not add extra logic in a bunch of places. But I have no strong preference between having a `std::array` and a `std::array`, and I agree the `unhandled switch case` warnings are annoying. I'll change it shortly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133240/new/ https://reviews.llvm.org/D133240 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This seems in line with what was being discussed. Jim, do you have any thoughts on this? Comment at: lldb/include/lldb/lldb-enumerations.h:841 + + kNumFormatterMatchTypes, +}; I see the enums in this file use wildly inconsistent styles for the "largest value" enumerator. However, you've picked the one I like the least, because this tends to produce `unhandled switch case "kNumFormatterMatchTypes"` warnings. If you feel like you need to have a sentinel, I'd recommend going with the `eLastFormatterMatchType = ` pattern (used e.g. in the StateType enum). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133240/new/ https://reviews.llvm.org/D133240 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe updated this revision to Diff 459618.
jgorbe added a comment.
Fixed mis-spelled member name in comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133240/new/
https://reviews.llvm.org/D133240
Files:
lldb/bindings/interface/SBTypeNameSpecifier.i
lldb/include/lldb/API/SBTypeNameSpecifier.h
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SBTypeNameSpecifier.cpp
lldb/source/Core/FormatEntity.cpp
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
target->GetBitfieldBitSize());
auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
if (val_obj_display ==
ValueObject::eValueObjectRepresentationStyleSummary &&
!DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+ LLDB_INSTRUMENT_VA(this, name, match_type);
if (name == nullptr || (*name) == 0)
m_opaque_sp.reset();
@@ -72,13 +79,20 @@
return SBType();
}
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+ LLDB_INSTRUMENT_VA(this);
+ if (!IsValid())
+return eFormatterMatchExact;
+ return m_opaque_sp->GetMatchType();
+}
+
bool SBTypeNameSpecifier::IsRegex() {
LLDB_INSTRUMENT_VA(this);
if (!IsValid())
return false;
- return m_opaque_sp->IsRegex();
+ return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
}
bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
if (!IsValid())
return !rhs.IsValid();
- if (IsRegex() != rhs.IsRegex())
+ if (GetMatchType() != rhs.GetMatchType())
return false;
if (GetName() == nullptr || rhs.GetName() == nullptr)
return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
eTemplateArgumentKindNullPtr,
};
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+ eFormatterMatchExact,
+ eFormatterMatchRegex,
+
+ kNumFormatterMatchTypes,
+};
+
/// Options that can be set for a formatter to alter its behavior. Not
/// all of these are applicable to all formatter types.
FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -41,9 +41,11 @@
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;
+ /// Indicates what kind of matching strategy should be used:
+ /// - eFormatterMatchExact: match the exact type name in m_type_name.
+ /// - eFormatterMatchRegex: match using the RegularExpression object
+ /// `m_type_name_regex` instead.
+ lldb::FormatterMatchType m_match_type;
// 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
@@ -71,22 +73,25 @@
TypeMatcher() = delete;
/// 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) {}
+ : m_type_name(type_name), m_match_type(lldb::eFormatterMatchExact) {}
/// Creates a matcher that accepts any type matching the given regex.
TypeMatcher(RegularExpression regex)
- : m_type_name_regex(std::move(regex)), m_is_regex(true) {}
+ :
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe marked an inline comment as done. jgorbe added inline comments. Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:46 /// just matching by comparing with m_type_name string. - bool m_is_regex; + lldb::FormatterMatchType m_match_type; hawkinsw wrote: > I am really intrigued by this entire patchset and just wanted to drop by to > help, if I could. I hope that I am not upsetting you by commenting on > something so trivial, but it seems like we would want to change the comment > for this data member while we are at it? > > Again, I think this work is great and I hope I am helping! Good catch! I have updated the comment. Thanks for pointing it out :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133240/new/ https://reviews.llvm.org/D133240 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe updated this revision to Diff 459610.
jgorbe added a comment.
Updated comment above the declaration of TypeMatcher::m_match_type that
referred to the old m_is_regex boolean, to address review feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133240/new/
https://reviews.llvm.org/D133240
Files:
lldb/bindings/interface/SBTypeNameSpecifier.i
lldb/include/lldb/API/SBTypeNameSpecifier.h
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SBTypeNameSpecifier.cpp
lldb/source/Core/FormatEntity.cpp
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
target->GetBitfieldBitSize());
auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
if (val_obj_display ==
ValueObject::eValueObjectRepresentationStyleSummary &&
!DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+ LLDB_INSTRUMENT_VA(this, name, match_type);
if (name == nullptr || (*name) == 0)
m_opaque_sp.reset();
@@ -72,13 +79,20 @@
return SBType();
}
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+ LLDB_INSTRUMENT_VA(this);
+ if (!IsValid())
+return eFormatterMatchExact;
+ return m_opaque_sp->GetMatchType();
+}
+
bool SBTypeNameSpecifier::IsRegex() {
LLDB_INSTRUMENT_VA(this);
if (!IsValid())
return false;
- return m_opaque_sp->IsRegex();
+ return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
}
bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
if (!IsValid())
return !rhs.IsValid();
- if (IsRegex() != rhs.IsRegex())
+ if (GetMatchType() != rhs.GetMatchType())
return false;
if (GetName() == nullptr || rhs.GetName() == nullptr)
return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
eTemplateArgumentKindNullPtr,
};
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+ eFormatterMatchExact,
+ eFormatterMatchRegex,
+
+ kNumFormatterMatchTypes,
+};
+
/// Options that can be set for a formatter to alter its behavior. Not
/// all of these are applicable to all formatter types.
FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -41,9 +41,11 @@
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;
+ /// Indicates what kind of matching strategy should be used:
+ /// - eFormatterMatchExact: match the exact type name in m_name.
+ /// - eFormatterMatchRegex: match using the RegularExpression object
+ /// `m_type_name_regex` instead.
+ lldb::FormatterMatchType m_match_type;
// 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
@@ -71,22 +73,25 @@
TypeMatcher() = delete;
/// 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) {}
+ : m_type_name(type_name), m_match_type(lldb::eFormatterMatchExact) {}
/// Creates a matcher that accepts any type matching the given regex.
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
hawkinsw added inline comments. Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:46 /// just matching by comparing with m_type_name string. - bool m_is_regex; + lldb::FormatterMatchType m_match_type; I am really intrigued by this entire patchset and just wanted to drop by to help, if I could. I hope that I am not upsetting you by commenting on something so trivial, but it seems like we would want to change the comment for this data member while we are at it? Again, I think this work is great and I hope I am helping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133240/new/ https://reviews.llvm.org/D133240 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe added a comment.
This is the first step towards something like this big diff:
https://reviews.llvm.org/differential/diff/457740/. It's a pretty simple patch,
but I would like some feedback about whether you think it's going in the right
direction or I should do something else instead.
Here's some more context for that diff linked above. There is, as you could
expect, some amount of plumbing so that at callback matching time we have
around pointers to the script interpreter, the type of the value, etc, and we
can call the callback. But the biggest problem I've found is that there's some
logic that is duplicated for each one of {exact, regex} AND for each kind of
formatter (summaries, synthetic child providers...). An example of this is
`TypeCategoryImpl::AnyMatches`. Adding a new matching strategy ({exact, regex,
callback}) makes the combinatorial problem even worse. So a big chunk of that
patch is moving stuff around to try to minimize this problem.
The main refactoring is replacing the existing `FormatterContainerPair` (that
has two members that contain exact match formatters and regex formatters,
respectively) with a `TieredFormatterContainer` which has an array of
containers in priority order. The idea is to sink as much of duplicated logic
as possible around `TypeCategoryImpl` (for example, `GetFormatAtIndex`,
`GetSummaryAtIndex`, `GetFilterAtIndex`...) into this
`TieredFormatterContainer` to reuse the implementation.
I hope this helps understand the general idea a bit better.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133240/new/
https://reviews.llvm.org/D133240
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.
jgorbe created this revision.
jgorbe added reviewers: jingham, labath.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.
Modify `SBTypeNameSpecifier` and `lldb_private::TypeMatcher` so they
have an enum value for the type of matching to perform instead of an
`m_is_regex` boolean value.
This change paves the way for introducing formatter matching based on
the result of a python callback in addition to the existing name-based
matching. See the RFC thread at
https://discourse.llvm.org/t/rfc-python-callback-for-data-formatters-type-matching/64204
for more details.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D133240
Files:
lldb/bindings/interface/SBTypeNameSpecifier.i
lldb/include/lldb/API/SBTypeNameSpecifier.h
lldb/include/lldb/DataFormatters/FormatClasses.h
lldb/include/lldb/DataFormatters/FormattersContainer.h
lldb/include/lldb/lldb-enumerations.h
lldb/source/API/SBTypeNameSpecifier.cpp
lldb/source/Core/FormatEntity.cpp
Index: lldb/source/Core/FormatEntity.cpp
===
--- lldb/source/Core/FormatEntity.cpp
+++ lldb/source/Core/FormatEntity.cpp
@@ -825,7 +825,7 @@
bitfield_name.Printf("%s:%d", target->GetTypeName().AsCString(),
target->GetBitfieldBitSize());
auto type_sp = std::make_shared(
-bitfield_name.GetString(), false);
+bitfield_name.GetString(), lldb::eFormatterMatchExact);
if (val_obj_display ==
ValueObject::eValueObjectRepresentationStyleSummary &&
!DataVisualization::GetSummaryForType(type_sp))
Index: lldb/source/API/SBTypeNameSpecifier.cpp
===
--- lldb/source/API/SBTypeNameSpecifier.cpp
+++ lldb/source/API/SBTypeNameSpecifier.cpp
@@ -20,8 +20,15 @@
SBTypeNameSpecifier::SBTypeNameSpecifier() { LLDB_INSTRUMENT_VA(this); }
SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name, bool is_regex)
-: m_opaque_sp(new TypeNameSpecifierImpl(name, is_regex)) {
+: SBTypeNameSpecifier(name, is_regex ? eFormatterMatchRegex
+ : eFormatterMatchExact) {
LLDB_INSTRUMENT_VA(this, name, is_regex);
+}
+
+SBTypeNameSpecifier::SBTypeNameSpecifier(const char *name,
+ FormatterMatchType match_type)
+: m_opaque_sp(new TypeNameSpecifierImpl(name, match_type)) {
+ LLDB_INSTRUMENT_VA(this, name, match_type);
if (name == nullptr || (*name) == 0)
m_opaque_sp.reset();
@@ -72,13 +79,20 @@
return SBType();
}
+FormatterMatchType SBTypeNameSpecifier::GetMatchType() {
+ LLDB_INSTRUMENT_VA(this);
+ if (!IsValid())
+return eFormatterMatchExact;
+ return m_opaque_sp->GetMatchType();
+}
+
bool SBTypeNameSpecifier::IsRegex() {
LLDB_INSTRUMENT_VA(this);
if (!IsValid())
return false;
- return m_opaque_sp->IsRegex();
+ return m_opaque_sp->GetMatchType() == eFormatterMatchRegex;
}
bool SBTypeNameSpecifier::GetDescription(
@@ -116,7 +130,7 @@
if (!IsValid())
return !rhs.IsValid();
- if (IsRegex() != rhs.IsRegex())
+ if (GetMatchType() != rhs.GetMatchType())
return false;
if (GetName() == nullptr || rhs.GetName() == nullptr)
return false;
Index: lldb/include/lldb/lldb-enumerations.h
===
--- lldb/include/lldb/lldb-enumerations.h
+++ lldb/include/lldb/lldb-enumerations.h
@@ -832,6 +832,15 @@
eTemplateArgumentKindNullPtr,
};
+/// Type of match to be performed when looking for a formatter for a data type.
+/// Used by classes like SBTypeNameSpecifier or lldb_private::TypeMatcher.
+enum FormatterMatchType {
+ eFormatterMatchExact,
+ eFormatterMatchRegex,
+
+ kNumFormatterMatchTypes,
+};
+
/// Options that can be set for a formatter to alter its behavior. Not
/// all of these are applicable to all formatter types.
FLAGS_ENUM(TypeOptions){eTypeOptionNone = (0u),
Index: lldb/include/lldb/DataFormatters/FormattersContainer.h
===
--- lldb/include/lldb/DataFormatters/FormattersContainer.h
+++ lldb/include/lldb/DataFormatters/FormattersContainer.h
@@ -43,7 +43,7 @@
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;
+ lldb::FormatterMatchType m_match_type;
// 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
@@ -71,22 +71,25 @@
TypeMatcher() = delete;
/// 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) {}
+ : m_type_name(type_name),
