vsk updated this revision to Diff 136640.
vsk retitled this revision from "[Symbol] Add InvalidType, a force-checked
recoverable error" to "[Symbol] Add InvalidTypeError, a force-checked
recoverable error".
vsk added a comment.
- While playing around with InvalidTypeError, I found that it's useful to be
able to pass a log category mask to the error-logging macros. I've implemented
support for that.
- I also found that the RETURN_IF_UNEXPECTED macro is very convenient in
practice. There seems to be no objection to it provided that I document how to
set breakpoints for failure cases. I've brought the macro back.
Thanks for your feedback! PTAL.
https://reviews.llvm.org/D43912
Files:
include/lldb/Symbol/CompilerType.h
include/lldb/Utility/Log.h
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
source/Symbol/CompilerType.cpp
Index: source/Symbol/CompilerType.cpp
===================================================================
--- source/Symbol/CompilerType.cpp
+++ source/Symbol/CompilerType.cpp
@@ -1080,3 +1080,5 @@
return lhs.GetTypeSystem() != rhs.GetTypeSystem() ||
lhs.GetOpaqueQualType() != rhs.GetOpaqueQualType();
}
+
+char lldb_private::InvalidTypeError::ID;
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -625,10 +625,11 @@
/// The type as it appears in the source context.
///
/// @return
- /// Returns the moved type, or an empty type if there was a problem.
+ /// Returns the moved type, or an error.
//------------------------------------------------------------------
- TypeFromUser DeportType(ClangASTContext &target, ClangASTContext &source,
- TypeFromParser parser_type);
+ llvm::Expected<TypeFromUser> DeportType(ClangASTContext &target,
+ ClangASTContext &source,
+ TypeFromParser parser_type);
ClangASTContext *GetClangASTContext();
};
Index: source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -277,9 +277,10 @@
return ret;
}
-TypeFromUser ClangExpressionDeclMap::DeportType(ClangASTContext &target,
- ClangASTContext &source,
- TypeFromParser parser_type) {
+llvm::Expected<TypeFromUser>
+ClangExpressionDeclMap::DeportType(ClangASTContext &target,
+ ClangASTContext &source,
+ TypeFromParser parser_type) {
assert (&target == m_target->GetScratchClangASTContext());
assert ((TypeSystem*)&source == parser_type.GetTypeSystem());
assert (source.getASTContext() == m_ast_context);
@@ -302,10 +303,10 @@
source_file,
clang::QualType::getFromOpaquePtr(parser_type.GetOpaqueQualType()));
return TypeFromUser(exported_type.getAsOpaquePtr(), &target);
- } else {
- lldbassert(0 && "No mechanism for deporting a type!");
- return TypeFromUser();
}
+
+ lldbassert(0 && "No mechanism for deporting a type!");
+ return llvm::make_error<InvalidTypeError>("Could not deport type");
}
bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
@@ -328,8 +329,12 @@
if (target == nullptr)
return false;
- TypeFromUser user_type =
+ llvm::Expected<TypeFromUser> user_type_or_err =
DeportType(*target->GetScratchClangASTContext(), *ast, parser_type);
+ RETURN_IF_UNEXPECTED(user_type_or_err, false, LIBLLDB_LOG_EXPRESSIONS);
+ TypeFromUser &user_type = *user_type_or_err;
+ assert(user_type.IsValid() &&
+ "Persistent variable's type wasn't copied successfully");
uint32_t offset = m_parser_vars->m_materializer->AddResultVariable(
user_type, is_lvalue, m_keep_result_in_memory, m_result_delegate, err);
@@ -366,13 +371,12 @@
ClangASTContext *context(target->GetScratchClangASTContext());
- TypeFromUser user_type = DeportType(*context, *ast, parser_type);
-
- if (!user_type.GetOpaqueQualType()) {
- if (log)
- log->Printf("Persistent variable's type wasn't copied successfully");
- return false;
- }
+ llvm::Expected<TypeFromUser> user_type_or_err =
+ DeportType(*context, *ast, parser_type);
+ RETURN_IF_UNEXPECTED(user_type_or_err, false, log);
+ TypeFromUser &user_type = *user_type_or_err;
+ assert(user_type.IsValid() &&
+ "Persistent variable's type wasn't copied successfully");
if (!m_parser_vars->m_target_info.IsValid())
return false;
Index: include/lldb/Utility/Log.h
===================================================================
--- include/lldb/Utility/Log.h
+++ include/lldb/Utility/Log.h
@@ -212,6 +212,23 @@
void operator=(const Log &) = delete;
};
+// Helper utilities for macros which consume and log errors. These are not
+// meant to be used directly.
+template <typename... Args>
+void _takeAndLogError(const char *file, const char *func, llvm::Error E,
+ Log *log, Args &&... args) {
+ if (E && log)
+ log->FormatError(std::move(E), file, func, std::forward<Args>(args)...);
+ else
+ llvm::consumeError(std::move(E));
+}
+template <typename... Args>
+void _takeAndLogError(const char *file, const char *func, llvm::Error E,
+ uint32_t log_categories, Args &&... args) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(log_categories));
+ _takeAndLogError(file, func, std::move(E), log, std::forward<Args>(args)...);
+}
+
} // namespace lldb_private
#define LLDB_LOG(log, ...) \
@@ -230,15 +247,26 @@
// Write message to log, if error is set. In the log message refer to the error
// with {0}. Error is cleared regardless of whether logging is enabled.
+//
+// For convenience, log may either be a Log instance or an unsigned value
+// specifying log categories.
#define LLDB_LOG_ERROR(log, error, ...) \
+ ::lldb_private::_takeAndLogError(__FILE__, __func__, error, log, __VA_ARGS__)
+
+// If item contains an unexpected result, log the error and return the given
+// return value.
+//
+// For convenience, log may either be a Log instance or an unsigned value
+// specifying log categories.
+//
+// To set a breakpoint on the failure case, break on the constructor of the
+// appropriate ErrorInfo. Set a breakpoint on ErrorInfoBase to catch all errors.
+#define RETURN_IF_UNEXPECTED(item, ret_val, log) \
do { \
- ::lldb_private::Log *log_private = (log); \
- ::llvm::Error error_private = (error); \
- if (log_private && error_private) { \
- log_private->FormatError(::std::move(error_private), __FILE__, __func__, \
- __VA_ARGS__); \
- } else \
- ::llvm::consumeError(::std::move(error_private)); \
+ if (::llvm::Error error_private = (item).takeError()) { \
+ LLDB_LOG_ERROR(log, std::move(error_private), "{0}"); \
+ return ret_val; \
+ } \
} while (0)
#endif // LLDB_UTILITY_LOG_H
Index: include/lldb/Symbol/CompilerType.h
===================================================================
--- include/lldb/Symbol/CompilerType.h
+++ include/lldb/Symbol/CompilerType.h
@@ -21,6 +21,7 @@
#include "lldb/Core/ClangForward.h"
#include "lldb/lldb-private.h"
#include "llvm/ADT/APSInt.h"
+#include "llvm/Support/Error.h"
namespace lldb_private {
@@ -441,6 +442,23 @@
CompilerType type;
};
+/// A force-checked error used to describe type construction failures.
+class InvalidTypeError : public llvm::ErrorInfo<InvalidTypeError> {
+public:
+ static char ID;
+ std::string Reason;
+
+ InvalidTypeError(llvm::StringRef Reason) : Reason(Reason) {}
+
+ void log(llvm::raw_ostream &OS) const override {
+ OS << "Invalid type: " << Reason;
+ }
+
+ std::error_code convertToErrorCode() const override {
+ return std::make_error_code(std::errc::invalid_argument);
+ }
+};
+
} // namespace lldb_private
#endif // liblldb_CompilerType_h_
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
