[Lldb-commits] [PATCH] D134771: [NFCI] Simplify TypeCategoryImpl for-each callbacks.
Jorge Gorbe Moya via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 28 11:06:23 PDT 2022
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
More information about the lldb-commits
mailing list