Skip to content

Commit d33fa70

Browse files
committed
[lldb] Inline expression evaluator error visualization (llvm#106470)
This patch is a reworking of Pete Lawrence's (@PortalPete) proposal for better expression evaluator error messages: llvm#80938 Before: ``` $ lldb -o "expr a+b" (lldb) expr a+b error: <user expression 0>:1:1: use of undeclared identifier 'a' a+b ^ error: <user expression 0>:1:3: use of undeclared identifier 'b' a+b ^ ``` After: ``` (lldb) expr a+b ^ ^ │ ╰─ error: use of undeclared identifier 'b' ╰─ error: use of undeclared identifier 'a' ``` This eliminates the confusing `<user expression 0>:1:3` source location and avoids echoing the expression to the console again, which results in a cleaner presentation that makes it easier to grasp what's going on. You can't see it here, bug the word "error" is now also in color, if so desired. Depends on llvm#106442.
1 parent df4d7d3 commit d33fa70

File tree

25 files changed

+327
-53
lines changed

25 files changed

+327
-53
lines changed

lldb/include/lldb/API/SBDebugger.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,8 @@ class LLDB_API SBDebugger {
304304

305305
bool GetUseColor() const;
306306

307+
bool SetShowInlineDiagnostics(bool);
308+
307309
bool SetUseSourceCache(bool use_source_cache);
308310

309311
bool GetUseSourceCache() const;

lldb/include/lldb/Core/Debugger.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,10 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
364364

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

367+
bool GetShowInlineDiagnostics() const;
368+
369+
bool SetShowInlineDiagnostics(bool);
370+
367371
bool LoadPlugin(const FileSpec &spec, Status &error);
368372

369373
void RunIOHandlers();

lldb/include/lldb/Expression/DiagnosticManager.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ struct DiagnosticDetail {
3939
unsigned line = 0;
4040
uint16_t column = 0;
4141
uint16_t length = 0;
42+
bool hidden = false;
4243
bool in_user_input = false;
4344
};
4445
/// Contains {{}, 1, 3, 3, true} in the example above.

lldb/include/lldb/Interpreter/CommandObject.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,13 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
340340
return false;
341341
}
342342

343+
/// Set the command input as it appeared in the terminal. This
344+
/// is used to have errors refer directly to the original command.
345+
void SetOriginalCommandString(std::string s) { m_original_command = s; }
346+
347+
/// \param offset_in_command is on what column \c args_string
348+
/// appears, if applicable. This enables diagnostics that refer back
349+
/// to the user input.
343350
virtual void Execute(const char *args_string,
344351
CommandReturnObject &result) = 0;
345352

@@ -404,6 +411,7 @@ class CommandObject : public std::enable_shared_from_this<CommandObject> {
404411
std::string m_cmd_help_short;
405412
std::string m_cmd_help_long;
406413
std::string m_cmd_syntax;
414+
std::string m_original_command;
407415
Flags m_flags;
408416
std::vector<CommandArgumentEntry> m_arguments;
409417
lldb::CommandOverrideCallback m_deprecated_command_override_callback;

lldb/source/API/SBDebugger.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1483,6 +1483,12 @@ bool SBDebugger::GetUseColor() const {
14831483
return (m_opaque_sp ? m_opaque_sp->GetUseColor() : false);
14841484
}
14851485

1486+
bool SBDebugger::SetShowInlineDiagnostics(bool value) {
1487+
LLDB_INSTRUMENT_VA(this, value);
1488+
1489+
return (m_opaque_sp ? m_opaque_sp->SetShowInlineDiagnostics(value) : false);
1490+
}
1491+
14861492
bool SBDebugger::SetUseSourceCache(bool value) {
14871493
LLDB_INSTRUMENT_VA(this, value);
14881494

lldb/source/Commands/CommandObjectExpression.cpp

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
//
77
//===----------------------------------------------------------------------===//
88

9-
#include "llvm/ADT/StringRef.h"
10-
119
#include "CommandObjectExpression.h"
10+
#include "DiagnosticRendering.h"
1211
#include "lldb/Core/Debugger.h"
12+
#include "lldb/Expression/DiagnosticManager.h"
1313
#include "lldb/Expression/ExpressionVariable.h"
1414
#include "lldb/Expression/REPL.h"
1515
#include "lldb/Expression/UserExpression.h"
@@ -486,19 +486,34 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
486486

487487
result.SetStatus(eReturnStatusSuccessFinishResult);
488488
} else {
489-
const char *error_cstr = result_valobj_sp->GetError().AsCString();
490-
if (error_cstr && error_cstr[0]) {
491-
const size_t error_cstr_len = strlen(error_cstr);
492-
const bool ends_with_newline = error_cstr[error_cstr_len - 1] == '\n';
493-
if (strstr(error_cstr, "error:") != error_cstr)
494-
error_stream.PutCString("error: ");
495-
error_stream.Write(error_cstr, error_cstr_len);
496-
if (!ends_with_newline)
497-
error_stream.EOL();
489+
// Retrieve the diagnostics.
490+
std::vector<DiagnosticDetail> details;
491+
llvm::consumeError(llvm::handleErrors(
492+
result_valobj_sp->GetError().ToError(),
493+
[&](ExpressionError &error) { details = error.GetDetails(); }));
494+
// Find the position of the expression in the command.
495+
std::optional<uint16_t> expr_pos;
496+
size_t nchar = m_original_command.find(expr);
497+
if (nchar != std::string::npos)
498+
expr_pos = nchar + GetDebugger().GetPrompt().size();
499+
500+
if (!details.empty()) {
501+
bool show_inline =
502+
GetDebugger().GetShowInlineDiagnostics() && !expr.contains('\n');
503+
RenderDiagnosticDetails(error_stream, expr_pos, show_inline, details);
498504
} else {
499-
error_stream.PutCString("error: unknown error\n");
505+
const char *error_cstr = result_valobj_sp->GetError().AsCString();
506+
llvm::StringRef error(error_cstr);
507+
if (!error.empty()) {
508+
if (!error.starts_with("error:"))
509+
error_stream << "error: ";
510+
error_stream << error;
511+
if (!error.ends_with('\n'))
512+
error_stream.EOL();
513+
} else {
514+
error_stream << "error: unknown error\n";
515+
}
500516
}
501-
502517
result.SetStatus(eReturnStatusFailed);
503518
}
504519
}
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
//===-- DiagnosticRendering.h -----------------------------------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLDB_SOURCE_COMMANDS_DIAGNOSTICRENDERING_H
10+
#define LLDB_SOURCE_COMMANDS_DIAGNOSTICRENDERING_H
11+
12+
#include "lldb/Expression/DiagnosticManager.h"
13+
#include "lldb/Utility/Stream.h"
14+
#include "llvm/Support/WithColor.h"
15+
16+
namespace lldb_private {
17+
18+
static llvm::raw_ostream &PrintSeverity(Stream &stream,
19+
lldb::Severity severity) {
20+
llvm::HighlightColor color;
21+
llvm::StringRef text;
22+
switch (severity) {
23+
case lldb::eSeverityError:
24+
color = llvm::HighlightColor::Error;
25+
text = "error: ";
26+
break;
27+
case lldb::eSeverityWarning:
28+
color = llvm::HighlightColor::Warning;
29+
text = "warning: ";
30+
break;
31+
case lldb::eSeverityInfo:
32+
color = llvm::HighlightColor::Remark;
33+
text = "note: ";
34+
break;
35+
}
36+
return llvm::WithColor(stream.AsRawOstream(), color, llvm::ColorMode::Enable)
37+
<< text;
38+
}
39+
40+
// Public for unittesting.
41+
static void RenderDiagnosticDetails(Stream &stream,
42+
std::optional<uint16_t> offset_in_command,
43+
bool show_inline,
44+
llvm::ArrayRef<DiagnosticDetail> details) {
45+
if (details.empty())
46+
return;
47+
48+
if (!offset_in_command) {
49+
for (const DiagnosticDetail &detail : details) {
50+
PrintSeverity(stream, detail.severity);
51+
stream << detail.rendered << '\n';
52+
}
53+
return;
54+
}
55+
56+
// Print a line with caret indicator(s) below the lldb prompt + command.
57+
const size_t padding = *offset_in_command;
58+
stream << std::string(padding, ' ');
59+
60+
size_t offset = 1;
61+
std::vector<DiagnosticDetail> remaining_details, other_details,
62+
hidden_details;
63+
for (const DiagnosticDetail &detail : details) {
64+
if (!show_inline || !detail.source_location) {
65+
other_details.push_back(detail);
66+
continue;
67+
}
68+
if (detail.source_location->hidden) {
69+
hidden_details.push_back(detail);
70+
continue;
71+
}
72+
if (!detail.source_location->in_user_input) {
73+
other_details.push_back(detail);
74+
continue;
75+
}
76+
77+
auto &loc = *detail.source_location;
78+
remaining_details.push_back(detail);
79+
if (offset > loc.column)
80+
continue;
81+
stream << std::string(loc.column - offset, ' ') << '^';
82+
if (loc.length > 1)
83+
stream << std::string(loc.length - 1, '~');
84+
offset = loc.column + 1;
85+
}
86+
stream << '\n';
87+
88+
// Work through each detail in reverse order using the vector/stack.
89+
bool did_print = false;
90+
for (auto detail = remaining_details.rbegin();
91+
detail != remaining_details.rend();
92+
++detail, remaining_details.pop_back()) {
93+
// Get the information to print this detail and remove it from the stack.
94+
// Print all the lines for all the other messages first.
95+
stream << std::string(padding, ' ');
96+
size_t offset = 1;
97+
for (auto &remaining_detail :
98+
llvm::ArrayRef(remaining_details).drop_back(1)) {
99+
uint16_t column = remaining_detail.source_location->column;
100+
stream << std::string(column - offset, ' ') << "";
101+
offset = column + 1;
102+
}
103+
104+
// Print the line connecting the ^ with the error message.
105+
uint16_t column = detail->source_location->column;
106+
if (offset <= column)
107+
stream << std::string(column - offset, ' ') << "╰─ ";
108+
109+
// Print a colorized string based on the message's severity type.
110+
PrintSeverity(stream, detail->severity);
111+
112+
// Finally, print the message and start a new line.
113+
stream << detail->message << '\n';
114+
did_print = true;
115+
}
116+
117+
// Print the non-located details.
118+
for (const DiagnosticDetail &detail : other_details) {
119+
PrintSeverity(stream, detail.severity);
120+
stream << detail.rendered << '\n';
121+
did_print = true;
122+
}
123+
124+
// Print the hidden details as a last resort.
125+
if (!did_print)
126+
for (const DiagnosticDetail &detail : hidden_details) {
127+
PrintSeverity(stream, detail.severity);
128+
stream << detail.rendered << '\n';
129+
}
130+
}
131+
132+
} // namespace lldb_private
133+
#endif

lldb/source/Core/CoreProperties.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,4 +225,8 @@ let Definition = "debugger" in {
225225
DefaultEnumValue<"eDWIMPrintVerbosityNone">,
226226
EnumValues<"OptionEnumValues(g_dwim_print_verbosities)">,
227227
Desc<"The verbosity level used by dwim-print.">;
228+
def ShowInlineDiagnostics: Property<"show-inline-diagnostics", "Boolean">,
229+
Global,
230+
DefaultFalse,
231+
Desc<"Controls whether diagnostics can refer directly to the command input, drawing arrows to it. If false, diagnostics will echo the input.">;
228232
}

lldb/source/Core/Debugger.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -592,7 +592,18 @@ lldb::DWIMPrintVerbosity Debugger::GetDWIMPrintVerbosity() const {
592592
const uint32_t idx = ePropertyDWIMPrintVerbosity;
593593
return GetPropertyAtIndexAs<lldb::DWIMPrintVerbosity>(
594594
idx, static_cast<lldb::DWIMPrintVerbosity>(
595-
g_debugger_properties[idx].default_uint_value));
595+
g_debugger_properties[idx].default_uint_value != 0));
596+
}
597+
598+
bool Debugger::GetShowInlineDiagnostics() const {
599+
const uint32_t idx = ePropertyShowInlineDiagnostics;
600+
return GetPropertyAtIndexAs<bool>(
601+
idx, g_debugger_properties[idx].default_uint_value);
602+
}
603+
604+
bool Debugger::SetShowInlineDiagnostics(bool b) {
605+
const uint32_t idx = ePropertyShowInlineDiagnostics;
606+
return SetPropertyAtIndex(idx, b);
596607
}
597608

598609
#pragma mark Debugger

lldb/source/Expression/DiagnosticManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ ExpressionError::ExpressionError(lldb::ExpressionResults result,
3737
: ErrorInfo(std::error_code(result, expression_category())), m_message(msg),
3838
m_details(details) {}
3939

40-
static const char *StringForSeverity(lldb::Severity severity) {
40+
static llvm::StringRef StringForSeverity(lldb::Severity severity) {
4141
switch (severity) {
4242
// this should be exhaustive
4343
case lldb::eSeverityError:

0 commit comments

Comments
 (0)