Skip to content

Commit a269325

Browse files
adrian-prantladrian-prantl
committed
[lldb] Make deep copies of Status explicit (NFC)
1 parent 98a5687 commit a269325

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+230
-182
lines changed

‎lldb/bindings/python/python-swigsafecast.swig‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp) {
3434
}
3535

3636
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
37-
return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
37+
return ToSWIGHelper(new lldb::SBError(status.Clone()), SWIGTYPE_p_lldb__SBError);
3838
}
3939

4040
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr<lldb::SBStructuredData> data_sb) {

‎lldb/include/lldb/API/SBError.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ class LLDB_API SBError {
9797
friend class lldb_private::ScriptInterpreter;
9898
friend class lldb_private::python::SWIGBridge;
9999

100-
SBError(const lldb_private::Status &error);
100+
SBError(lldb_private::Status &&error);
101101

102102
lldb_private::Status *get();
103103

@@ -107,7 +107,7 @@ class LLDB_API SBError {
107107

108108
lldb_private::Status &ref();
109109

110-
void SetError(const lldb_private::Status &lldb_error);
110+
void SetError(lldb_private::Status &&lldb_error);
111111

112112
private:
113113
std::unique_ptr<lldb_private::Status> m_opaque_up;

‎lldb/include/lldb/API/SBValueList.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ class LLDB_API SBValueList {
9696

9797
std::unique_ptr<ValueListImpl> m_opaque_up;
9898

99-
void SetError(const lldb_private::Status &status);
99+
void SetError(lldb_private::Status &&status);
100100
};
101101

102102
} // namespace lldb

‎lldb/include/lldb/Core/ValueObjectConstResult.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ class ValueObjectConstResult : public ValueObject {
6161

6262
// When an expression fails to evaluate, we return an error
6363
static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
64-
const Status &error);
64+
Status &&error);
6565

6666
std::optional<uint64_t> GetByteSize() override;
6767

@@ -146,7 +146,7 @@ class ValueObjectConstResult : public ValueObject {
146146
ConstString name, Module *module = nullptr);
147147

148148
ValueObjectConstResult(ExecutionContextScope *exe_scope,
149-
ValueObjectManager &manager, const Status &error);
149+
ValueObjectManager &manager, Status &&error);
150150

151151
ValueObject *CreateChildAtIndex(size_t idx) override {
152152
return m_impl.CreateChildAtIndex(idx);

‎lldb/include/lldb/Target/Process.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1296,8 +1296,6 @@ class Process : public std::enable_shared_from_this<Process>,
12961296
const EvaluateExpressionOptions &options,
12971297
DiagnosticManager &diagnostic_manager);
12981298

1299-
static const char *ExecutionResultAsCString(lldb::ExpressionResults result);
1300-
13011299
void GetStatus(Stream &ostrm);
13021300

