jankratochvil marked 5 inline comments as done. jankratochvil added a comment.
In D66654#1643082 <https://reviews.llvm.org/D66654#1643082>, @JDevlieghere wrote: > How much work would it be to return an `llvm::Expected<bool>` from > `Get_Impl`? I'd really prefer that over the current approach. Done. Although now if (Get(...)) became confusing which I have commented in `lldb_private::FormatDropExpected()`. BTW I haven't found how to format a table for Doxygen. Also by searching around I have found other tables in LLDB/LLVM are also not recognized by Doxygen as tables. ================ Comment at: lldb/include/lldb/DataFormatters/FormattersContainer.h:296 - bool Get_Impl(ConstString key, MapValueType &value, + bool Get_Impl(ConstString key, MapValueType &value, ValueObject *valobj, lldb::RegularExpressionSP *dummy) { ---------------- JDevlieghere wrote: > If returning an expected isn't feasible, we should pass at least a `Status*` > instead of setting the error in the `ValueObject` directly. With `Expected<bool>` this is no longer applicable. ================ Comment at: lldb/source/DataFormatters/TypeCategory.cpp:303 + if (GetTypeFormatsContainer()->Get(type_name, format_sp, + nullptr /*valobj*/)) { if (matching_category) ---------------- JDevlieghere wrote: > It's really unfortunate that so many call sites now require a null pointer. > Can we make it a default argument? With `Expected<bool>` this is no longer applicable. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66654/new/ https://reviews.llvm.org/D66654 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
