From d3bd5b3d71ae9fc3a3a45e05d5dba6b1ecbcb2f5 Mon Sep 17 00:00:00 2001 From: Lawrence D'Anna Date: Tue, 15 Oct 2019 17:12:49 +0000 Subject: eliminate virtual methods from PythonDataObjects Summary: This patch eliminates a bunch of boilerplate from PythonDataObjects, as well as the use of virtual methods. In my opinion it also makes the Reset logic a lot more clear and easy to follow. The price is yet another template. I think it's worth it. Reviewers: JDevlieghere, jasonmolenda, labath, zturner Reviewed By: JDevlieghere, labath Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D68918 llvm-svn: 374916 --- .../ScriptInterpreter/Python/PythonDataObjects.cpp | 299 +++------------------ 1 file changed, 37 insertions(+), 262 deletions(-) (limited to 'lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp') diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 4d77e552246..e2112b15008 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -213,43 +213,19 @@ StructuredData::ObjectSP PythonObject::CreateStructuredObject() const { } // PythonString -PythonBytes::PythonBytes() : PythonObject() {} -PythonBytes::PythonBytes(llvm::ArrayRef bytes) : PythonObject() { - SetBytes(bytes); -} +PythonBytes::PythonBytes(llvm::ArrayRef bytes) { SetBytes(bytes); } -PythonBytes::PythonBytes(const uint8_t *bytes, size_t length) : PythonObject() { +PythonBytes::PythonBytes(const uint8_t *bytes, size_t length) { SetBytes(llvm::ArrayRef(bytes, length)); } -PythonBytes::PythonBytes(PyRefType type, PyObject *py_obj) : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a string -} - -PythonBytes::~PythonBytes() {} - bool PythonBytes::Check(PyObject *py_obj) { if (!py_obj) return false; return PyBytes_Check(py_obj); } -void PythonBytes::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonBytes::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - llvm::ArrayRef PythonBytes::GetBytes() const { if (!IsValid()) return llvm::ArrayRef(); @@ -290,36 +266,12 @@ PythonByteArray::PythonByteArray(const uint8_t *bytes, size_t length) { Reset(PyRefType::Owned, PyByteArray_FromStringAndSize(str, length)); } -PythonByteArray::PythonByteArray(PyRefType type, PyObject *o) { - Reset(type, o); -} - -PythonByteArray::PythonByteArray(const PythonBytes &object) - : PythonObject(object) {} - -PythonByteArray::~PythonByteArray() {} - bool PythonByteArray::Check(PyObject *py_obj) { if (!py_obj) return false; return PyByteArray_Check(py_obj); } -void PythonByteArray::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonByteArray::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - llvm::ArrayRef PythonByteArray::GetBytes() const { if (!IsValid()) return llvm::ArrayRef(); @@ -357,17 +309,7 @@ Expected PythonString::FromUTF8(llvm::StringRef string) { return Take(str); } -PythonString::PythonString(PyRefType type, PyObject *py_obj) : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a string -} - -PythonString::PythonString(llvm::StringRef string) : PythonObject() { - SetString(string); -} - -PythonString::PythonString() : PythonObject() {} - -PythonString::~PythonString() {} +PythonString::PythonString(llvm::StringRef string) { SetString(string); } bool PythonString::Check(PyObject *py_obj) { if (!py_obj) @@ -382,29 +324,26 @@ bool PythonString::Check(PyObject *py_obj) { return false; } -void PythonString::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonString::Check(py_obj)) { - PythonObject::Reset(); - return; - } +void PythonString::Convert(PyRefType &type, PyObject *&py_obj) { #if PY_MAJOR_VERSION < 3 // In Python 2, Don't store PyUnicode objects directly, because we need // access to their underlying character buffers which Python 2 doesn't // provide. if (PyUnicode_Check(py_obj)) { - PyObject *s = PyUnicode_AsUTF8String(result.get()); - if (s == NULL) + PyObject *s = PyUnicode_AsUTF8String(py_obj); + if (s == nullptr) { PyErr_Clear(); - result.Reset(PyRefType::Owned, s); + if (type == PyRefType::Owned) + Py_DECREF(py_obj); + return; + } + if (type == PyRefType::Owned) + Py_DECREF(py_obj); + else + type = PyRefType::Owned; + py_obj = s; } #endif - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); } llvm::StringRef PythonString::GetString() const { @@ -468,18 +407,7 @@ StructuredData::StringSP PythonString::CreateStructuredString() const { // PythonInteger -PythonInteger::PythonInteger() : PythonObject() {} - -PythonInteger::PythonInteger(PyRefType type, PyObject *py_obj) - : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a integer type -} - -PythonInteger::PythonInteger(int64_t value) : PythonObject() { - SetInteger(value); -} - -PythonInteger::~PythonInteger() {} +PythonInteger::PythonInteger(int64_t value) { SetInteger(value); } bool PythonInteger::Check(PyObject *py_obj) { if (!py_obj) @@ -494,16 +422,7 @@ bool PythonInteger::Check(PyObject *py_obj) { #endif } -void PythonInteger::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonInteger::Check(py_obj)) { - PythonObject::Reset(); - return; - } - +void PythonInteger::Convert(PyRefType &type, PyObject *&py_obj) { #if PY_MAJOR_VERSION < 3 // Always store this as a PyLong, which makes interoperability between Python // 2.x and Python 3.x easier. This is only necessary in 2.x, since 3.x @@ -512,16 +431,23 @@ void PythonInteger::Reset(PyRefType type, PyObject *py_obj) { // Since we converted the original object to a different type, the new // object is an owned object regardless of the ownership semantics // requested by the user. - result.Reset(PyRefType::Owned, PyLong_FromLongLong(PyInt_AsLong(py_obj))); + long long value = PyInt_AsLong(py_obj); + PyObject *l = nullptr; + if (!PyErr_Occurred()) + l = PyLong_FromLongLong(value); + if (l == nullptr) { + PyErr_Clear(); + if (type == PyRefType::Owned) + Py_DECREF(py_obj); + return; + } + if (type == PyRefType::Owned) + Py_DECREF(py_obj); + else + type = PyRefType::Owned; + py_obj = l; } #endif - - assert(PyLong_Check(result.get()) && - "Couldn't get a PyLong from this PyObject"); - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); } int64_t PythonInteger::GetInteger() const { @@ -555,11 +481,6 @@ StructuredData::IntegerSP PythonInteger::CreateStructuredInteger() const { // PythonBoolean -PythonBoolean::PythonBoolean(PyRefType type, PyObject *py_obj) - : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a boolean type -} - PythonBoolean::PythonBoolean(bool value) { SetValue(value); } @@ -568,21 +489,6 @@ bool PythonBoolean::Check(PyObject *py_obj) { return py_obj ? PyBool_Check(py_obj) : false; } -void PythonBoolean::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonBoolean::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - bool PythonBoolean::GetValue() const { return m_py_obj ? PyObject_IsTrue(m_py_obj) : false; } @@ -599,42 +505,21 @@ StructuredData::BooleanSP PythonBoolean::CreateStructuredBoolean() const { // PythonList -PythonList::PythonList(PyInitialValue value) : PythonObject() { +PythonList::PythonList(PyInitialValue value) { if (value == PyInitialValue::Empty) Reset(PyRefType::Owned, PyList_New(0)); } -PythonList::PythonList(int list_size) : PythonObject() { +PythonList::PythonList(int list_size) { Reset(PyRefType::Owned, PyList_New(list_size)); } -PythonList::PythonList(PyRefType type, PyObject *py_obj) : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a list -} - -PythonList::~PythonList() {} - bool PythonList::Check(PyObject *py_obj) { if (!py_obj) return false; return PyList_Check(py_obj); } -void PythonList::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonList::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - uint32_t PythonList::GetSize() const { if (IsValid()) return PyList_GET_SIZE(m_py_obj); @@ -676,19 +561,15 @@ StructuredData::ArraySP PythonList::CreateStructuredArray() const { // PythonTuple -PythonTuple::PythonTuple(PyInitialValue value) : PythonObject() { +PythonTuple::PythonTuple(PyInitialValue value) { if (value == PyInitialValue::Empty) Reset(PyRefType::Owned, PyTuple_New(0)); } -PythonTuple::PythonTuple(int tuple_size) : PythonObject() { +PythonTuple::PythonTuple(int tuple_size) { Reset(PyRefType::Owned, PyTuple_New(tuple_size)); } -PythonTuple::PythonTuple(PyRefType type, PyObject *py_obj) : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a tuple -} - PythonTuple::PythonTuple(std::initializer_list objects) { m_py_obj = PyTuple_New(objects.size()); @@ -712,29 +593,12 @@ PythonTuple::PythonTuple(std::initializer_list objects) { } } -PythonTuple::~PythonTuple() {} - bool PythonTuple::Check(PyObject *py_obj) { if (!py_obj) return false; return PyTuple_Check(py_obj); } -void PythonTuple::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonTuple::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - uint32_t PythonTuple::GetSize() const { if (IsValid()) return PyTuple_GET_SIZE(m_py_obj); @@ -768,18 +632,11 @@ StructuredData::ArraySP PythonTuple::CreateStructuredArray() const { // PythonDictionary -PythonDictionary::PythonDictionary(PyInitialValue value) : PythonObject() { +PythonDictionary::PythonDictionary(PyInitialValue value) { if (value == PyInitialValue::Empty) Reset(PyRefType::Owned, PyDict_New()); } -PythonDictionary::PythonDictionary(PyRefType type, PyObject *py_obj) - : PythonObject() { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a dictionary -} - -PythonDictionary::~PythonDictionary() {} - bool PythonDictionary::Check(PyObject *py_obj) { if (!py_obj) return false; @@ -787,21 +644,6 @@ bool PythonDictionary::Check(PyObject *py_obj) { return PyDict_Check(py_obj); } -void PythonDictionary::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonDictionary::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - uint32_t PythonDictionary::GetSize() const { if (IsValid()) return PyDict_Size(m_py_obj); @@ -841,14 +683,6 @@ PythonDictionary::CreateStructuredDictionary() const { return result; } -PythonModule::PythonModule() : PythonObject() {} - -PythonModule::PythonModule(PyRefType type, PyObject *py_obj) { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a module -} - -PythonModule::~PythonModule() {} - PythonModule PythonModule::BuiltinsModule() { #if PY_MAJOR_VERSION >= 3 return AddModule("builtins"); @@ -890,33 +724,10 @@ bool PythonModule::Check(PyObject *py_obj) { return PyModule_Check(py_obj); } -void PythonModule::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonModule::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - PythonDictionary PythonModule::GetDictionary() const { return PythonDictionary(PyRefType::Borrowed, PyModule_GetDict(m_py_obj)); } -PythonCallable::PythonCallable() : PythonObject() {} - -PythonCallable::PythonCallable(PyRefType type, PyObject *py_obj) { - Reset(type, py_obj); // Use "Reset()" to ensure that py_obj is a callable -} - -PythonCallable::~PythonCallable() {} - bool PythonCallable::Check(PyObject *py_obj) { if (!py_obj) return false; @@ -924,21 +735,6 @@ bool PythonCallable::Check(PyObject *py_obj) { return PyCallable_Check(py_obj); } -void PythonCallable::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonCallable::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - PythonCallable::ArgInfo PythonCallable::GetNumInitArguments() const { ArgInfo result = {0, false, false, false}; if (!IsValid()) @@ -1011,12 +807,6 @@ operator()(std::initializer_list args) { PyObject_CallObject(m_py_obj, arg_tuple.get())); } -PythonFile::PythonFile() : PythonObject() {} - -PythonFile::PythonFile(PyRefType type, PyObject *o) { Reset(type, o); } - -PythonFile::~PythonFile() {} - bool PythonFile::Check(PyObject *py_obj) { if (!py_obj) return false; @@ -1047,21 +837,6 @@ bool PythonFile::Check(PyObject *py_obj) { #endif } -void PythonFile::Reset(PyRefType type, PyObject *py_obj) { - // Grab the desired reference type so that if we end up rejecting `py_obj` it - // still gets decremented if necessary. - PythonObject result(type, py_obj); - - if (!PythonFile::Check(py_obj)) { - PythonObject::Reset(); - return; - } - - // Calling PythonObject::Reset(const PythonObject&) will lead to stack - // overflow since it calls back into the virtual implementation. - PythonObject::Reset(PyRefType::Borrowed, result.get()); -} - FileUP PythonFile::GetUnderlyingFile() const { if (!IsValid()) return nullptr; -- cgit v1.2.3