Skip to content

[lldb] Inline expression evaluator error visualization #106470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lldb/include/lldb/API/SBDebugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ class LLDB_API SBDebugger {

bool GetUseColor() const;

bool SetShowInlineDiagnostics(bool);

bool SetUseSourceCache(bool use_source_cache);

bool GetUseSourceCache() const;
Expand Down
4 changes: 4 additions & 0 deletions lldb/include/lldb/Core/Debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>,

const std::string &GetInstanceName() { return m_instance_name; }

bool GetShowInlineDiagnostics() const;

bool SetShowInlineDiagnostics(bool);

bool LoadPlugin(const FileSpec &spec, Status &error);

void RunIOHandlers();
Expand Down
1 change: 1 addition & 0 deletions lldb/include/lldb/Expression/DiagnosticManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ struct DiagnosticDetail {
unsigned line = 0;
uint16_t column = 0;
uint16_t length = 0;
bool hidden = false;
bool in_user_input = false;
};
/// Contains {{}, 1, 3, 3, true} in the example above.
Expand Down
8 changes: 8 additions & 0 deletions lldb/include/lldb/Interpreter/CommandObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,13 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
return false;
}

/// Set the command input as it appeared in the terminal. This
/// is used to have errors refer directly to the original command.
void SetOriginalCommandString(std::string s) { m_original_command = s; }

/// \param offset_in_command is on what column \c args_string
/// appears, if applicable. This enables diagnostics that refer back
/// to the user input.
virtual void Execute(const char *args_string,
CommandReturnObject &result) = 0;

Expand Down Expand Up @@ -404,6 +411,7 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
std::string m_cmd_help_short;
std::string m_cmd_help_long;
std::string m_cmd_syntax;
std::string m_original_command;
Flags m_flags;
std::vector<CommandArgumentEntry> m_arguments;
lldb::CommandOverrideCallback m_deprecated_command_override_callback;
Expand Down
6 changes: 6 additions & 0 deletions lldb/source/API/SBDebugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,12 @@ bool SBDebugger::GetUseColor() const {
return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
}

bool SBDebugger::SetShowInlineDiagnostics(bool value) {
LLDB_INSTRUMENT_VA(this, value);

return (m_opaque_sp ? m_opaque_sp->SetShowInlineDiagnostics(value) : false);
}

