From 246d686fced236cc7e03272e69c64944d244b2c8 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 12 Apr 2021 16:22:37 -0700 Subject: [PATCH] Bug fix: trampoline_self_life_support CpCtor, MvCtor. --- .../pybind11/trampoline_self_life_support.h | 16 ++-- tests/CMakeLists.txt | 1 + ..._class_sh_trampoline_self_life_support.cpp | 84 +++++++++++++++++++ ...t_class_sh_trampoline_self_life_support.py | 37 ++++++++ 4 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 tests/test_class_sh_trampoline_self_life_support.cpp create mode 100644 tests/test_class_sh_trampoline_self_life_support.py diff --git a/include/pybind11/trampoline_self_life_support.h b/include/pybind11/trampoline_self_life_support.h index fe2e4899f9..806dc9dde4 100644 --- a/include/pybind11/trampoline_self_life_support.h +++ b/include/pybind11/trampoline_self_life_support.h @@ -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_; @@ -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) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 887503aef9..9200909371 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -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 diff --git a/tests/test_class_sh_trampoline_self_life_support.cpp b/tests/test_class_sh_trampoline_self_life_support.cpp new file mode 100644 index 0000000000..ddbceaad47 --- /dev/null +++ b/tests/test_class_sh_trampoline_self_life_support.cpp @@ -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 +#include + +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(m, "Big5") + .def(py::init()) + .def_readonly("history", &Big5::history); + + m.def("action", [](std::unique_ptr 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(obj.get()); + if (obj_trampoline != nullptr) { + switch (action_id) { + case 0: { // CpCtor + std::unique_ptr cp(new Big5Trampoline(*obj_trampoline)); + o2 = py::cast(std::move(cp)); + } break; + case 1: { // MvCtor + std::unique_ptr mv(new Big5Trampoline(std::move(*obj_trampoline))); + o2 = py::cast(std::move(mv)); + } break; + case 2: { // operator= lvalue + std::unique_ptr lv(new Big5Trampoline); + *lv = *obj_trampoline; + o2 = py::cast(std::move(lv)); + } break; + case 3: { // operator= rvalue + std::unique_ptr 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); + }); +} diff --git a/tests/test_class_sh_trampoline_self_life_support.py b/tests/test_class_sh_trampoline_self_life_support.py new file mode 100644 index 0000000000..f159e06a64 --- /dev/null +++ b/tests/test_class_sh_trampoline_self_life_support.py @@ -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