13031301
size_t GetThreadStatus(Stream &ostrm, bool only_threads_with_stop_reason,

‎lldb/include/lldb/Utility/Status.h‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,36 @@ class raw_ostream;
2626

2727
namespace lldb_private {
2828

29+
class MachKernelError
30+
: public llvm::ErrorInfo<MachKernelError, llvm::StringError> {
31+
public:
32+
using llvm::ErrorInfo<MachKernelError, llvm::StringError>::ErrorInfo;
33+
MachKernelError(std::error_code ec, const llvm::Twine &msg = {})
34+
: ErrorInfo(msg, ec) {}
35+
std::string message() const override;
36+
static char ID;
37+
};
38+
39+
class Win32Error : public llvm::ErrorInfo<Win32Error, llvm::StringError> {
40+
public:
41+
using llvm::ErrorInfo<Win32Error, llvm::StringError>::ErrorInfo;
42+
Win32Error(std::error_code ec, const llvm::Twine &msg = {})
43+
: ErrorInfo(msg, ec) {}
44+
std::string message() const override;
45+
static char ID;
46+
};
47+
48+
class ExpressionError
49+
: public llvm::ErrorInfo<ExpressionError, llvm::StringError> {
50+
public:
51+
using llvm::ErrorInfo<ExpressionError, llvm::StringError>::ErrorInfo;
52+
ExpressionError(std::error_code ec, std::string msg = {})
53+
: ErrorInfo(msg, ec) {}
54+
static char ID;
55+
};
56+
57+
const char *ExecutionResultAsCString(lldb::ExpressionResults result);
58+
2959
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
3060
///
3161
/// This class is designed to be able to hold any error code that can be
@@ -41,13 +71,32 @@ namespace lldb_private {
4171
/// of themselves for printing results and error codes. The string value will
4272
/// be fetched on demand and its string value will be cached until the error
4373
/// is cleared of the value of the error changes.
74+
///
75+
/// API design notes:
76+
///
77+
/// Most APIs that currently vend a Status would be better served by
78+
/// returning llvm::Expected<> instead. If possibles APIs should be
79+
/// refactored to avoid Status. The only legitimate long-term uses of
80+
/// Status are objects that need to store an error for a long time
81+
/// (which should be questioned as a design decision, too).
82+
///
83+
/// Implementation notes:
84+
///
85+
/// Internally, Status stores an llvm::Error.
86+
/// eErrorTypeInvalid
87+
/// eErrorTypeGeneric llvm::StringError
88+
/// eErrorTypePOSIX llvm::ECError
89+
/// eErrorTypeMachKernel MachKernelError
90+
/// eErrorTypeExpression llvm::ErrorList<ExpressionError>
91+
/// eErrorTypeWin32 Win32Error
92+
4493
class Status {
4594
public:
46-
/// Every error value that this object can contain needs to be able to fit
4795
/// into ValueType.
4896
typedef uint32_t ValueType;
4997

5098
Status();
99+
Status(Status &&other) = default;
51100

52101
/// Initialize the error object with a generic success value.
53102
///
@@ -91,10 +140,14 @@ class Status {
91140

92141
~Status();
93142

143+
const Status &operator=(Status &&);
94144
/// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
95145
static Status FromError(llvm::Error &&error);
96-
/// FIXME: Replace this with a takeError method.
146+
/// FIXME: Replace this with a takeError() method.
97147
llvm::Error ToError() const;
148+
/// Don't call this function in new code. Instead, redesign the API
149+
/// to use llvm::Expected instead of Status.
150+
Status Clone() const { return Status(ToError()); }
98151

99152
/// Get the error string associated with the current error.
100153
//

‎lldb/source/API/SBBreakpoint.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,7 @@ SBError SBBreakpoint::SetScriptCallbackFunction(
622622
callback_function_name,
623623
extra_args.m_impl_up
624624
->GetObjectSP());
625-
sb_error.SetError(error);
625+
sb_error.SetError(std::move(error));
626626
} else
627627
sb_error = Status::FromErrorString("invalid breakpoint");
628628

@@ -645,7 +645,7 @@ SBError SBBreakpoint::SetScriptCallbackBody(const char *callback_body_text) {
645645
.GetScriptInterpreter()
646646
->SetBreakpointCommandCallback(bp_options, callback_body_text,
647647
/*is_callback=*/false);
648-
sb_error.SetError(error);
648+
sb_error.SetError(std::move(error));
649649
} else
650650
sb_error = Status::FromErrorString("invalid breakpoint");
651651

@@ -670,7 +670,7 @@ SBError SBBreakpoint::AddNameWithErrorHandling(const char *new_name) {
670670
bkpt_sp->GetTarget().GetAPIMutex());
671671
Status error;
672672
bkpt_sp->GetTarget().AddNameToBreakpoint(bkpt_sp, new_name, error);
673-
status.SetError(error);
673+
status.SetError(std::move(error));
674674
} else {
675675
status = Status::FromErrorString("invalid breakpoint");
676676
}

‎lldb/source/API/SBBreakpointLocation.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ SBError SBBreakpointLocation::SetScriptCallbackFunction(
239239
callback_function_name,
240240
extra_args.m_impl_up
241241
->GetObjectSP());
242-
sb_error.SetError(error);
242+
sb_error.SetError(std::move(error));
243243
} else
244244
sb_error = Status::FromErrorString("invalid breakpoint");
245245

@@ -264,7 +264,7 @@ SBBreakpointLocation::SetScriptCallbackBody(const char *callback_body_text) {
264264
.GetScriptInterpreter()
265265
->SetBreakpointCommandCallback(bp_options, callback_body_text,
266266
/*is_callback=*/false);
267-
sb_error.SetError(error);
267+
sb_error.SetError(std::move(error));
268268
} else
269269
sb_error = Status::FromErrorString("invalid breakpoint");
270270

‎lldb/source/API/SBBreakpointName.cpp‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -570,14 +570,13 @@ SBError SBBreakpointName::SetScriptCallbackFunction(
570570
m_impl_up->GetTarget()->GetAPIMutex());
571571

572572
BreakpointOptions &bp_options = bp_name->GetOptions();
573-
Status error;
574-
error = m_impl_up->GetTarget()
575-
->GetDebugger()
576-
.GetScriptInterpreter()
577-
->SetBreakpointCommandCallbackFunction(
578-
bp_options, callback_function_name,
579-
extra_args.m_impl_up->GetObjectSP());
580-
sb_error.SetError(error);
573+
Status error = m_impl_up->GetTarget()
574+
->GetDebugger()
575+
.GetScriptInterpreter()
576+
->SetBreakpointCommandCallbackFunction(
577+
bp_options, callback_function_name,
578+
extra_args.m_impl_up->GetObjectSP());
579+
sb_error.SetError(std::move(error));
581580
UpdateName(*bp_name);
582581
return sb_error;
583582
}
@@ -600,7 +599,7 @@ SBBreakpointName::SetScriptCallbackBody(const char *callback_body_text) {
600599
.GetScriptInterpreter()
601600
->SetBreakpointCommandCallback(
602601
bp_options, callback_body_text, /*is_callback=*/false);
603-
sb_error.SetError(error);
602+
sb_error.SetError(std::move(error));
604603
if (!sb_error.Fail())
605604
UpdateName(*bp_name);
606605

‎lldb/source/API/SBDebugger.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1360,7 +1360,7 @@ SBError SBDebugger::SetInternalVariable(const char *var_name, const char *value,
13601360
"invalid debugger instance name '%s'", debugger_instance_name);
13611361
}
13621362
if (error.Fail())
1363-
sb_error.SetError(error);
1363+
sb_error.SetError(std::move(error));
13641364
return sb_error;
13651365
}
13661366

0 commit comments

Comments
 (0)