bool SBDebugger::SetUseSourceCache(bool value) {
LLDB_INSTRUMENT_VA(this, value);

Expand Down
154 changes: 143 additions & 11 deletions lldb/source/Commands/CommandObjectExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "CommandObjectExpression.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Expression/DiagnosticManager.h"
#include "lldb/Expression/ExpressionVariable.h"
#include "lldb/Expression/REPL.h"
#include "lldb/Expression/UserExpression.h"
Expand Down Expand Up @@ -398,6 +399,122 @@ CanBeUsedForElementCountPrinting(ValueObject &valobj) {
return Status();
}

static llvm::raw_ostream &PrintSeverity(Stream &stream,
lldb::Severity severity) {
llvm::HighlightColor color;
llvm::StringRef text;
switch (severity) {
case eSeverityError:
color = llvm::HighlightColor::Error;
text = "error: ";
break;
case eSeverityWarning:
color = llvm::HighlightColor::Warning;
text = "warning: ";
break;
case eSeverityInfo:
color = llvm::HighlightColor::Remark;
text = "note: ";
break;
}
return llvm::WithColor(stream.AsRawOstream(), color, llvm::ColorMode::Enable)
<< text;
}

namespace lldb_private {
// Public for unittesting.
void RenderDiagnosticDetails(Stream &stream,
std::optional<uint16_t> offset_in_command,
bool show_inline,
llvm::ArrayRef<DiagnosticDetail> details) {
if (details.empty())
return;

if (!offset_in_command) {
for (const DiagnosticDetail &detail : details) {
PrintSeverity(stream, detail.severity);
stream << detail.rendered << '\n';
}
return;
}

// Print a line with caret indicator(s) below the lldb prompt + command.
const size_t padding = *offset_in_command;
stream << std::string(padding, ' ');

size_t offset = 1;
std::vector<DiagnosticDetail> remaining_details, other_details,
hidden_details;
for (const DiagnosticDetail &detail : details) {
if (!show_inline || !detail.source_location) {
other_details.push_back(detail);
continue;
}
if (detail.source_location->hidden) {
hidden_details.push_back(detail);
continue;
}
if (!detail.source_location->in_user_input) {
other_details.push_back(detail);
continue;
}

auto &loc = *detail.source_location;
remaining_details.push_back(detail);
if (offset > loc.column)
continue;
stream << std::string(loc.column - offset, ' ') << '^';
if (loc.length > 1)
stream << std::string(loc.length - 1, '~');
offset = loc.column + 1;
}
stream << '\n';

// Work through each detail in reverse order using the vector/stack.
bool did_print = false;
for (auto detail = remaining_details.rbegin();
detail != remaining_details.rend();
++detail, remaining_details.pop_back()) {
// Get the information to print this detail and remove it from the stack.
// Print all the lines for all the other messages first.
stream << std::string(padding, ' ');
size_t offset = 1;
for (auto &remaining_detail :
llvm::ArrayRef(remaining_details).drop_back(1)) {
uint16_t column = remaining_detail.source_location->column;
stream << std::string(column - offset, ' ') << "│";
offset = column + 1;
}

// Print the line connecting the ^ with the error message.
uint16_t column = detail->source_location->column;
if (offset <= column)
stream << std::string(column - offset, ' ') << "╰─ ";

// Print a colorized string based on the message's severity type.
PrintSeverity(stream, detail->severity);

// Finally, print the message and start a new line.
stream << detail->message << '\n';
did_print = true;
}

// Print the non-located details.
for (const DiagnosticDetail &detail : other_details) {
PrintSeverity(stream, detail.severity);
stream << detail.rendered << '\n';
did_print = true;
}

// Print the hidden details as a last resort.
if (!did_print)
for (const DiagnosticDetail &detail : hidden_details) {
PrintSeverity(stream, detail.severity);
stream << detail.rendered << '\n';
}
}
} // namespace lldb_private

bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
Stream &output_stream,
Stream &error_stream,
Expand Down Expand Up @@ -486,19 +603,34 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,

result.SetStatus(eReturnStatusSuccessFinishResult);
} else {
const char *error_cstr = result_valobj_sp->GetError().AsCString();
if (error_cstr && error_cstr[0]) {
const size_t error_cstr_len = strlen(error_cstr);
const bool ends_with_newline = error_cstr[error_cstr_len - 1] == '\n';
if (strstr(error_cstr, "error:") != error_cstr)
error_stream.PutCString("error: ");
error_stream.Write(error_cstr, error_cstr_len);
if (!ends_with_newline)
error_stream.EOL();
// Retrieve the diagnostics.
std::vector<DiagnosticDetail> details;
llvm::consumeError(llvm::handleErrors(
result_valobj_sp->GetError().ToError(),
[&](ExpressionError &error) { details = error.GetDetails(); }));
// Find the position of the expression in the command.
std::optional<uint16_t> expr_pos;
size_t nchar = m_original_command.find(expr);
if (nchar != std::string::npos)
expr_pos = nchar + GetDebugger().GetPrompt().size();

if (!details.empty()) {
bool show_inline =
GetDebugger().GetShowInlineDiagnostics() && !expr.contains('\n');
RenderDiagnosticDetails(error_stream, expr_pos, show_inline, details);
} else {
error_stream.PutCString("error: unknown error\n");
const char *error_cstr = result_valobj_sp->GetError().AsCString();
llvm::StringRef error(error_cstr);
if (!error.empty()) {
if (!error.starts_with("error:"))
error_stream << "error: ";
error_stream << error;
if (!error.ends_with('\n'))
error_stream.EOL();
} else {
error_stream << "error: unknown error\n";
}
}

result.SetStatus(eReturnStatusFailed);
}
}
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Core/CoreProperties.td
Original file line number Diff line number Diff line change
Expand Up @@ -225,4 +225,8 @@ let Definition = "debugger" in {
DefaultEnumValue<"eDWIMPrintVerbosityNone">,
EnumValues<"OptionEnumValues(g_dwim_print_verbosities)">,
Desc<"The verbosity level used by dwim-print.">;
def ShowInlineDiagnostics: Property<"show-inline-diagnostics", "Boolean">,
Global,
DefaultFalse,
Desc<"Controls whether diagnostics can refer directly to the command input, drawing arrows to it. If false, diagnostics will echo the input.">;
}
13 changes: 12 additions & 1 deletion lldb/source/Core/Debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,18 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity() const {
const uint32_t idx = ePropertyDWIMPrintVerbosity;
return GetPropertyAtIndexAs<lldb::DWIMPrintVerbosity>(
idx, static_cast<lldb::DWIMPrintVerbosity>(
g_debugger_properties[idx].default_uint_value));
g_debugger_properties[idx].default_uint_value != 0));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you intend to change DWIM code?

}

