sgraenitz created this revision.
sgraenitz added reviewers: teemperor, bulbazord, JDevlieghere.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
sgraenitz requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: LLDB.
I came accross this, because a lot of regression tests were saying:
(lldb) p argc
error: expression failed to parse:
error: couldn't install checkers, unknown error
With this change, error messages provide more detail:
(lldb) p argc
error: expression failed to parse:
error: couldn't install checkers:
error: Couldn't lookup symbols:
__objc_load
I didn't find a case where `Diagnostics()` is not empty. Also it looks like
this isn't covered in any test (yet).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D146541
Files:
lldb/include/lldb/Expression/DynamicCheckerFunctions.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
Index: lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
+++ lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.h
@@ -46,10 +46,10 @@
/// The execution context to install the functions into.
///
/// \return
- /// True on success; false on failure, or if the functions have
- /// already been installed.
- bool Install(DiagnosticManager &diagnostic_manager,
- ExecutionContext &exe_ctx) override;
+ /// Either llvm::ErrorSuccess or Error with llvm::ErrorInfo
+ ///
+ llvm::Error Install(DiagnosticManager &diagnostic_manager,
+ ExecutionContext &exe_ctx) override;
bool DoCheckersExplainStop(lldb::addr_t addr, Stream &message) override;
Index: lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/IRDynamicChecks.cpp
@@ -47,33 +47,30 @@
ClangDynamicCheckerFunctions::~ClangDynamicCheckerFunctions() = default;
-bool ClangDynamicCheckerFunctions::Install(
+llvm::Error ClangDynamicCheckerFunctions::Install(
DiagnosticManager &diagnostic_manager, ExecutionContext &exe_ctx) {
- auto utility_fn_or_error = exe_ctx.GetTargetRef().CreateUtilityFunction(
- g_valid_pointer_check_text, VALID_POINTER_CHECK_NAME,
- lldb::eLanguageTypeC, exe_ctx);
- if (!utility_fn_or_error) {
- llvm::consumeError(utility_fn_or_error.takeError());
- return false;
- }
- m_valid_pointer_check = std::move(*utility_fn_or_error);
+ Expected<std::unique_ptr<UtilityFunction>> utility_fn =
+ exe_ctx.GetTargetRef().CreateUtilityFunction(
+ g_valid_pointer_check_text, VALID_POINTER_CHECK_NAME,
+ lldb::eLanguageTypeC, exe_ctx);
+ if (!utility_fn)
+ return utility_fn.takeError();
+ m_valid_pointer_check = std::move(*utility_fn);
if (Process *process = exe_ctx.GetProcessPtr()) {
ObjCLanguageRuntime *objc_language_runtime =
ObjCLanguageRuntime::Get(*process);
if (objc_language_runtime) {
- auto utility_fn_or_error = objc_language_runtime->CreateObjectChecker(
- VALID_OBJC_OBJECT_CHECK_NAME, exe_ctx);
- if (!utility_fn_or_error) {
- llvm::consumeError(utility_fn_or_error.takeError());
- return false;
- }
- m_objc_object_check = std::move(*utility_fn_or_error);
+ Expected<std::unique_ptr<UtilityFunction>> checker_fn =
+ objc_language_runtime->CreateObjectChecker(VALID_OBJC_OBJECT_CHECK_NAME, exe_ctx);
+ if (!checker_fn)
+ return checker_fn.takeError();
+ m_objc_object_check = std::move(*checker_fn);
}
}
- return true;
+ return Error::success();
}
bool ClangDynamicCheckerFunctions::DoCheckersExplainStop(lldb::addr_t addr,
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -1410,14 +1410,12 @@
ClangDynamicCheckerFunctions *dynamic_checkers =
new ClangDynamicCheckerFunctions();
- DiagnosticManager install_diagnostics;
-
- if (!dynamic_checkers->Install(install_diagnostics, exe_ctx)) {
- if (install_diagnostics.Diagnostics().size())
- err.SetErrorString(install_diagnostics.GetString().c_str());
- else
- err.SetErrorString("couldn't install checkers, unknown error");
-
+ DiagnosticManager install_diags;
+ if (Error Err = dynamic_checkers->Install(install_diags, exe_ctx)) {
+ std::string ErrMsg = "couldn't install checkers: " + toString(std::move(Err));
+ if (install_diags.Diagnostics().size())
+ ErrMsg = ErrMsg + "\n" + install_diags.GetString().c_str();
+ err.SetErrorString(ErrMsg);
return err;
}
Index: lldb/include/lldb/Expression/DynamicCheckerFunctions.h
===================================================================
--- lldb/include/lldb/Expression/DynamicCheckerFunctions.h
+++ lldb/include/lldb/Expression/DynamicCheckerFunctions.h
@@ -11,6 +11,8 @@
#include "lldb/lldb-types.h"
+#include "llvm/Support/Error.h"
+
namespace lldb_private {
class DiagnosticManager;
@@ -46,10 +48,10 @@
/// The execution context to install the functions into.
///
/// \return
- /// True on success; false on failure, or if the functions have
- /// already been installed.
- virtual bool Install(DiagnosticManager &diagnostic_manager,
- ExecutionContext &exe_ctx) = 0;
+ /// Either llvm::ErrorSuccess or Error with llvm::ErrorInfo
+ ///
+ virtual llvm::Error Install(DiagnosticManager &diagnostic_manager,
+ ExecutionContext &exe_ctx) = 0;
virtual bool DoCheckersExplainStop(lldb::addr_t addr, Stream &message) = 0;
DynamicCheckerFunctionsKind GetKind() const { return m_kind; }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
