Skip to content
Permalink
Browse files

eliminate one form of PythonObject::Reset()

Summary:
I'd like to eliminate all forms of Reset() and all public constructors
on these objects, so the only way to make them is with Take<> and Retain<>
and the only way to copy or move them is with actual c++ copy, move, or
assignment.

This is a simple place to start.

Reviewers: JDevlieghere, clayborg, labath, jingham

Reviewed By: labath

Subscribers: lldb-commits

Tags: #lldb

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

llvm-svn: 375182
  • Loading branch information
Lawrence D'Anna
Lawrence D'Anna committed Oct 17, 2019
1 parent c86a6ac commit 03819d1c80ad5d7b34f8fe0d375fcd2ef6aa5e7f
@@ -410,7 +410,7 @@ void PythonString::SetString(llvm::StringRef string) {
llvm::consumeError(s.takeError());
Reset();
} else {
PythonObject::Reset(std::move(s.get()));
*this = std::move(s.get());
}
}

@@ -182,7 +182,8 @@ class PythonObject {
Reset(type, py_obj);
}

PythonObject(const PythonObject &rhs) : m_py_obj(nullptr) { Reset(rhs); }
PythonObject(const PythonObject &rhs)
: PythonObject(PyRefType::Borrowed, rhs.m_py_obj) {}

PythonObject(PythonObject &&rhs) {
m_py_obj = rhs.m_py_obj;
@@ -197,19 +198,6 @@ class PythonObject {
m_py_obj = nullptr;
}

void Reset(const PythonObject &rhs) {
if (!rhs.IsValid())
Reset();
else
Reset(PyRefType::Borrowed, rhs.m_py_obj);
}

// PythonObject is implicitly convertible to PyObject *, which will call the
// wrong overload. We want to explicitly disallow this, since a PyObject
// *always* owns its reference. Therefore the overload which takes a
// PyRefType doesn't make sense, and the copy constructor should be used.
void Reset(PyRefType type, const PythonObject &ref) = delete;

void Reset(PyRefType type, PyObject *py_obj) {
if (py_obj == m_py_obj)
return;
@@ -244,19 +232,9 @@ class PythonObject {
return result;
}

PythonObject &operator=(const PythonObject &other) {
Reset(PyRefType::Borrowed, other.get());
return *this;
}

void Reset(PythonObject &&other) {
PythonObject &operator=(PythonObject other) {
Reset();
m_py_obj = other.m_py_obj;
other.m_py_obj = nullptr;
}

PythonObject &operator=(PythonObject &&other) {
Reset(std::move(other));
m_py_obj = std::exchange(other.m_py_obj, nullptr);
return *this;
}

@@ -323,8 +323,8 @@ TEST_F(PythonDataObjectsTest, TestPythonListValueEquality) {
PythonList list(PyRefType::Owned, py_list);

PythonObject list_items[list_size];
list_items[0].Reset(PythonInteger(long_value0));
list_items[1].Reset(PythonString(string_value1));
list_items[0] = PythonInteger(long_value0);
list_items[1] = PythonString(string_value1);

for (unsigned i = 0; i < list_size; ++i)
list.SetItemAtIndex(i, list_items[i]);
@@ -469,10 +469,10 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryValueEquality) {
PythonObject py_keys[dict_entries];
PythonObject py_values[dict_entries];

py_keys[0].Reset(PythonString(key_0));
py_keys[1].Reset(PythonInteger(key_1));
py_values[0].Reset(PythonInteger(value_0));
py_values[1].Reset(PythonString(value_1));
py_keys[0] = PythonString(key_0);
py_keys[1] = PythonInteger(key_1);
py_values[0] = PythonInteger(value_0);
py_values[1] = PythonString(value_1);

PyObject *py_dict = PyDict_New();
EXPECT_TRUE(PythonDictionary::Check(py_dict));
@@ -509,10 +509,10 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) {
PythonString keys[dict_entries];
PythonObject values[dict_entries];

keys[0].Reset(PythonString(key_0));
keys[1].Reset(PythonString(key_1));
values[0].Reset(PythonInteger(value_0));
values[1].Reset(PythonString(value_1));
keys[0] = PythonString(key_0);
keys[1] = PythonString(key_1);
values[0] = PythonInteger(value_0);
values[1] = PythonString(value_1);

PythonDictionary dict(PyInitialValue::Empty);
for (int i = 0; i < 2; ++i)

0 comments on commit 03819d1

Please sign in to comment.
You can’t perform that action at this time.