bool Debugger::GetShowInlineDiagnostics() const {
const uint32_t idx = ePropertyShowInlineDiagnostics;
return GetPropertyAtIndexAs<bool>(
idx, g_debugger_properties[idx].default_uint_value);
}

bool Debugger::SetShowInlineDiagnostics(bool b) {
const uint32_t idx = ePropertyShowInlineDiagnostics;
return SetPropertyAtIndex(idx, b);
}

#pragma mark Debugger
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Expression/DiagnosticManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ ExpressionError::ExpressionError(lldb::ExpressionResults result,
: ErrorInfo(std::error_code(result, expression_category())), m_message(msg),
m_details(details) {}

static const char *StringForSeverity(lldb::Severity severity) {
static llvm::StringRef StringForSeverity(lldb::Severity severity) {
switch (severity) {
// this should be exhaustive
case lldb::eSeverityError:
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Interpreter/CommandInterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1887,7 +1887,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
CommandReturnObject &result,
bool force_repeat_command) {
std::string command_string(command_line);
std::string original_command_string(command_line);
std::string original_command_string(command_string);
std::string real_original_command_string(command_string);

Log *log = GetLog(LLDBLog::Commands);
llvm::PrettyStackTraceFormat stack_trace("HandleCommand(command = \"%s\")",
Expand Down Expand Up @@ -2076,6 +2077,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
}

ElapsedTime elapsed(execute_time);
cmd_obj->SetOriginalCommandString(real_original_command_string);
cmd_obj->Execute(remainder.c_str(), result);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,22 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
m_manager = manager;
}

/// Returns the last ClangDiagnostic message that the DiagnosticManager
/// received or a nullptr if the DiagnosticMangager hasn't seen any
/// Clang diagnostics yet.
/// Returns the last error ClangDiagnostic message that the
/// DiagnosticManager received or a nullptr.
ClangDiagnostic *MaybeGetLastClangDiag() const {
if (m_manager->Diagnostics().empty())
return nullptr;
lldb_private::Diagnostic *diag = m_manager->Diagnostics().back().get();
ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag);
return clang_diag;
auto &diags = m_manager->Diagnostics();
for (auto it = diags.rbegin(); it != diags.rend(); it++) {
lldb_private::Diagnostic *diag = it->get();
if (ClangDiagnostic *clang_diag = dyn_cast<ClangDiagnostic>(diag)) {
if (clang_diag->GetSeverity() == lldb::eSeverityWarning)
return nullptr;
if (clang_diag->GetSeverity() == lldb::eSeverityError)
return clang_diag;
}
}
return nullptr;
}

void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
Expand Down Expand Up @@ -214,8 +221,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
m_passthrough->HandleDiagnostic(DiagLevel, Info);

DiagnosticDetail detail;
bool make_new_diagnostic = true;

switch (DiagLevel) {
case DiagnosticsEngine::Level::Fatal:
case DiagnosticsEngine::Level::Error:
Expand All @@ -229,9 +234,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
detail.severity = lldb::eSeverityInfo;
break;
case DiagnosticsEngine::Level::Note:
m_manager->AppendMessageToDiagnostic(m_output);
make_new_diagnostic = false;

// 'note:' diagnostics for errors and warnings can also contain Fix-Its.
// We add these Fix-Its to the last error diagnostic to make sure
// that we later have all Fix-Its related to an 'error' diagnostic when
Expand All @@ -249,7 +251,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
AddAllFixIts(clang_diag, Info);
break;
}
if (make_new_diagnostic) {
// ClangDiagnostic messages are expected to have no whitespace/newlines
// around them.
std::string stripped_output =
Expand All @@ -269,6 +270,7 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
loc.line = fsloc.getSpellingLineNumber();
loc.column = fsloc.getSpellingColumnNumber();
loc.in_user_input = filename == m_filename;
loc.hidden = filename.starts_with("<lldb wrapper ");

// Find the range of the primary location.
for (const auto &range : Info.getRanges()) {
Expand Down Expand Up @@ -298,7 +300,6 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
AddAllFixIts(new_diagnostic.get(), Info);

m_manager->AddDiagnostic(std::move(new_diagnostic));
}
}

void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class CommandObjectThreadTraceExportCTF : public CommandObjectParsed {
Options *GetOptions() override { return &m_options; }

protected:
void DoExecute(Args &command, CommandReturnObject &result) override;
void DoExecute(Args &args, CommandReturnObject &result) override;

CommandOptions m_options;
};
Expand Down
Loading
Loading