Skip to content

Bug fix: trampoline_self_life_support CpCtor, MvCtor. #2947

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.

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
Apr 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 10 additions & 6 deletions include/pybind11/trampoline_self_life_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ PYBIND11_NAMESPACE_END(detail)
struct trampoline_self_life_support {
detail::value_and_holder v_h;

trampoline_self_life_support() = default;

void activate_life_support(const detail::value_and_holder &v_h_) {
Py_INCREF((PyObject *) v_h_.inst);
v_h = v_h_;
Expand All @@ -46,12 +48,14 @@ struct trampoline_self_life_support {
}
}

// Some compilers complain about implicitly defined versions of some of the following:
trampoline_self_life_support() = default;
trampoline_self_life_support(const trampoline_self_life_support &) = default;
trampoline_self_life_support(trampoline_self_life_support &&) = default;
trampoline_self_life_support &operator=(const trampoline_self_life_support &) = default;
trampoline_self_life_support &operator=(trampoline_self_life_support &&) = default;
// For the next two, the default implementations generate undefined behavior (ASAN failures
// manually verified). The reason is that v_h needs to be kept default-initialized.
trampoline_self_life_support(const trampoline_self_life_support &) {}
trampoline_self_life_support(trampoline_self_life_support &&) {}

// These should never be needed (please provide test cases if you think they are).
trampoline_self_life_support &operator=(const trampoline_self_life_support &) = delete;
trampoline_self_life_support &operator=(trampoline_self_life_support &&) = delete;
};

PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ set(PYBIND11_TEST_FILES
test_class_sh_factory_constructors.cpp
test_class_sh_inheritance.cpp
test_class_sh_trampoline_basic.cpp
test_class_sh_trampoline_self_life_support.cpp
test_class_sh_trampoline_shared_ptr_cpp_arg.cpp
test_class_sh_trampoline_unique_ptr.cpp
test_class_sh_unique_ptr_member.cpp
Expand Down
84 changes: 84 additions & 0 deletions tests/test_class_sh_trampoline_self_life_support.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// Copyright (c) 2021 The Pybind Development Team.
// All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

#include "pybind11/smart_holder.h"
#include "pybind11/trampoline_self_life_support.h"
#include "pybind11_tests.h"

#include <memory>
#include <string>

namespace {

struct Big5 { // Also known as "rule of five".
std::string history;

explicit Big5(std::string history_start) : history{history_start} {}

Big5(const Big5 &other) { history = other.history + "_CpCtor"; }

Big5(Big5 &&other) { history = other.history + "_MvCtor"; }

Big5 &operator=(const Big5 &other) {
history = other.history + "_OpEqLv";
return *this;
}

Big5 &operator=(Big5 &&other) {
history = other.history + "_OpEqRv";
return *this;
}

virtual ~Big5() = default;

protected:
Big5() : history{"DefaultConstructor"} {}
};

struct Big5Trampoline : Big5, py::trampoline_self_life_support {
using Big5::Big5;
};

} // namespace

PYBIND11_SMART_HOLDER_TYPE_CASTERS(Big5)

TEST_SUBMODULE(class_sh_trampoline_self_life_support, m) {
py::classh<Big5, Big5Trampoline>(m, "Big5")
.def(py::init<std::string>())
.def_readonly("history", &Big5::history);

m.def("action", [](std::unique_ptr<Big5> obj, int action_id) {
py::object o2 = py::none();
// This is very unusual, but needed to directly exercise the trampoline_self_life_support
// CpCtor, MvCtor, operator= lvalue, operator= rvalue.
auto obj_trampoline = dynamic_cast<Big5Trampoline *>(obj.get());
if (obj_trampoline != nullptr) {
switch (action_id) {
case 0: { // CpCtor
std::unique_ptr<Big5> cp(new Big5Trampoline(*obj_trampoline));
o2 = py::cast(std::move(cp));
} break;
case 1: { // MvCtor
std::unique_ptr<Big5> mv(new Big5Trampoline(std::move(*obj_trampoline)));
o2 = py::cast(std::move(mv));
} break;
case 2: { // operator= lvalue
std::unique_ptr<Big5> lv(new Big5Trampoline);
*lv = *obj_trampoline;
o2 = py::cast(std::move(lv));
} break;
case 3: { // operator= rvalue
std::unique_ptr<Big5> rv(new Big5Trampoline);
*rv = std::move(*obj_trampoline);
o2 = py::cast(std::move(rv));
} break;
default:
break;
}
}
py::object o1 = py::cast(std::move(obj));
return py::make_tuple(o1, o2);
});
}
37 changes: 37 additions & 0 deletions tests/test_class_sh_trampoline_self_life_support.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# -*- coding: utf-8 -*-
import pytest

import pybind11_tests.class_sh_trampoline_self_life_support as m


class PyBig5(m.Big5):
pass


def test_m_big5():
obj = m.Big5("Seed")
assert obj.history == "Seed"
o1, o2 = m.action(obj, 0)
assert o1 is not obj
assert o1.history == "Seed"
with pytest.raises(ValueError) as excinfo:
obj.history
assert "Python instance was disowned" in str(excinfo.value)
assert o2 is None


@pytest.mark.parametrize(
"action_id, expected_history",
[
(0, "Seed_CpCtor"),
(1, "Seed_MvCtor"),
(2, "Seed_OpEqLv"),
(3, "Seed_OpEqRv"),
],
)
def test_py_big5(action_id, expected_history):
obj = PyBig5("Seed")
assert obj.history == "Seed"
o1, o2 = m.action(obj, action_id)
assert o1 is obj
assert o2.history == expected_history