Skip to content

[lldb] Change the implementation of Status to store an llvm::Error (NFC) #106774

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 18, 2024
Merged

[lldb] Change the implementation of Status to store an llvm::Error (NFC) #106774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 76 additions & 9 deletions lldb/include/lldb/Utility/Status.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,69 @@ namespace lldb_private {

const char *ExpressionResultAsCString(lldb::ExpressionResults result);

/// Going a bit against the spirit of llvm::Error,
/// lldb_private::Status need to store errors long-term and sometimes
/// copy them. This base class defines an interface for this
/// operation.
class CloneableError
: public llvm::ErrorInfo<CloneableError, llvm::ErrorInfoBase> {
public:
using llvm::ErrorInfo<CloneableError, llvm::ErrorInfoBase>::ErrorInfo;
CloneableError() : ErrorInfo() {}
virtual std::unique_ptr<CloneableError> Clone() const = 0;
static char ID;
};

/// Common base class for all error-code errors.
class CloneableECError
: public llvm::ErrorInfo<CloneableECError, CloneableError> {
public:
using llvm::ErrorInfo<CloneableECError, CloneableError>::ErrorInfo;
CloneableECError() = delete;
CloneableECError(std::error_code ec) : ErrorInfo(), EC(ec) {}
std::error_code convertToErrorCode() const override { return EC; }
void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
std::unique_ptr<CloneableError> Clone() const override;
static char ID;

protected:
std::error_code EC;
};

/// FIXME: Move these declarations closer to where they're used.
class MachKernelError
: public llvm::ErrorInfo<MachKernelError, CloneableECError> {
public:
using llvm::ErrorInfo<MachKernelError, CloneableECError>::ErrorInfo;
MachKernelError(std::error_code ec) : ErrorInfo(ec) {}
std::string message() const override;
std::unique_ptr<CloneableError> Clone() const override;
static char ID;
};

class Win32Error : public llvm::ErrorInfo<Win32Error, CloneableECError> {
public:
using llvm::ErrorInfo<Win32Error, CloneableECError>::ErrorInfo;
Win32Error(std::error_code ec, const llvm::Twine &msg = {}) : ErrorInfo(ec) {}
std::string message() const override;
std::unique_ptr<CloneableError> Clone() const override;
static char ID;
};

class ExpressionError
: public llvm::ErrorInfo<ExpressionError, CloneableECError> {
public:
using llvm::ErrorInfo<ExpressionError, CloneableECError>::ErrorInfo;
ExpressionError(std::error_code ec, std::string msg = {})
: ErrorInfo(ec), m_string(msg) {}
std::unique_ptr<CloneableError> Clone() const override;
std::string message() const override { return m_string; }
static char ID;

protected:
std::string m_string;
};

/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
///
/// This class is designed to be able to hold any error code that can be
Expand Down Expand Up @@ -100,9 +163,7 @@ class Status {
}

static Status FromExpressionError(lldb::ExpressionResults result,
std::string msg) {
return Status(result, lldb::eErrorTypeExpression, msg);
}
std::string msg);

/// Set the current error to errno.
///
Expand All @@ -115,6 +176,7 @@ class Status {
const Status &operator=(Status &&);
/// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
static Status FromError(llvm::Error error);

/// FIXME: Replace this with a takeError() method.
llvm::Error ToError() const;
/// Don't call this function in new code. Instead, redesign the API
Expand Down Expand Up @@ -149,12 +211,20 @@ class Status {

/// Access the error value.
///
/// If the internally stored \ref llvm::Error is an \ref
/// llvm::ErrorList then this returns the error value of the first
/// error.
///
/// \return
/// The error value.
ValueType GetError() const;

/// Access the error type.
///
/// If the internally stored \ref llvm::Error is an \ref
/// llvm::ErrorList then this returns the error value of the first
/// error.
///
/// \return
/// The error type enumeration value.
lldb::ErrorType GetType() const;
Expand All @@ -170,12 +240,9 @@ class Status {
bool Success() const;

protected:
Status(llvm::Error error);
/// Status code as an integer value.
ValueType m_code = 0;
/// The type of the above error code.
lldb::ErrorType m_type = lldb::eErrorTypeInvalid;
/// A string representation of the error code.
Status(llvm::Error error) : m_error(std::move(error)) {}
llvm::Error m_error;
/// TODO: Replace this with just callling toString(m_error).
mutable std::string m_string;
};

Expand Down
31 changes: 20 additions & 11 deletions lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,8 +993,8 @@ void PythonException::Restore() {
}

PythonException::~PythonException() {
Py_XDECREF(m_exception_type);
Py_XDECREF(m_exception);
Py_XDECREF(m_exception_type);
Py_XDECREF(m_traceback);
Py_XDECREF(m_repr_bytes);
}
Expand Down Expand Up @@ -1108,9 +1108,10 @@ template <typename Base> class OwnedPythonFile : public Base {
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 {
Expand Down Expand Up @@ -1196,15 +1197,17 @@ class PythonIOFile : public OwnedPythonFile<File> {
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();
}

Status Flush() override {
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();
}

Expand Down Expand Up @@ -1240,7 +1243,8 @@ class BinaryPythonFile : public PythonIOFile {
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));
Expand All @@ -1260,7 +1264,8 @@ class BinaryPythonFile : public PythonIOFile {
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
Expand All @@ -1269,7 +1274,8 @@ class BinaryPythonFile : public PythonIOFile {
}
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();
Expand Down Expand Up @@ -1300,7 +1306,8 @@ class TextPythonFile : public PythonIOFile {
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!");
Expand All @@ -1321,14 +1328,16 @@ class TextPythonFile : public PythonIOFile {
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();
Expand Down
Loading
Loading