From b44da2446b17aaa847bf76f81a01870917f8736b Mon Sep 17 00:00:00 2001 From: Adrian Prantl Date: Wed, 18 Sep 2024 14:54:49 -0700 Subject: [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 eErrorTypeWin32 Win32Error ``` Relanding with built-in cloning support for llvm::ECError, and support for initializing a Windows error with a NO_ERROR error code. --- .../ScriptInterpreter/Python/PythonDataObjects.cpp | 29 ++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) (limited to 'lldb/source/Plugins/ScriptInterpreter/Python') 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((const char *)buf), num_bytes, PyBUF_READ); if (!pybuffer_p) - return Status::FromError(llvm::make_error()); + // Cloning since the wrapped exception may still reference the PyThread. + return Status::FromError(llvm::make_error()).Clone(); auto pybuffer = Take(pybuffer_p); num_bytes = 0; auto bytes_written = As(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(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( 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(); -- cgit v1.1