[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidType, a force-checked recoverable error

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: include/lldb/Utility/Log.h:249-254
+      ::lldb_private::Log *log_private = (log);                                \
+      if (log_private)                                                         \
+        log_private->FormatError(::std::move(error_private), __FILE__,         \
+                                 __func__, "{0}");                             \
+      else                                                                     \
+        ::llvm::consumeError(::std::move(error_private));                      \
----------------
it looks like this could be just handled by delegating to the LLDB_LOG_ERROR macro.


================
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;
+    }
----------------
vsk wrote:
> 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.
Yeah, I'm fine with the new macro as well. I see it mostly as a transitionary measure. Once most of the code uses llvm::Error, we should be able to just propagate that (or do something interesting with the error).


https://reviews.llvm.org/D43912