vsk added inline comments.
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:335-340
+ if (llvm::Error E = user_type_or_err.takeError()) {
+ std::string Reason = llvm::toString(std::move(E));
+ if (log)
+ log->Printf("%s", Reason.c_str());
+ return false;
+ }
----------------
labath wrote:
> How about:
> ```
> if (!user_type_or_err) {
> LLDB_LOG_ERROR(log, user_type_or_err.takeError(), "{0}");
> return false;
> }
> ```
I think it'd be even more convenient to include the return into the macro, as
Zachary suggested.
================
Comment at:
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:382-387
+ if (llvm::Error E = user_type_or_err.takeError()) {
+ std::string Reason = llvm::toString(std::move(E));
if (log)
- log->Printf("Persistent variable's type wasn't copied successfully");
+ log->Printf("%s", Reason.c_str());
return false;
}
----------------
zturner wrote:
> I wonder if we need a macro like
>
> ```
> #define RETURN_IF_UNEXPECTED(item, ret_val, log_categories) \
> if (auto E = item.takeError()) { \
> std::string Reason = llvm::toString(std::move(E)); \
> Log *log(lldb_private::GetLogIfAllCategoriesSet(log_categories)); \
> if (log) \
> log->Printf("%s", Reason.c_str());
> return ret_val;
> }
> ```
>
> then we say:
>
> ```
> RETURN_IF_UNEXPECTED(user_type_or_err, false, LIBLLDB_LOG_EXPRESSIONS);
> ```
>
> I don't have too strong of an opinion, but as this pattern becomes more
> pervasive, it's annoying to have to write out 5-6 lines of code every time
> you call a function that returns an `Expected<T>`.
Thanks for the suggestion. I've introduced a macro which is pretty similar to
what you've shown here.
https://reviews.llvm.org/D43912
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
