diff options
author | Adrian Prantl <aprantl@apple.com> | 2024-09-18 14:54:49 -0700 |
---|---|---|
committer | Adrian Prantl <aprantl@apple.com> | 2024-09-20 17:08:36 -0700 |
commit | b44da2446b17aaa847bf76f81a01870917f8736b (patch) | |
tree | 3eb33031a6529f3c808081795472c36443479d31 /lldb/source/Plugins/ScriptInterpreter/Python | |
parent | f732157a9d067e4d300905c831a964222e0eadee (diff) | |
download | llvm-b44da2446b17aaa847bf76f81a01870917f8736b.zip llvm-b44da2446b17aaa847bf76f81a01870917f8736b.tar.gz llvm-b44da2446b17aaa847bf76f81a01870917f8736b.tar.bz2 |
[lldb] Change the implementation of Status to store an llvm::Error (NFC) (#106774)
(based on a conversation I had with @labath yesterday in
https://github.com/llvm/llvm-project/pull/106442)
Most APIs that currently vend a Status would be better served by
returning llvm::Expected<> instead. If possibles APIs should be
refactored to avoid Status. The only legitimate long-term uses of Status
are objects that need to store an error for a long time (which should be
questioned as a design decision, too).
This patch makes the transition to llvm::Error easier by making the
places that cannot switch to llvm::Error explicit: They are marked with
a call to Status::clone(). Every other API can and should be refactored
to use llvm::Expected. In the end Status should only be used in very few
places.
Whenever an unchecked Error is dropped by Status it logs this to the
verbose API channel.
Implementation notes:
This patch introduces two new kinds of error_category as well as new
llvm::Error types. Here is the mapping of lldb::ErrorType to
llvm::Errors:
```
(eErrorTypeInvalid)
eErrorTypeGeneric llvm::StringError
eErrorTypePOSIX llvm::ECError
eErrorTypeMachKernel MachKernelError
eErrorTypeExpression llvm::ErrorList<ExpressionError>
eErrorTypeWin32 Win32Error
```
Relanding with built-in cloning support for llvm::ECError, and support
for initializing a Windows error with a NO_ERROR error code.
Diffstat (limited to 'lldb/source/Plugins/ScriptInterpreter/Python')
-rw-r--r-- | lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp | 29 |
1 files changed, 19 insertions, 10 deletions
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 24cf3430..90ccd10 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -1108,9 +1108,10 @@ public: py_error = Status::FromError(r.takeError()); } base_error = Base::Close(); + // Cloning since the wrapped exception may still reference the PyThread. if (py_error.Fail()) - return py_error; - return base_error; + return py_error.Clone(); + return base_error.Clone(); }; PyObject *GetPythonObject() const { @@ -1196,7 +1197,8 @@ public: return Flush(); auto r = m_py_obj.CallMethod("close"); if (!r) - return Status::FromError(r.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(r.takeError()).Clone(); return Status(); } @@ -1204,7 +1206,8 @@ public: GIL takeGIL; auto r = m_py_obj.CallMethod("flush"); if (!r) - return Status::FromError(r.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(r.takeError()).Clone(); return Status(); } @@ -1240,7 +1243,8 @@ public: PyObject *pybuffer_p = PyMemoryView_FromMemory( const_cast<char *>((const char *)buf), num_bytes, PyBUF_READ); if (!pybuffer_p) - return Status::FromError(llvm::make_error<PythonException>()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(llvm::make_error<PythonException>()).Clone(); auto pybuffer = Take<PythonObject>(pybuffer_p); num_bytes = 0; auto bytes_written = As<long long>(m_py_obj.CallMethod("write", pybuffer)); @@ -1260,7 +1264,8 @@ public: auto pybuffer_obj = m_py_obj.CallMethod("read", (unsigned long long)num_bytes); if (!pybuffer_obj) - return Status::FromError(pybuffer_obj.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(pybuffer_obj.takeError()).Clone(); num_bytes = 0; if (pybuffer_obj.get().IsNone()) { // EOF @@ -1269,7 +1274,8 @@ public: } auto pybuffer = PythonBuffer::Create(pybuffer_obj.get()); if (!pybuffer) - return Status::FromError(pybuffer.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(pybuffer.takeError()).Clone(); memcpy(buf, pybuffer.get().get().buf, pybuffer.get().get().len); num_bytes = pybuffer.get().get().len; return Status(); @@ -1300,7 +1306,8 @@ public: auto bytes_written = As<long long>(m_py_obj.CallMethod("write", pystring.get())); if (!bytes_written) - return Status::FromError(bytes_written.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(bytes_written.takeError()).Clone(); if (bytes_written.get() < 0) return Status::FromErrorString( ".write() method returned a negative number!"); @@ -1321,14 +1328,16 @@ public: auto pystring = As<PythonString>( m_py_obj.CallMethod("read", (unsigned long long)num_chars)); if (!pystring) - return Status::FromError(pystring.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(pystring.takeError()).Clone(); if (pystring.get().IsNone()) { // EOF return Status(); } auto stringref = pystring.get().AsUTF8(); if (!stringref) - return Status::FromError(stringref.takeError()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(stringref.takeError()).Clone(); num_bytes = stringref.get().size(); memcpy(buf, stringref.get().begin(), num_bytes); return Status(); |