labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
(it looks like I did not click "submit" for some reason)
================
Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:169
+ template <typename Callable> ForEachCallback(Callable c) : callback(c) {}
+ std::function<bool(const TypeMatcher &, const std::shared_ptr<T>)>
callback;
};
----------------
And one reference here as well.
================
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.
----------------
jgorbe wrote:
> 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?
No, this is what I had in mind, but I somehow missed the fact that the call
site is templated. Ok, let's stick to this then.
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
