eliminate nontrivial Reset(...) from TypedPythonObject

Summary:
This deletes `Reset(...)`, except for the no-argument form `Reset()`
from `TypedPythonObject`, and therefore from `PythonString`, `PythonList`,
etc.

It updates the various callers to use assignment, `As<>`, `Take<>`,
and `Retain<>`, as appropriate.

followon to https://reviews.llvm.org/D69080

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D69133

llvm-svn: 375350
This commit is contained in:
Lawrence D'Anna 2019-10-19 18:43:49 +00:00
parent 1d509201e2
commit 722b618924
4 changed files with 78 additions and 46 deletions

View File

@ -34,6 +34,7 @@ using namespace lldb_private::python;
using llvm::cantFail;
using llvm::Error;
using llvm::Expected;
using llvm::Twine;
template <> Expected<bool> python::As<bool>(Expected<PythonObject> &&obj) {
if (!obj)
@ -278,7 +279,7 @@ PythonByteArray::PythonByteArray(llvm::ArrayRef<uint8_t> bytes)
PythonByteArray::PythonByteArray(const uint8_t *bytes, size_t length) {
const char *str = reinterpret_cast<const char *>(bytes);
Reset(PyRefType::Owned, PyByteArray_FromStringAndSize(str, length));
*this = Take<PythonByteArray>(PyByteArray_FromStringAndSize(str, length));
}
bool PythonByteArray::Check(PyObject *py_obj) {
@ -522,11 +523,11 @@ StructuredData::BooleanSP PythonBoolean::CreateStructuredBoolean() const {
PythonList::PythonList(PyInitialValue value) {
if (value == PyInitialValue::Empty)
Reset(PyRefType::Owned, PyList_New(0));
*this = Take<PythonList>(PyList_New(0));
}
PythonList::PythonList(int list_size) {
Reset(PyRefType::Owned, PyList_New(list_size));
*this = Take<PythonList>(PyList_New(list_size));
}
bool PythonList::Check(PyObject *py_obj) {
@ -578,11 +579,11 @@ StructuredData::ArraySP PythonList::CreateStructuredArray() const {
PythonTuple::PythonTuple(PyInitialValue value) {
if (value == PyInitialValue::Empty)
Reset(PyRefType::Owned, PyTuple_New(0));
*this = Take<PythonTuple>(PyTuple_New(0));
}
PythonTuple::PythonTuple(int tuple_size) {
Reset(PyRefType::Owned, PyTuple_New(tuple_size));
*this = Take<PythonTuple>(PyTuple_New(tuple_size));
}
PythonTuple::PythonTuple(std::initializer_list<PythonObject> objects) {
@ -649,7 +650,7 @@ StructuredData::ArraySP PythonTuple::CreateStructuredArray() const {
PythonDictionary::PythonDictionary(PyInitialValue value) {
if (value == PyInitialValue::Empty)
Reset(PyRefType::Owned, PyDict_New());
*this = Take<PythonDictionary>(PyDict_New());
}
bool PythonDictionary::Check(PyObject *py_obj) {
@ -696,10 +697,10 @@ PythonDictionary::GetItem(const PythonObject &key) const {
return Retain<PythonObject>(o);
}
Expected<PythonObject> PythonDictionary::GetItem(const char *key) const {
Expected<PythonObject> PythonDictionary::GetItem(const Twine &key) const {
if (!IsValid())
return nullDeref();
PyObject *o = PyDict_GetItemString(m_py_obj, key);
PyObject *o = PyDict_GetItemString(m_py_obj, NullTerminated(key));
if (PyErr_Occurred())
return exception();
if (!o)
@ -717,11 +718,11 @@ Error PythonDictionary::SetItem(const PythonObject &key,
return Error::success();
}
Error PythonDictionary::SetItem(const char *key,
Error PythonDictionary::SetItem(const Twine &key,
const PythonObject &value) const {
if (!IsValid() || !value.IsValid())
return nullDeref();
int r = PyDict_SetItemString(m_py_obj, key, value.get());
int r = PyDict_SetItemString(m_py_obj, NullTerminated(key), value.get());
if (r < 0)
return exception();
return Error::success();
@ -763,20 +764,20 @@ PythonModule PythonModule::AddModule(llvm::StringRef module) {
return PythonModule(PyRefType::Borrowed, PyImport_AddModule(str.c_str()));
}
Expected<PythonModule> PythonModule::Import(const char *name) {
PyObject *mod = PyImport_ImportModule(name);
Expected<PythonModule> PythonModule::Import(const Twine &name) {
PyObject *mod = PyImport_ImportModule(NullTerminated(name));
if (!mod)
return exception();
return Take<PythonModule>(mod);
}
Expected<PythonObject> PythonModule::Get(const char *name) {
Expected<PythonObject> PythonModule::Get(const Twine &name) {
if (!IsValid())
return nullDeref();
PyObject *dict = PyModule_GetDict(m_py_obj);
if (!dict)
return exception();
PyObject *item = PyDict_GetItemString(dict, name);
PyObject *item = PyDict_GetItemString(dict, NullTerminated(name));
if (!item)
return exception();
return Retain<PythonObject>(item);
@ -790,7 +791,9 @@ bool PythonModule::Check(PyObject *py_obj) {
}
PythonDictionary PythonModule::GetDictionary() const {
return PythonDictionary(PyRefType::Borrowed, PyModule_GetDict(m_py_obj));
if (!IsValid())
return PythonDictionary();
return Retain<PythonDictionary>(PyModule_GetDict(m_py_obj));
}
bool PythonCallable::Check(PyObject *py_obj) {
@ -1092,6 +1095,8 @@ public:
assert(m_py_obj);
GIL takeGIL;
Close();
// we need to ensure the python object is released while we still
// hold the GIL
m_py_obj.Reset();
}

View File

@ -151,6 +151,30 @@ template <typename T> T Retain(PyObject *obj) {
return std::move(thing);
}
// This class can be used like a utility function to convert from
// a llvm-friendly Twine into a null-terminated const char *,
// which is the form python C APIs want their strings in.
//
// Example:
// const llvm::Twine &some_twine;
// PyFoo_Bar(x, y, z, NullTerminated(some_twine));
//
// Why a class instead of a function? If the twine isn't already null
// terminated, it will need a temporary buffer to copy the string
// into. We need that buffer to stick around for the lifetime of the
// statement.
class NullTerminated {
const char *str;
llvm::SmallString<32> storage;
public:
NullTerminated(const llvm::Twine &twine) {
llvm::StringRef ref = twine.toNullTerminatedStringRef(storage);
str = ref.begin();
}
operator const char *() { return str; }
};
} // namespace python
enum class PyInitialValue { Invalid, Empty };
@ -323,10 +347,11 @@ public:
return python::Take<PythonObject>(obj);
}
llvm::Expected<PythonObject> GetAttribute(const char *name) const {
llvm::Expected<PythonObject> GetAttribute(const llvm::Twine &name) const {
using namespace python;
if (!m_py_obj)
return nullDeref();
PyObject *obj = PyObject_GetAttrString(m_py_obj, name);
PyObject *obj = PyObject_GetAttrString(m_py_obj, NullTerminated(name));
if (!obj)
return exception();
return python::Take<PythonObject>(obj);
@ -392,10 +417,11 @@ public:
// This can be eliminated once we drop python 2 support.
static void Convert(PyRefType &type, PyObject *&py_obj) {}
using PythonObject::Reset;
void Reset() { PythonObject::Reset(); }
void Reset(PyRefType type, PyObject *py_obj) {
Reset();
void Reset(PyRefType type, PyObject *py_obj) = delete;
TypedPythonObject(PyRefType type, PyObject *py_obj) {
if (!py_obj)
return;
T::Convert(type, py_obj);
@ -405,8 +431,6 @@ public:
Py_DECREF(py_obj);
}
TypedPythonObject(PyRefType type, PyObject *py_obj) { Reset(type, py_obj); }
TypedPythonObject() {}
};
@ -562,9 +586,9 @@ public:
const PythonObject &value); // DEPRECATED
llvm::Expected<PythonObject> GetItem(const PythonObject &key) const;
llvm::Expected<PythonObject> GetItem(const char *key) const;
llvm::Expected<PythonObject> GetItem(const llvm::Twine &key) const;
llvm::Error SetItem(const PythonObject &key, const PythonObject &value) const;
llvm::Error SetItem(const char *key, const PythonObject &value) const;
llvm::Error SetItem(const llvm::Twine &key, const PythonObject &value) const;
StructuredData::DictionarySP CreateStructuredDictionary() const;
};
@ -592,9 +616,9 @@ public:
return std::move(mod.get());
}
static llvm::Expected<PythonModule> Import(const char *name);
static llvm::Expected<PythonModule> Import(const llvm::Twine &name);
llvm::Expected<PythonObject> Get(const char *name);
llvm::Expected<PythonObject> Get(const llvm::Twine &name);
PythonDictionary GetDictionary() const;
};
@ -708,6 +732,17 @@ template <typename T> T unwrapOrSetPythonException(llvm::Expected<T> expected) {
return T();
}
namespace python {
// This is only here to help incrementally migrate old, exception-unsafe
// code.
template <typename T> T unwrapIgnoringErrors(llvm::Expected<T> expected) {
if (expected)
return std::move(expected.get());
llvm::consumeError(expected.takeError());
return T();
}
} // namespace python
} // namespace lldb_private
#endif

View File

@ -55,6 +55,7 @@
using namespace lldb;
using namespace lldb_private;
using namespace lldb_private::python;
// Defined in the SWIG source file
#if PY_MAJOR_VERSION >= 3
@ -765,19 +766,16 @@ PythonDictionary &ScriptInterpreterPythonImpl::GetSessionDictionary() {
if (!main_dict.IsValid())
return m_session_dict;
PythonObject item = main_dict.GetItemForKey(PythonString(m_dictionary_name));
m_session_dict.Reset(PyRefType::Borrowed, item.get());
m_session_dict = unwrapIgnoringErrors(
As<PythonDictionary>(main_dict.GetItem(m_dictionary_name)));
return m_session_dict;
}
PythonDictionary &ScriptInterpreterPythonImpl::GetSysModuleDictionary() {
if (m_sys_module_dict.IsValid())
return m_sys_module_dict;
PythonObject sys_module(PyRefType::Borrowed, PyImport_AddModule("sys"));
if (sys_module.IsValid())
m_sys_module_dict.Reset(PyRefType::Borrowed,
PyModule_GetDict(sys_module.get()));
PythonModule sys_module = unwrapIgnoringErrors(PythonModule::Import("sys"));
m_sys_module_dict = sys_module.GetDictionary();
return m_sys_module_dict;
}
@ -1053,9 +1051,8 @@ bool ScriptInterpreterPythonImpl::ExecuteOneLineWithReturn(
PythonDictionary locals = GetSessionDictionary();
if (!locals.IsValid()) {
locals.Reset(
PyRefType::Owned,
PyObject_GetAttrString(globals.get(), m_dictionary_name.c_str()));
locals = unwrapIgnoringErrors(
As<PythonDictionary>(globals.GetAttribute(m_dictionary_name)));
}
if (!locals.IsValid())
@ -1204,9 +1201,8 @@ Status ScriptInterpreterPythonImpl::ExecuteMultipleLines(
PythonDictionary locals = GetSessionDictionary();
if (!locals.IsValid())
locals.Reset(
PyRefType::Owned,
PyObject_GetAttrString(globals.get(), m_dictionary_name.c_str()));
locals = unwrapIgnoringErrors(
As<PythonDictionary>(globals.GetAttribute(m_dictionary_name)));
if (!locals.IsValid())
locals = globals;

View File

@ -27,8 +27,7 @@ public:
void SetUp() override {
PythonTestSuite::SetUp();
PythonString sys_module("sys");
m_sys_module.Reset(PyRefType::Owned, PyImport_Import(sys_module.get()));
m_sys_module = unwrapIgnoringErrors(PythonModule::Import("sys"));
m_main_module = PythonModule::MainModule();
m_builtins_module = PythonModule::BuiltinsModule();
}
@ -70,13 +69,10 @@ TEST_F(PythonDataObjectsTest, TestResetting) {
PythonDictionary dict(PyInitialValue::Empty);
PyObject *new_dict = PyDict_New();
dict.Reset(PyRefType::Owned, new_dict);
dict = Take<PythonDictionary>(new_dict);
EXPECT_EQ(new_dict, dict.get());
dict.Reset(PyRefType::Owned, nullptr);
EXPECT_EQ(nullptr, dict.get());
dict.Reset(PyRefType::Owned, PyDict_New());
dict = Take<PythonDictionary>(PyDict_New());
EXPECT_NE(nullptr, dict.get());
dict.Reset();
EXPECT_EQ(nullptr, dict.get());