Skip to content

Commit 97b3812

Browse files
committed
Support inline diagnostics in CommandReturnObject
and implement them for dwim-print (a.k.a. `p`) as an example. The next step will be to expose them as structured data in SBCommandReturnObject.
1 parent 99f527d commit 97b3812

15 files changed

+217
-107
lines changed

lldb/include/lldb/Expression/DiagnosticManager.h

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "lldb/lldb-defines.h"
1313
#include "lldb/lldb-types.h"
1414

15+
#include "lldb/Utility/DiagnosticsRendering.h"
1516
#include "lldb/Utility/FileSpec.h"
1617
#include "lldb/Utility/Status.h"
1718

@@ -23,49 +24,22 @@
2324

2425
namespace lldb_private {
2526

26-
/// A compiler-independent representation of a Diagnostic. Expression
27-
/// evaluation failures often have more than one diagnostic that a UI
28-
/// layer might want to render differently, for example to colorize
29-
/// it.
30-
///
31-
/// Running example:
32-
/// (lldb) expr 1+foo
33-
/// error: <user expression 0>:1:3: use of undeclared identifier 'foo'
34-
/// 1+foo
35-
/// ^
36-
struct DiagnosticDetail {
37-
struct SourceLocation {
38-
FileSpec file;
39-
unsigned line = 0;
40-
uint16_t column = 0;
41-
uint16_t length = 0;
42-
bool hidden = false;
43-
bool in_user_input = false;
44-
};
45-
/// Contains {{}, 1, 3, 3, true} in the example above.
46-
std::optional<SourceLocation> source_location;
47-
/// Contains eSeverityError in the example above.
48-
lldb::Severity severity = lldb::eSeverityInfo;
49-
/// Contains "use of undeclared identifier 'x'" in the example above.
50-
std::string message;
51-
/// Contains the fully rendered error message.
52-
std::string rendered;
53-
};
54-
5527
/// An llvm::Error used to communicate diagnostics in Status. Multiple
5628
/// diagnostics may be chained in an llvm::ErrorList.
5729
class ExpressionError
58-
: public llvm::ErrorInfo<ExpressionError, ExpressionErrorBase> {
30+
: public llvm::ErrorInfo<ExpressionError, DiagnosticError> {
5931
std::string m_message;
6032
std::vector<DiagnosticDetail> m_details;
6133

6234
public:
6335
static char ID;
64-
using llvm::ErrorInfo<ExpressionError, ExpressionErrorBase>::ErrorInfo;
36+
using llvm::ErrorInfo<ExpressionError, DiagnosticError>::ErrorInfo;
6537
ExpressionError(lldb::ExpressionResults result, std::string msg,
6638
std::vector<DiagnosticDetail> details = {});
6739
std::string message() const override;
68-
llvm::ArrayRef<DiagnosticDetail> GetDetails() const { return m_details; }
40+
llvm::ArrayRef<DiagnosticDetail> GetDetails() const override {
41+
return m_details;
42+
}
6943
std::error_code convertToErrorCode() const override;
7044
void log(llvm::raw_ostream &OS) const override;
7145
std::unique_ptr<CloneableError> Clone() const override;

lldb/include/lldb/Interpreter/CommandReturnObject.h

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#define LLDB_INTERPRETER_COMMANDRETURNOBJECT_H
1111

1212
#include "lldb/Host/StreamFile.h"
13+
#include "lldb/Utility/DiagnosticsRendering.h"
1314
#include "lldb/Utility/StreamString.h"
1415
#include "lldb/Utility/StreamTee.h"
1516
#include "lldb/lldb-private.h"
@@ -29,19 +30,16 @@ class CommandReturnObject {
2930

3031
~CommandReturnObject() = default;
3132

33+
llvm::StringRef GetInlineDiagnosticsData(unsigned indent);
34+
3235
llvm::StringRef GetOutputData() {
3336
lldb::StreamSP stream_sp(m_out_stream.GetStreamAtIndex(eStreamStringIndex));
3437
if (stream_sp)
3538
return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
3639
return llvm::StringRef();
3740
}
3841

39-
llvm::StringRef GetErrorData() {
40-
lldb::StreamSP stream_sp(m_err_stream.GetStreamAtIndex(eStreamStringIndex));
41-
if (stream_sp)
42-
return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
43-
return llvm::StringRef();
44-
}
42+
llvm::StringRef GetErrorData();
4543

4644
Stream &GetOutputStream() {
4745
// Make sure we at least have our normal string stream output stream
@@ -135,6 +133,11 @@ class CommandReturnObject {
135133

136134
void SetError(llvm::Error error);
137135

136+
void SetDiagnosticIndent(uint16_t indent) { m_diagnostic_indent = indent; }
137+
std::optional<uint16_t> GetDiagnosticIndent() const {
138+
return m_diagnostic_indent;
139+
}
140+
138141
lldb::ReturnStatus GetStatus() const;
139142

140143
void SetStatus(lldb::ReturnStatus status);
@@ -160,6 +163,9 @@ class CommandReturnObject {
160163

161164
StreamTee m_out_stream;
162165
StreamTee m_err_stream;
166+
std::vector<DiagnosticDetail> m_diagnostics;
167+
StreamString m_diag_stream;
168+
std::optional<uint16_t> m_diagnostic_indent;
163169

164170
lldb::ReturnStatus m_status = lldb::eReturnStatusStarted;
165171

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
//===-- DiagnosticsRendering.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_UTILITY_DIAGNOSTICSRENDERING_H
10+
#define LLDB_UTILITY_DIAGNOSTICSRENDERING_H
11+
12+
#include "lldb/Utility/Status.h"
13+
#include "lldb/Utility/Stream.h"
14+
#include "llvm/Support/WithColor.h"
15+
16+
namespace lldb_private {
17+
18+
/// A compiler-independent representation of an \c
19+
/// lldb_private::Diagnostic. Expression evaluation failures often
20+
/// have more than one diagnostic that a UI layer might want to render
21+
/// differently, for example to colorize it.
22+
///
23+
/// Running example:
24+
/// (lldb) expr 1 + foo
25+
/// error: <user expression 0>:1:3: use of undeclared identifier 'foo'
26+
/// 1 + foo
27+
/// ^~~
28+
struct DiagnosticDetail {
29+
/// A source location consisting of a file name and position.
30+
struct SourceLocation {
31+
/// \c "<user expression 0>" in the example above.
32+
FileSpec file;
33+
/// \c 1 in the example above.
34+
unsigned line = 0;
35+
/// \c 5 in the example above.
36+
uint16_t column = 0;
37+
/// \c 3 in the example above.
38+
uint16_t length = 0;
39+
/// Whether this source location should be surfaced to the
40+
/// user. For example, syntax errors diagnosed in LLDB's
41+
/// expression wrapper code have this set to true.
42+
bool hidden = false;
43+
/// Whether this source location refers to something the user
44+
/// typed as part of the command, i.e., if this qualifies for
45+
/// inline display, or if the source line would need to be echoed
46+
/// again for the message to make sense.
47+
bool in_user_input = false;
48+
};
49+
/// Contains this diagnostic's source location, if applicable.
50+
std::optional<SourceLocation> source_location;
51+
/// Contains \c eSeverityError in the example above.
52+
lldb::Severity severity = lldb::eSeverityInfo;
53+
/// Contains "use of undeclared identifier 'foo'" in the example above.
54+
std::string message;
55+
/// Contains the fully rendered error message, without "error: ",
56+
/// but including the source context.
57+
std::string rendered;
58+
};
59+
60+
class DiagnosticError
61+
: public llvm::ErrorInfo<DiagnosticError, CloneableECError> {
62+
public:
63+
using llvm::ErrorInfo<DiagnosticError, CloneableECError>::ErrorInfo;
64+
DiagnosticError(std::error_code ec, std::string msg = {}) : ErrorInfo(ec) {}
65+
lldb::ErrorType GetErrorType() const override;
66+
virtual llvm::ArrayRef<DiagnosticDetail> GetDetails() const;
67+
static char ID;
68+
};
69+
70+
const char *ExpressionResultAsCString(lldb::ExpressionResults result);
71+
72+
llvm::raw_ostream &PrintSeverity(Stream &stream, lldb::Severity severity);
73+
74+
void RenderDiagnosticDetails(Stream &stream,
75+
std::optional<uint16_t> offset_in_command,
76+
bool show_inline,
77+
llvm::ArrayRef<DiagnosticDetail> details);
78+
} // namespace lldb_private
79+
#endif

lldb/include/lldb/Utility/Status.h

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#ifndef LLDB_UTILITY_STATUS_H
1010
#define LLDB_UTILITY_STATUS_H
1111

12+
#include "lldb/Utility/FileSpec.h"
1213
#include "lldb/lldb-defines.h"
1314
#include "lldb/lldb-enumerations.h"
1415
#include "llvm/ADT/StringRef.h"
@@ -26,8 +27,6 @@ class raw_ostream;
2627

2728
namespace lldb_private {
2829

29-
const char *ExpressionResultAsCString(lldb::ExpressionResults result);
30-
3130
/// Going a bit against the spirit of llvm::Error,
3231
/// lldb_private::Status need to store errors long-term and sometimes
3332
/// copy them. This base class defines an interface for this
@@ -79,16 +78,6 @@ class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
7978
static char ID;
8079
};
8180

82-
class ExpressionErrorBase
83-
: public llvm::ErrorInfo<ExpressionErrorBase, CloneableECError> {
84-
public:
85-
using llvm::ErrorInfo<ExpressionErrorBase, CloneableECError>::ErrorInfo;
86-
ExpressionErrorBase(std::error_code ec, std::string msg = {})
87-
: ErrorInfo(ec) {}
88-
lldb::ErrorType GetErrorType() const override;
89-
static char ID;
90-
};
91-
9281
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
9382
///
9483
/// This class is designed to be able to hold any error code that can be

lldb/source/Commands/CommandObjectDWIMPrint.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,13 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
191191
ExpressionResults expr_result = target.EvaluateExpression(
192192
expr, exe_scope, valobj_sp, eval_options, &fixed_expression);
193193

194+
// Record the position of the expression in the command.
195+
if (fixed_expression.empty()) {
196+
size_t pos = m_original_command.find(expr);
197+
if (pos != llvm::StringRef::npos)
198+
result.SetDiagnosticIndent(pos);
199+
}
200+
194201
// Only mention Fix-Its if the expression evaluator applied them.
195202
// Compiler errors refer to the final expression after applying Fix-It(s).
196203
if (!fixed_expression.empty() && target.GetEnableNotifyAboutFixIts()) {

lldb/source/Commands/CommandObjectExpression.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "CommandObjectExpression.h"
10-
#include "DiagnosticRendering.h"
1110
#include "lldb/Core/Debugger.h"
12-
#include "lldb/Expression/DiagnosticManager.h"
1311
#include "lldb/Expression/ExpressionVariable.h"
1412
#include "lldb/Expression/REPL.h"
1513
#include "lldb/Expression/UserExpression.h"
@@ -22,6 +20,7 @@
2220
#include "lldb/Target/Process.h"
2321
#include "lldb/Target/StackFrame.h"
2422
#include "lldb/Target/Target.h"
23+
#include "lldb/Utility/DiagnosticsRendering.h"
2524
#include "lldb/lldb-enumerations.h"
2625
#include "lldb/lldb-private-enumerations.h"
2726

@@ -490,7 +489,7 @@ bool CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
490489
std::vector<DiagnosticDetail> details;
491490
llvm::consumeError(llvm::handleErrors(
492491
result_valobj_sp->GetError().ToError(),
493-
[&](ExpressionError &error) { details = error.GetDetails(); }));
492+
[&](DiagnosticError &error) { details = error.GetDetails(); }));
494493
// Find the position of the expression in the command.
495494
std::optional<uint16_t> expr_pos;
496495
size_t nchar = m_original_command.find(expr);

lldb/source/Interpreter/CommandInterpreter.cpp

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2069,7 +2069,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
20692069
remainder.c_str());
20702070

20712071
// To test whether or not transcript should be saved, `transcript_item` is
2072-
// used instead of `GetSaveTrasncript()`. This is because the latter will
2072+
// used instead of `GetSaveTranscript()`. This is because the latter will
20732073
// fail when the command is "settings set interpreter.save-transcript true".
20742074
if (transcript_item) {
20752075
transcript_item->AddStringItem("commandName", cmd_obj->GetCommandName());
@@ -3180,15 +3180,26 @@ void CommandInterpreter::IOHandlerInputComplete(IOHandler &io_handler,
31803180
if ((result.Succeeded() &&
31813181
io_handler.GetFlags().Test(eHandleCommandFlagPrintResult)) ||
31823182
io_handler.GetFlags().Test(eHandleCommandFlagPrintErrors)) {
3183-
// Display any STDOUT/STDERR _prior_ to emitting the command result text
31843183
GetProcessOutput();
31853184

3185+
// Display any inline diagnostics first.
3186+
if (!result.GetImmediateErrorStream() &&
3187+
GetDebugger().GetShowInlineDiagnostics()) {
3188+
unsigned prompt_len = GetDebugger().GetPrompt().size();
3189+
if (auto indent = result.GetDiagnosticIndent()) {
3190+
llvm::StringRef diags =
3191+
result.GetInlineDiagnosticsData(prompt_len + *indent);
3192+
PrintCommandOutput(io_handler, diags, true);
3193+
}
3194+
}
3195+
3196+
// Display any STDOUT/STDERR _prior_ to emitting the command result text.
31863197
if (!result.GetImmediateOutputStream()) {
31873198
llvm::StringRef output = result.GetOutputData();
31883199
PrintCommandOutput(io_handler, output, true);
31893200
}
31903201

3191-
// Now emit the command error text from the command we just executed
3202+
// Now emit the command error text from the command we just executed.
31923203
if (!result.GetImmediateErrorStream()) {
31933204
llvm::StringRef error = result.GetErrorData();
31943205
PrintCommandOutput(io_handler, error, false);

lldb/source/Interpreter/CommandReturnObject.cpp

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
#include "lldb/Interpreter/CommandReturnObject.h"
1010

11+
#include "lldb/Utility/DiagnosticsRendering.h"
1112
#include "lldb/Utility/Status.h"
1213
#include "lldb/Utility/StreamString.h"
1314

@@ -112,8 +113,39 @@ void CommandReturnObject::SetError(Status error) {
112113
}
113114

114115
void CommandReturnObject::SetError(llvm::Error error) {
115-
if (error)
116+
// Retrieve any diagnostics.
117+
error = llvm::handleErrors(std::move(error), [&](DiagnosticError &error) {
118+
SetStatus(eReturnStatusFailed);
119+
m_diagnostics = error.GetDetails();
120+
});
121+
if (error) {
116122
AppendError(llvm::toString(std::move(error)));
123+
}
124+
}
125+
126+
llvm::StringRef CommandReturnObject::GetInlineDiagnosticsData(unsigned indent) {
127+
RenderDiagnosticDetails(m_diag_stream, indent, true, m_diagnostics);
128+
// Duplex the diagnostics to the secondary stream (but not inlined).
129+
if (auto stream_sp = m_err_stream.GetStreamAtIndex(eStreamStringIndex))
130+
RenderDiagnosticDetails(*stream_sp, std::nullopt, false, m_diagnostics);
131+
132+
// Clear them so GetErrorData() doesn't render them again.
133+
m_diagnostics.clear();
134+
return m_diag_stream.GetString();
135+
}
136+
137+
llvm::StringRef CommandReturnObject::GetErrorData() {
138+
// Diagnostics haven't been fetched; render them now (not inlined).
139+
if (!m_diagnostics.empty()) {
140+
RenderDiagnosticDetails(GetErrorStream(), std::nullopt, false,
141+
m_diagnostics);
142+
m_diagnostics.clear();
143+
}
144+
145+
lldb::StreamSP stream_sp(m_err_stream.GetStreamAtIndex(eStreamStringIndex));
146+
if (stream_sp)
147+
return std::static_pointer_cast<StreamString>(stream_sp)->GetString();
148+
return llvm::StringRef();
117149
}
118150

119151
// Similar to AppendError, but do not prepend 'Status: ' to message, and don't

lldb/source/Utility/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ add_lldb_library(lldbUtility NO_INTERNAL_DEPENDENCIES
3838
DataEncoder.cpp
3939
DataExtractor.cpp
4040
Diagnostics.cpp
41+
DiagnosticsRendering.cpp
4142
Environment.cpp
4243
ErrorMessages.cpp
4344
Event.cpp

0 commit comments

Comments
 (0)