jgorbe marked an inline comment as done.
jgorbe added inline comments.
================
Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:136
+ void ForEach(std::function<bool(const TypeMatcher &,
+ const std::shared_ptr<FormatterImpl>)>
+ callback) {
----------------
labath wrote:
> did you want to add a reference here? A const by-value argument is not
> particularly useful.
Yeah, I just changed it. Thanks for catching it!
================
Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:159-162
+ // TypeFilterImpl inherits from SyntheticChildren, so we can't simply
overload
+ // ForEach on the type of the callback because it would result in "call to
+ // member function 'ForEach' is ambiguous" errors. Instead we use this
+ // templated struct to hold the formatter type and the callback.
----------------
labath wrote:
> What if we just embed the type information into the method name? (So we could
> have a set of `ForEachFormat`,`ForEachSummary`, ... methods instead of a
> single overloaded ForEach)
The problem with that is that the call site is
```
template <typename FormatterType>
class CommandObjectTypeFormatterList {
[...]
bool DoExecute(...) {
TypeCategoryImpl::ForEachCallbacks<FormatterType> foreach;
category->ForEach(foreach);
```
So if we want to keep that template for `CommandObjectTypeFormatterList` to
avoid repeating the implementation of `type {format, summary, filter,
synthetic} list`, we still need to switch based on type //somewhere//. So it
might as well be here. Or did you have anything else in mind?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134771/new/
https://reviews.llvm.org/D134771
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
