Skip to content

Commit b798f4b

Browse files
adrian-prantladrian-prantl
authored
[lldb] Make deep copies of Status explicit (NFC) (#107170)
1 parent 53d5ffe commit b798f4b

Some content is hidden

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

57 files changed

+172
-141
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp) {
3333
SWIGTYPE_p_lldb__SBBreakpoint);
3434
}
3535

36-
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
37-
return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
36+
PythonObject SWIGBridge::ToSWIGWrapper(Status status) {
37+
return ToSWIGHelper(new lldb::SBError(std::move(status)), 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/Utility/Status.h

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,32 @@ const char *ExpressionResultAsCString(lldb::ExpressionResults result);
4343
/// of themselves for printing results and error codes. The string value will
4444
/// be fetched on demand and its string value will be cached until the error
4545
/// is cleared of the value of the error changes.
46+
///
47+
/// API design notes:
48+
///
49+
/// Most APIs that currently vend a Status would be better served by
50+
/// returning llvm::Expected<> instead. If possibles APIs should be
51+
/// refactored to avoid Status. The only legitimate long-term uses of
52+
/// Status are objects that need to store an error for a long time
53+
/// (which should be questioned as a design decision, too).
54+
///
55+
/// Implementation notes:
56+
///
57+
/// Internally, Status stores an llvm::Error.
58+
/// eErrorTypeInvalid
59+
/// eErrorTypeGeneric llvm::StringError
60+
/// eErrorTypePOSIX llvm::ECError
61+
/// eErrorTypeMachKernel MachKernelError
62+
/// eErrorTypeExpression llvm::ErrorList<ExpressionError>
63+
/// eErrorTypeWin32 Win32Error
64+
4665
class Status {
4766
public:
48-
/// Every error value that this object can contain needs to be able to fit
4967
/// into ValueType.
5068
typedef uint32_t ValueType;
5169

5270
Status();
71+
Status(Status &&other) = default;
5372

5473
/// Initialize the error object with a generic success value.
5574
///
@@ -93,10 +112,14 @@ class Status {
93112

94113
~Status();
95114

115+
const Status &operator=(Status &&);
96116
/// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
97117
static Status FromError(llvm::Error error);
98-
/// FIXME: Replace this with a takeError method.
118+
/// FIXME: Replace this with a takeError() method.
99119
llvm::Error ToError() const;
120+
/// Don't call this function in new code. Instead, redesign the API
121+
/// to use llvm::Expected instead of Status.
122+
Status Clone() const { return Status(ToError()); }
100123

101124
/// Get the error string associated with the current error.
102125
//

‎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

‎lldb/source/API/SBError.cpp

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ SBError::SBError() { LLDB_INSTRUMENT_VA(this); }
2323
SBError::SBError(const SBError &rhs) {
2424
LLDB_INSTRUMENT_VA(this, rhs);
2525

26-
m_opaque_up = clone(rhs.m_opaque_up);
26+
if (rhs.m_opaque_up)
27+
m_opaque_up = std::make_unique<Status>(rhs.m_opaque_up->Clone());
2728
}
2829

2930
SBError::SBError(const char *message) {
@@ -32,8 +33,8 @@ SBError::SBError(const char *message) {
3233
SetErrorString(message);
3334
}
3435

35-
SBError::SBError(const lldb_private::Status &status)
36-
: m_opaque_up(new Status(status)) {
36+
SBError::SBError(lldb_private::Status &&status)
37+
: m_opaque_up(new Status(std::move(status))) {
3738
LLDB_INSTRUMENT_VA(this, status);
3839
}
3940

@@ -43,7 +44,9 @@ const SBError &SBError::operator=(const SBError &rhs) {
4344
LLDB_INSTRUMENT_VA(this, rhs);
4445

4546
if (this != &rhs)
46-
m_opaque_up = clone(rhs.m_opaque_up);
47+
if (rhs.m_opaque_up)
48+
m_opaque_up = std::make_unique<Status>(rhs.m_opaque_up->Clone());
49+
4750
return *this;
4851
}
4952

@@ -111,9 +114,9 @@ void SBError::SetError(uint32_t err, ErrorType type) {
111114
*m_opaque_up = Status(err, type);
112115
}
113116

114-
void SBError::SetError(const Status &lldb_error) {
117+
void SBError::SetError(Status &&lldb_error) {
115118
CreateIfNeeded();
116-
*m_opaque_up = lldb_error;
119+
*m_opaque_up = std::move(lldb_error);
117120
}
118121

119122
void SBError::SetErrorToErrno() {

‎lldb/source/API/SBFile.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ SBError SBFile::Read(uint8_t *buf, size_t num_bytes, size_t *bytes_read) {
6262
error = Status::FromErrorString("invalid SBFile");
6363
*bytes_read = 0;
6464
} else {
65-
Status status = m_opaque_sp->Read(buf, num_bytes);
66-
error.SetError(status);
65+
error.SetError(m_opaque_sp->Read(buf, num_bytes));
6766
*bytes_read = num_bytes;
6867
}
6968
return error;
@@ -78,8 +77,7 @@ SBError SBFile::Write(const uint8_t *buf, size_t num_bytes,
7877
error = Status::FromErrorString("invalid SBFile");
7978
*bytes_written = 0;
8079
} else {
81-
Status status = m_opaque_sp->Write(buf, num_bytes);
82-
error.SetError(status);
80+
error.SetError(m_opaque_sp->Write(buf, num_bytes));
8381
*bytes_written = num_bytes;
8482
}
8583
return error;
@@ -92,8 +90,7 @@ SBError SBFile::Flush() {
9290
if (!m_opaque_sp) {
9391
error = Status::FromErrorString("invalid SBFile");
9492
} else {
95-
Status status = m_opaque_sp->Flush();
96-
error.SetError(status);
93+
error.SetError(m_opaque_sp->Flush());
9794
}
9895
return error;
9996
}
@@ -106,10 +103,8 @@ bool SBFile::IsValid() const {
106103
SBError SBFile::Close() {
107104
LLDB_INSTRUMENT_VA(this);
108105
SBError error;
109-
if (m_opaque_sp) {
110-
Status status = m_opaque_sp->Close();
111-
error.SetError(status);
112-
}
106+
if (m_opaque_sp)
107+
error.SetError(m_opaque_sp->Close());
113108
return error;
114109
}
115110

‎lldb/source/API/SBFormat.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ SBFormat::SBFormat(const char *format, lldb::SBError &error) {
3636
FormatEntrySP format_entry_sp = std::make_shared<FormatEntity::Entry>();
3737
Status status = FormatEntity::Parse(format, *format_entry_sp);
3838

39-
error.SetError(status);
39+
error.SetError(std::move(status));
4040
if (error.Success())
4141
m_opaque_sp = format_entry_sp;
4242
}

‎lldb/source/API/SBFrame.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
809809
Status var_error;
810810
variable_list = frame->GetVariableList(true, &var_error);
811811
if (var_error.Fail())
812-
value_list.SetError(var_error);
812+
value_list.SetError(std::move(var_error));
813813
if (variable_list) {
814814
const size_t num_variables = variable_list->GetSize();
815815
if (num_variables) {
@@ -1033,7 +1033,8 @@ SBValue SBFrame::EvaluateExpression(const char *expr) {
10331033
Status error;
10341034
error = Status::FromErrorString("can't evaluate expressions when the "
10351035
"process is running.");
1036-
ValueObjectSP error_val_sp = ValueObjectConstResult::Create(nullptr, error);
1036+
ValueObjectSP error_val_sp =
1037+
ValueObjectConstResult::Create(nullptr, std::move(error));
10371038
result.SetSP(error_val_sp, false);
10381039
}
10391040
return result;
@@ -1129,13 +1130,13 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
11291130
Status error;
11301131
error = Status::FromErrorString("can't evaluate expressions when the "
11311132
"process is running.");
1132-
expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
1133+
expr_value_sp = ValueObjectConstResult::Create(nullptr, std::move(error));
11331134
expr_result.SetSP(expr_value_sp, false);
11341135
}
11351136
} else {
11361137
Status error;
11371138
error = Status::FromErrorString("sbframe object is not valid.");
1138-
expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
1139+
expr_value_sp = ValueObjectConstResult::Create(nullptr, std::move(error));
11391140
expr_result.SetSP(expr_value_sp, false);
11401141
}
11411142

‎lldb/source/API/SBPlatform.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,7 @@ SBProcess SBPlatform::Attach(SBAttachInfo &attach_info,
586586
Status status;
587587
ProcessSP process_sp = platform_sp->Attach(info, debugger.ref(),
588588
target.GetSP().get(), status);
589-
error.SetError(status);
589+
error.SetError(std::move(status));
590590
return SBProcess(process_sp);
591591
}
592592

@@ -728,7 +728,7 @@ SBError SBPlatform::SetLocateModuleCallback(
728728
symbol_file_spec = symbol_file_spec_sb.ref();
729729
}
730730

731-
return error.ref();
731+
return error.ref().Clone();
732732
});
733733
return SBError();
734734
}

‎lldb/source/API/SBProcess.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1450,7 +1450,7 @@ lldb::SBError SBProcess::DeallocateMemory(lldb::addr_t ptr) {
14501450
std::lock_guard<std::recursive_mutex> guard(
14511451
process_sp->GetTarget().GetAPIMutex());
14521452
Status error = process_sp->DeallocateMemory(ptr);
1453-
sb_error.SetError(error);
1453+
sb_error.SetError(std::move(error));
14541454
} else {
14551455
sb_error = Status::FromErrorString("process is running");
14561456
}

‎lldb/source/API/SBSaveCoreOptions.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,7 @@ SBSaveCoreOptions::operator=(const SBSaveCoreOptions &rhs) {
4040

4141
SBError SBSaveCoreOptions::SetPluginName(const char *name) {
4242
LLDB_INSTRUMENT_VA(this, name);
43-
lldb_private::Status error = m_opaque_up->SetPluginName(name);
44-
return SBError(error);
43+
return SBError(m_opaque_up->SetPluginName(name));
4544
}
4645

4746
void SBSaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) {

‎lldb/source/API/SBStructuredData.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ lldb::SBError SBStructuredData::GetDescription(lldb::SBStream &stream) const {
133133

134134
Status error = m_impl_up->GetDescription(stream.ref());
135135
SBError sb_error;
136-
sb_error.SetError(error);
136+
sb_error.SetError(std::move(error));
137137
return sb_error;
138138
}
139139

‎lldb/source/API/SBTarget.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,7 @@ SBTarget::WatchpointCreateByAddress(lldb::addr_t addr, size_t size,
13691369
CompilerType *type = nullptr;
13701370
watchpoint_sp =
13711371
target_sp->CreateWatchpoint(addr, size, type, watch_type, cw_error);
1372-
error.SetError(cw_error);
1372+
error.SetError(std::move(cw_error));
13731373
sb_watchpoint.SetSP(watchpoint_sp);
13741374
}
13751375

@@ -2326,7 +2326,8 @@ lldb::SBValue SBTarget::EvaluateExpression(const char *expr,
23262326
Status error;
23272327
error = Status::FromErrorString("can't evaluate expressions when the "
23282328
"process is running.");
2329-
expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
2329+
expr_value_sp =
2330+
ValueObjectConstResult::Create(nullptr, std::move(error));
23302331
}
23312332
} else {
23322333
target->EvaluateExpression(expr, frame, expr_value_sp, options.ref());

‎lldb/source/API/SBThread.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,7 @@ SBError SBThread::JumpToLine(lldb::SBFileSpec &file_spec, uint32_t line) {
958958
Thread *thread = exe_ctx.GetThreadPtr();
959959

960960
Status err = thread->JumpToLine(file_spec.ref(), line, true);
961-
sb_error.SetError(err);
961+
sb_error.SetError(std::move(err));
962962
return sb_error;
963963
}
964964

0 commit comments

Comments
 (0)