diff --git a/lldb/include/lldb/Expression/DiagnosticManager.h b/lldb/include/lldb/Expression/DiagnosticManager.h index 06bf1d115f154..b4c888066943d 100644 --- a/lldb/include/lldb/Expression/DiagnosticManager.h +++ b/lldb/include/lldb/Expression/DiagnosticManager.h @@ -10,6 +10,7 @@ #define LLDB_EXPRESSION_DIAGNOSTICMANAGER_H #include "lldb/lldb-defines.h" +#include "lldb/lldb-private-enumerations.h" #include "lldb/lldb-types.h" #include "llvm/ADT/STLExtras.h" @@ -20,20 +21,6 @@ namespace lldb_private { -enum DiagnosticOrigin { - eDiagnosticOriginUnknown = 0, - eDiagnosticOriginLLDB, - eDiagnosticOriginClang, - eDiagnosticOriginSwift, - eDiagnosticOriginLLVM -}; - -enum DiagnosticSeverity { - eDiagnosticSeverityError, - eDiagnosticSeverityWarning, - eDiagnosticSeverityRemark -}; - const uint32_t LLDB_INVALID_COMPILER_ID = UINT32_MAX; class Diagnostic { diff --git a/lldb/include/lldb/Interpreter/CommandReturnObject.h b/lldb/include/lldb/Interpreter/CommandReturnObject.h index 8c4dcb54d708f..3efbde9b31474 100644 --- a/lldb/include/lldb/Interpreter/CommandReturnObject.h +++ b/lldb/include/lldb/Interpreter/CommandReturnObject.h @@ -162,6 +162,7 @@ class CommandReturnObject { StreamTee m_err_stream; lldb::ReturnStatus m_status = lldb::eReturnStatusStarted; + Status m_error_status; bool m_did_change_process_state = false; bool m_suppress_immediate_output = false; diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h index fa5768141fa45..17e3257e435a4 100644 --- a/lldb/include/lldb/Utility/Status.h +++ b/lldb/include/lldb/Utility/Status.h @@ -9,6 +9,7 @@ #ifndef LLDB_UTILITY_STATUS_H #define LLDB_UTILITY_STATUS_H +#include "lldb/Expression/DiagnosticManager.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" #include "llvm/ADT/StringRef.h" @@ -114,7 +115,7 @@ class Status { /// The error type enumeration value. lldb::ErrorType GetType() const; - void SetExpressionError(lldb::ExpressionResults, const char *mssg); + void SetExpressionError(lldb::ExpressionResults, const char *mssg = nullptr); int SetExpressionErrorWithFormat(lldb::ExpressionResults, const char *format, ...) __attribute__((format(printf, 3, 4))); @@ -180,12 +181,20 @@ class Status { /// success (non-erro), \b false otherwise. bool Success() const; + struct Detail; + void AddDetail(Status::Detail detail); + void SetErrorDetails(DiagnosticManager &diagnostic_manager); + std::vector GetDetails() const; + protected: /// Member variables ValueType m_code = 0; ///< Status code as an integer value. lldb::ErrorType m_type = lldb::eErrorTypeInvalid; ///< The type of the above error code. mutable std::string m_string; ///< A string representation of the error code. + std::vector m_status_details; + mutable std::string m_string_with_details; + private: explicit Status(const llvm::formatv_object_base &payload) { SetErrorToGenericError(); @@ -193,6 +202,41 @@ class Status { } }; +struct Status::Detail { +private: + std::vector m_message_lines; + DiagnosticSeverity m_message_type; + DiagnosticOrigin m_message_origin; + + mutable std::optional m_message; + + static std::string StringForSeverity(DiagnosticSeverity severity); + + std::vector + GetMessageLinesFromDiagnostic(Diagnostic *diagnostic); + +public: + Detail(const std::unique_ptr &diagnostic) + : m_message_lines(GetMessageLinesFromDiagnostic(diagnostic.get())), + m_message_type(diagnostic->GetSeverity()), + m_message_origin(diagnostic->getKind()) {} + + Detail(const Detail &other) + : m_message_lines(other.m_message_lines), + m_message_type(other.m_message_type), + m_message_origin(other.m_message_origin) {} + + Detail &operator=(const Detail &rhs) { + m_message_lines = rhs.m_message_lines; + m_message_type = rhs.m_message_type; + m_message_origin = rhs.m_message_origin; + return *this; + } + + std::string GetMessage() const; + std::vector GetMessageLines() const; + DiagnosticSeverity GetType() const { return m_message_type; } +}; } // namespace lldb_private namespace llvm { diff --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h index 5f1597200a83e..004d79112e1ec 100644 --- a/lldb/include/lldb/lldb-private-enumerations.h +++ b/lldb/include/lldb/lldb-private-enumerations.h @@ -234,6 +234,20 @@ enum LoadDependentFiles { eLoadDependentsNo, }; +enum DiagnosticOrigin { + eDiagnosticOriginUnknown = 0, + eDiagnosticOriginLLDB, + eDiagnosticOriginClang, + eDiagnosticOriginSwift, + eDiagnosticOriginLLVM +}; + +enum DiagnosticSeverity { + eDiagnosticSeverityError, + eDiagnosticSeverityWarning, + eDiagnosticSeverityRemark +}; + inline std::string GetStatDescription(lldb_private::StatisticKind K) { switch (K) { case StatisticKind::ExpressionSuccessful: diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp index 98de059c2e296..9a8d7219c0604 100644 --- a/lldb/source/Breakpoint/BreakpointLocation.cpp +++ b/lldb/source/Breakpoint/BreakpointLocation.cpp @@ -247,8 +247,6 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx, error.Clear(); - DiagnosticManager diagnostics; - if (condition_hash != m_condition_hash || !m_user_expression_sp || !m_user_expression_sp->IsParseCacheable() || !m_user_expression_sp->MatchesContext(exe_ctx)) { @@ -269,12 +267,14 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx, return true; } - if (!m_user_expression_sp->Parse(diagnostics, exe_ctx, - eExecutionPolicyOnlyWhenNeeded, true, - false)) { - error.SetErrorStringWithFormat( - "Couldn't parse conditional expression:\n%s", - diagnostics.GetString().c_str()); + DiagnosticManager diagnostic_manager; + + bool success = m_user_expression_sp->Parse(diagnostic_manager, exe_ctx, + eExecutionPolicyOnlyWhenNeeded, + true, false); + if (!success) { + error.SetErrorString("Couldn't parse conditional expression:"); + error.SetErrorDetails(diagnostic_manager); m_user_expression_sp.reset(); return true; } @@ -296,12 +296,13 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx, Status expr_error; - diagnostics.Clear(); + DiagnosticManager diagnostic_manager; ExpressionVariableSP result_variable_sp; - ExpressionResults result_code = m_user_expression_sp->Execute( - diagnostics, exe_ctx, options, m_user_expression_sp, result_variable_sp); + ExpressionResults result_code = + m_user_expression_sp->Execute(diagnostic_manager, exe_ctx, options, + m_user_expression_sp, result_variable_sp); bool ret; @@ -331,8 +332,8 @@ bool BreakpointLocation::ConditionSaysStop(ExecutionContext &exe_ctx, } } else { ret = false; - error.SetErrorStringWithFormat("Couldn't execute expression:\n%s", - diagnostics.GetString().c_str()); + error.SetErrorStringWithFormat("Couldn't execute expression:"); + error.SetErrorDetails(diagnostic_manager); } return ret; diff --git a/lldb/source/Expression/UserExpression.cpp b/lldb/source/Expression/UserExpression.cpp index c181712a2f0b2..03d3c93d5b79a 100644 --- a/lldb/source/Expression/UserExpression.cpp +++ b/lldb/source/Expression/UserExpression.cpp @@ -325,17 +325,16 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx, if (!parse_success) { std::string msg; - { - llvm::raw_string_ostream os(msg); - if (!diagnostic_manager.Diagnostics().empty()) - os << diagnostic_manager.GetString(); - else - os << "expression failed to parse (no further compiler diagnostics)"; - if (target->GetEnableNotifyAboutFixIts() && fixed_expression && - !fixed_expression->empty()) - os << "\nfixed expression suggested:\n " << *fixed_expression; + if (diagnostic_manager.Diagnostics().empty()) + msg += "expression failed to parse (no further compiler diagnostics)"; + + if (target->GetEnableNotifyAboutFixIts() && fixed_expression && + !fixed_expression->empty()) { + msg += "\nfixed expression suggested:\n "; + msg += *fixed_expression; } error.SetExpressionError(execution_results, msg.c_str()); + error.SetErrorDetails(diagnostic_manager); } } @@ -378,9 +377,10 @@ UserExpression::Evaluate(ExecutionContext &exe_ctx, if (!diagnostic_manager.Diagnostics().size()) error.SetExpressionError( execution_results, "expression failed to execute, unknown error"); - else - error.SetExpressionError(execution_results, - diagnostic_manager.GetString().c_str()); + else { + error.SetExpressionError(execution_results); + error.SetErrorDetails(diagnostic_manager); + } } else { if (expr_result) { result_valobj_sp = expr_result->GetValueObject(); diff --git a/lldb/source/Expression/UtilityFunction.cpp b/lldb/source/Expression/UtilityFunction.cpp index d7a3c9d41d048..9efebec05f76c 100644 --- a/lldb/source/Expression/UtilityFunction.cpp +++ b/lldb/source/Expression/UtilityFunction.cpp @@ -87,25 +87,25 @@ FunctionCaller *UtilityFunction::MakeFunctionCaller( return nullptr; } if (m_caller_up) { - DiagnosticManager diagnostics; + DiagnosticManager diagnostic_manager; unsigned num_errors = - m_caller_up->CompileFunction(thread_to_use_sp, diagnostics); + m_caller_up->CompileFunction(thread_to_use_sp, diagnostic_manager); if (num_errors) { - error.SetErrorStringWithFormat( - "Error compiling %s caller function: \"%s\".", - m_function_name.c_str(), diagnostics.GetString().c_str()); + error.SetErrorStringWithFormat("Error compiling %s caller function:", + m_function_name.c_str()); + error.SetErrorDetails(diagnostic_manager); m_caller_up.reset(); return nullptr; } - diagnostics.Clear(); + diagnostic_manager.Clear(); ExecutionContext exe_ctx(process_sp); - if (!m_caller_up->WriteFunctionWrapper(exe_ctx, diagnostics)) { - error.SetErrorStringWithFormat( - "Error inserting caller function for %s: \"%s\".", - m_function_name.c_str(), diagnostics.GetString().c_str()); + if (!m_caller_up->WriteFunctionWrapper(exe_ctx, diagnostic_manager)) { + error.SetErrorStringWithFormat("Error inserting caller function for %s:", + m_function_name.c_str()); + error.SetErrorDetails(diagnostic_manager); m_caller_up.reset(); return nullptr; } diff --git a/lldb/source/Interpreter/CommandReturnObject.cpp b/lldb/source/Interpreter/CommandReturnObject.cpp index 0bc58124f3941..6a640cd365f45 100644 --- a/lldb/source/Interpreter/CommandReturnObject.cpp +++ b/lldb/source/Interpreter/CommandReturnObject.cpp @@ -109,8 +109,15 @@ void CommandReturnObject::AppendError(llvm::StringRef in_string) { void CommandReturnObject::SetError(const Status &error, const char *fallback_error_cstr) { - if (error.Fail()) - AppendError(error.AsCString(fallback_error_cstr)); + m_error_status = error; + if (m_error_status.Fail()) { + std::vector details = m_error_status.GetDetails(); + if (!details.empty()) { + for (Status::Detail detail : details) + AppendError(detail.GetMessage()); + } else + AppendError(error.AsCString(fallback_error_cstr)); + } } void CommandReturnObject::SetError(llvm::Error error) { diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp index 574d661e2a215..6f6180ae0952e 100644 --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -1424,7 +1424,7 @@ lldb_private::Status ClangExpressionParser::PrepareForExecution( 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.SetErrorDetails(install_diags); err.SetErrorString(ErrMsg); return err; } @@ -1505,8 +1505,8 @@ lldb_private::Status ClangExpressionParser::RunStaticInitializers( exe_ctx, call_static_initializer, options, execution_errors); if (results != lldb::eExpressionCompleted) { - err.SetErrorStringWithFormat("couldn't run static initializer: %s", - execution_errors.GetString().c_str()); + err.SetErrorString("couldn't run static initializer: "); + err.SetErrorDetails(execution_errors); return err; } } diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index b4f1b76c39dbe..f285d7c0abf97 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -851,9 +851,8 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process, func_args_addr, arguments, diagnostics)) { - error.SetErrorStringWithFormat( - "dlopen error: could not write function arguments: %s", - diagnostics.GetString().c_str()); + error.SetErrorString("dlopen error: could not write function arguments: "); + error.SetErrorDetails(diagnostics); return LLDB_INVALID_IMAGE_TOKEN; } @@ -893,9 +892,9 @@ uint32_t PlatformPOSIX::DoLoadImage(lldb_private::Process *process, ExpressionResults results = do_dlopen_function->ExecuteFunction( exe_ctx, &func_args_addr, options, diagnostics, return_value); if (results != eExpressionCompleted) { - error.SetErrorStringWithFormat( - "dlopen error: failed executing dlopen wrapper function: %s", - diagnostics.GetString().c_str()); + error.SetErrorString( + "dlopen error: failed executing dlopen wrapper function: "); + error.SetErrorDetails(diagnostics); return LLDB_INVALID_IMAGE_TOKEN; } diff --git a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp index 88d543289a847..aded7fc755687 100644 --- a/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp +++ b/lldb/source/Plugins/Platform/Windows/PlatformWindows.cpp @@ -331,8 +331,9 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, diagnostics.Clear(); if (!invocation->WriteFunctionArguments(context, injected_parameters, parameters, diagnostics)) { - error.SetErrorStringWithFormat("LoadLibrary error: unable to write function parameters: %s", - diagnostics.GetString().c_str()); + error.SetErrorString( + "LoadLibrary error: unable to write function parameters: "); + error.SetErrorDetails(diagnostics); return LLDB_INVALID_IMAGE_TOKEN; } @@ -372,8 +373,9 @@ uint32_t PlatformWindows::DoLoadImage(Process *process, invocation->ExecuteFunction(context, &injected_parameters, options, diagnostics, value); if (result != eExpressionCompleted) { - error.SetErrorStringWithFormat("LoadLibrary error: failed to execute LoadLibrary helper: %s", - diagnostics.GetString().c_str()); + error.SetErrorString( + "LoadLibrary error: failed to execute LoadLibrary helper: "); + error.SetErrorDetails(diagnostics); return LLDB_INVALID_IMAGE_TOKEN; } diff --git a/lldb/source/Utility/Status.cpp b/lldb/source/Utility/Status.cpp index 3bd00bb20da25..9575c1d614635 100644 --- a/lldb/source/Utility/Status.cpp +++ b/lldb/source/Utility/Status.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -160,6 +161,18 @@ const char *Status::AsCString(const char *default_error_str) const { else return nullptr; // User wanted a nullptr string back... } + + if (!m_status_details.empty()) { + if (!m_string_with_details.size()) + return m_string_with_details.c_str(); + + m_string_with_details = m_string; + for (Status::Detail detail : m_status_details) + m_string_with_details += detail.GetMessage(); + + return m_string_with_details.c_str(); + } + return m_string.c_str(); } @@ -168,6 +181,8 @@ void Status::Clear() { m_code = 0; m_type = eErrorTypeInvalid; m_string.clear(); + m_string_with_details.clear(); + m_status_details.clear(); } // Access the error value. @@ -181,10 +196,14 @@ ErrorType Status::GetType() const { return m_type; } bool Status::Fail() const { return m_code != 0; } void Status::SetExpressionError(lldb::ExpressionResults result, - const char *mssg) { + const char *message) { m_code = result; m_type = eErrorTypeExpression; - m_string = mssg; + + if (message) + m_string = message; + else + m_string.clear(); } int Status::SetExpressionErrorWithFormat(lldb::ExpressionResults result, @@ -274,6 +293,18 @@ int Status::SetErrorStringWithVarArg(const char *format, va_list args) { return 0; } +void Status::SetErrorDetails(DiagnosticManager &diagnostic_manager) { + m_status_details.clear(); + m_string_with_details.clear(); + + const DiagnosticList &diagnostics = diagnostic_manager.Diagnostics(); + if (diagnostics.empty()) + return; + + for (auto &diagnostic : diagnostics) + m_status_details.push_back(Status::Detail(diagnostic)); +} + // Returns true if the error code in this object is considered a successful // return value. bool Status::Success() const { return m_code == 0; } @@ -284,3 +315,71 @@ void llvm::format_provider::format( llvm::format_provider::format(error.AsCString(), OS, Options); } + +void Status::AddDetail(Status::Detail detail) { + m_status_details.push_back(detail); + m_string_with_details.clear(); +} + +std::vector Status::GetDetails() const { + return m_status_details; +} + +std::string Status::Detail::StringForSeverity(DiagnosticSeverity severity) { + switch (severity) { + case lldb_private::eDiagnosticSeverityError: + return std::string("error: "); + case lldb_private::eDiagnosticSeverityWarning: + return std::string("warning: "); + case lldb_private::eDiagnosticSeverityRemark: + return std::string("note: "); + } +} + +std::vector +Status::Detail::GetMessageLinesFromDiagnostic(Diagnostic *diagnostic) { + const char newline = '\n'; + std::vector message_lines; + + std::string severity = StringForSeverity(m_message_type); + std::string message = severity; + + std::stringstream splitter(std::string(diagnostic->GetMessage())); + std::string split_line; + + while (getline(splitter, split_line, newline)) { + size_t position = split_line.find(severity); + if (position != std::string::npos) + split_line.erase(position, severity.length()); + + // Check for clang-style diagnostic message. + std::string separator(" | "); + position = split_line.find(separator); + if (split_line.find(separator) != std::string::npos) { + auto end = split_line.begin() + position + separator.size(); + split_line.erase(split_line.begin(), end); + } + + message_lines.push_back(split_line); + } + + return message_lines; +} + +std::vector Status::Detail::GetMessageLines() const { + return m_message_lines; +} + +std::string Status::Detail::GetMessage() const { + if (m_message) + return m_message.value(); + + std::string severity = StringForSeverity(m_message_type); + std::string message = severity; + for (auto line : m_message_lines) + message += line + "\n"; + + message += "\n"; + m_message = message; + return message; +}