[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error
vsk abandoned this revision. vsk added a subscriber: shafik. vsk added a comment. I don't think this got a lot of buy-in. Let's revisit this if the need to revamp CompilerType becomes more pressing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43912/new/ https://reviews.llvm.org/D43912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error
labath added a comment. Thanks. The logging stuff looks good to me. I know very little about the TypeFromUser class and friends, so someone else should probably ack that. https://reviews.llvm.org/D43912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error
vsk updated this revision to Diff 136656.
vsk added a comment.
- Clarify that the log_categories macro argument is for logging to the "lldb"
channel.
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 , ClangASTContext ,
- TypeFromParser parser_type);
+ llvm::Expected DeportType(ClangASTContext ,
+ ClangASTContext ,
+ 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 ,
-ClangASTContext ,
-TypeFromParser parser_type) {
+llvm::Expected
+ClangExpressionDeclMap::DeportType(ClangASTContext ,
+ ClangASTContext ,
+ TypeFromParser parser_type) {
assert ( == m_target->GetScratchClangASTContext());
assert ((TypeSystem*) == 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(), );
- } else {
-lldbassert(0 && "No mechanism for deporting a type!");
-return TypeFromUser();
}
+
+ lldbassert(0 && "No mechanism for deporting a type!");
+ return llvm::make_error("Could not deport type");
}
bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
@@ -328,8 +329,12 @@
if (target == nullptr)
return false;
-TypeFromUser user_type =
+llvm::Expected user_type_or_err =
DeportType(*target->GetScratchClangASTContext(), *ast, parser_type);
+RETURN_IF_UNEXPECTED(user_type_or_err, false, LIBLLDB_LOG_EXPRESSIONS);
+TypeFromUser _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 user_type_or_err =
+ DeportType(*context, *ast, parser_type);
+ RETURN_IF_UNEXPECTED(user_type_or_err, false, log);
+ TypeFromUser _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
+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)...);
+ else
+llvm::consumeError(std::move(E));
+}
+template
+void
[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error
labath added inline comments. Comment at: include/lldb/Utility/Log.h:259-260 +// +// For convenience, log may either be a Log instance or an unsigned value +// specifying log categories. +// The thing to remember here is that we have multiple log channels. So the unsigned value must be a category from the "lldb" channel. That is by far the most prevalent channel, so it's probably fine that we don't have a shorthand way of specifying the other channels, but it should be called out explicitly. https://reviews.llvm.org/D43912 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidTypeError, a force-checked recoverable error
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 , ClangASTContext ,
- TypeFromParser parser_type);
+ llvm::Expected DeportType(ClangASTContext ,
+ ClangASTContext ,
+ 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 ,
-ClangASTContext ,
-TypeFromParser parser_type) {
+llvm::Expected
+ClangExpressionDeclMap::DeportType(ClangASTContext ,
+ ClangASTContext ,
+ TypeFromParser parser_type) {
assert ( == m_target->GetScratchClangASTContext());
assert ((TypeSystem*) == 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(), );
- } else {
-lldbassert(0 && "No mechanism for deporting a type!");
-return TypeFromUser();
}
+
+ lldbassert(0 && "No mechanism for deporting a type!");
+ return llvm::make_error("Could not deport type");
}
bool ClangExpressionDeclMap::AddPersistentVariable(const NamedDecl *decl,
@@ -328,8 +329,12 @@
if (target == nullptr)
return false;
-TypeFromUser user_type =
+llvm::Expected user_type_or_err =
DeportType(*target->GetScratchClangASTContext(), *ast, parser_type);
+RETURN_IF_UNEXPECTED(user_type_or_err, false, LIBLLDB_LOG_EXPRESSIONS);
+TypeFromUser _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 user_type_or_err =
+ DeportType(*context, *ast, parser_type);
+ RETURN_IF_UNEXPECTED(user_type_or_err, false, log);
+ TypeFromUser _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
+++
