From 08339d6331e5917ccc705ba933d589d1c285dd51 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 22 Mar 2021 12:16:29 -0700 Subject: [PATCH] Adding tests to exercise corner cases involving disowning. (#2912) * Adding test_class_sh_disowning. * Fixing minor namespace naming inconsistency between test_class_sh_*.cpp files. * Replacing py::overload_cast with plain cast for C++11 compatibility. * Accommodate that the C++ order of evaluation of function arguments is unspecified. --- tests/CMakeLists.txt | 1 + tests/test_class_sh_disowning.cpp | 46 ++++++++++++ tests/test_class_sh_disowning.py | 78 ++++++++++++++++++++ tests/test_class_sh_factory_constructors.cpp | 32 ++++---- tests/test_class_sh_virtual_py_cpp_mix.cpp | 13 ++-- tests/test_class_sh_with_alias.cpp | 10 +-- 6 files changed, 152 insertions(+), 28 deletions(-) create mode 100644 tests/test_class_sh_disowning.cpp create mode 100644 tests/test_class_sh_disowning.py diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7388589e5..19ba6399b 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -102,6 +102,7 @@ set(PYBIND11_TEST_FILES test_chrono.cpp test_class.cpp test_class_sh_basic.cpp + test_class_sh_disowning.cpp test_class_sh_factory_constructors.cpp test_class_sh_inheritance.cpp test_class_sh_trampoline_shared_ptr_cpp_arg.cpp diff --git a/tests/test_class_sh_disowning.cpp b/tests/test_class_sh_disowning.cpp new file mode 100644 index 000000000..b9dc1e60b --- /dev/null +++ b/tests/test_class_sh_disowning.cpp @@ -0,0 +1,46 @@ +#include "pybind11_tests.h" + +#include + +#include + +namespace pybind11_tests { +namespace class_sh_disowning { + +template // Using int as a trick to easily generate a series of types. +struct Atype { + int val = 0; + Atype(int val_) : val{val_} {} + int get() const { return val * 10 + SerNo; } +}; + +int same_twice(std::unique_ptr> at1a, std::unique_ptr> at1b) { + return at1a->get() * 100 + at1b->get() * 10; +} + +int mixed(std::unique_ptr> at1, std::unique_ptr> at2) { + return at1->get() * 200 + at2->get() * 20; +} + +int overloaded(std::unique_ptr> at1, int i) { return at1->get() * 30 + i; } +int overloaded(std::unique_ptr> at2, int i) { return at2->get() * 40 + i; } + +} // namespace class_sh_disowning +} // namespace pybind11_tests + +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_disowning::Atype<1>) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_disowning::Atype<2>) + +TEST_SUBMODULE(class_sh_disowning, m) { + using namespace pybind11_tests::class_sh_disowning; + + py::classh>(m, "Atype1").def(py::init()).def("get", &Atype<1>::get); + py::classh>(m, "Atype2").def(py::init()).def("get", &Atype<2>::get); + + m.def("same_twice", same_twice); + + m.def("mixed", mixed); + + m.def("overloaded", (int (*)(std::unique_ptr>, int)) & overloaded); + m.def("overloaded", (int (*)(std::unique_ptr>, int)) & overloaded); +} diff --git a/tests/test_class_sh_disowning.py b/tests/test_class_sh_disowning.py new file mode 100644 index 000000000..510708656 --- /dev/null +++ b/tests/test_class_sh_disowning.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +from __future__ import print_function + +import pytest + +from pybind11_tests import class_sh_disowning as m + + +def test_same_twice(): + while True: + obj1a = m.Atype1(57) + obj1b = m.Atype1(62) + assert m.same_twice(obj1a, obj1b) == (57 * 10 + 1) * 100 + (62 * 10 + 1) * 10 + obj1c = m.Atype1(0) + with pytest.raises(ValueError): + # Disowning works for one argument, but not both. + m.same_twice(obj1c, obj1c) + with pytest.raises(ValueError): + obj1c.get() + return # Comment out for manual leak checking (use `top` command). + + +def test_mixed(capsys): + first_pass = True + while True: + obj1a = m.Atype1(90) + obj2a = m.Atype2(25) + assert m.mixed(obj1a, obj2a) == (90 * 10 + 1) * 200 + (25 * 10 + 2) * 20 + + # The C++ order of evaluation of function arguments is (unfortunately) unspecified: + # https://en.cppreference.com/w/cpp/language/eval_order + # Read on. + obj1b = m.Atype1(0) + with pytest.raises(ValueError): + # If the 1st argument is evaluated first, obj1b is disowned before the conversion for + # the already disowned obj2a fails as expected. + m.mixed(obj1b, obj2a) + obj2b = m.Atype2(0) + with pytest.raises(ValueError): + # If the 2nd argument is evaluated first, obj2b is disowned before the conversion for + # the already disowned obj1a fails as expected. + m.mixed(obj1a, obj2b) + + def was_disowned(obj): + try: + obj.get() + except ValueError: + return True + return False + + # Either obj1b or obj2b was disowned in the expected failed m.mixed() calls above, but not + # both. + was_disowned_results = (was_disowned(obj1b), was_disowned(obj2b)) + assert was_disowned_results.count(True) == 1 + if first_pass: + first_pass = False + with capsys.disabled(): + print( + "\nC++ function argument %d is evaluated first." + % (was_disowned_results.index(True) + 1) + ) + + return # Comment out for manual leak checking (use `top` command). + + +def test_overloaded(): + while True: + obj1 = m.Atype1(81) + obj2 = m.Atype2(60) + with pytest.raises(TypeError): + m.overloaded(obj1, "NotInt") + assert obj1.get() == 81 * 10 + 1 # Not disowned. + assert m.overloaded(obj1, 3) == (81 * 10 + 1) * 30 + 3 + with pytest.raises(TypeError): + m.overloaded(obj2, "NotInt") + assert obj2.get() == 60 * 10 + 2 # Not disowned. + assert m.overloaded(obj2, 2) == (60 * 10 + 2) * 40 + 2 + return # Comment out for manual leak checking (use `top` command). diff --git a/tests/test_class_sh_factory_constructors.cpp b/tests/test_class_sh_factory_constructors.cpp index 9a0e5f18d..fb8a8caee 100644 --- a/tests/test_class_sh_factory_constructors.cpp +++ b/tests/test_class_sh_factory_constructors.cpp @@ -6,7 +6,7 @@ #include namespace pybind11_tests { -namespace test_class_sh_factory_constructors { +namespace class_sh_factory_constructors { template // Using int as a trick to easily generate a series of types. struct atyp { // Short for "any type". @@ -69,25 +69,25 @@ struct with_alias { struct with_alias_alias : with_alias {}; struct sddwaa : std::default_delete {}; -} // namespace test_class_sh_factory_constructors +} // namespace class_sh_factory_constructors } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_valu) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_rref) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_cref) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_mref) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_cptr) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_mptr) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_shmp) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_shcp) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_uqmp) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_uqcp) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_udmp) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::atyp_udcp) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_factory_constructors::with_alias) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_valu) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_rref) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_cref) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_mref) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_cptr) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_mptr) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_shmp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_shcp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_uqmp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_uqcp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_udmp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::atyp_udcp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_factory_constructors::with_alias) TEST_SUBMODULE(class_sh_factory_constructors, m) { - using namespace pybind11_tests::test_class_sh_factory_constructors; + using namespace pybind11_tests::class_sh_factory_constructors; py::classh(m, "atyp_valu") .def(py::init(&rtrn_valu)) diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp index 74d2db417..02850fedf 100644 --- a/tests/test_class_sh_virtual_py_cpp_mix.cpp +++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp @@ -5,7 +5,7 @@ #include namespace pybind11_tests { -namespace test_class_sh_virtual_py_cpp_mix { +namespace class_sh_virtual_py_cpp_mix { class Base { public: @@ -43,16 +43,15 @@ struct CppDerivedVirtualOverrider : CppDerived, py::virtual_overrider_self_life_ int get() const override { PYBIND11_OVERRIDE(int, CppDerived, get); } }; -} // namespace test_class_sh_virtual_py_cpp_mix +} // namespace class_sh_virtual_py_cpp_mix } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_virtual_py_cpp_mix::Base) -PYBIND11_SMART_HOLDER_TYPE_CASTERS( - pybind11_tests::test_class_sh_virtual_py_cpp_mix::CppDerivedPlain) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_virtual_py_cpp_mix::CppDerived) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::Base) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::CppDerivedPlain) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_virtual_py_cpp_mix::CppDerived) TEST_SUBMODULE(class_sh_virtual_py_cpp_mix, m) { - using namespace pybind11_tests::test_class_sh_virtual_py_cpp_mix; + using namespace pybind11_tests::class_sh_virtual_py_cpp_mix; py::classh(m, "Base").def(py::init<>()).def("get", &Base::get); diff --git a/tests/test_class_sh_with_alias.cpp b/tests/test_class_sh_with_alias.cpp index 846533ddf..649eb61f0 100644 --- a/tests/test_class_sh_with_alias.cpp +++ b/tests/test_class_sh_with_alias.cpp @@ -5,7 +5,7 @@ #include namespace pybind11_tests { -namespace test_class_sh_with_alias { +namespace class_sh_with_alias { template // Using int as a trick to easily generate a series of types. struct Abase { @@ -73,14 +73,14 @@ void wrap(py::module_ m, const char *py_class_name) { m.def("AddInCppUniquePtr", AddInCppUniquePtr, py::arg("obj"), py::arg("other_val")); } -} // namespace test_class_sh_with_alias +} // namespace class_sh_with_alias } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_with_alias::Abase<0>) -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_with_alias::Abase<1>) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_with_alias::Abase<0>) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_with_alias::Abase<1>) TEST_SUBMODULE(class_sh_with_alias, m) { - using namespace pybind11_tests::test_class_sh_with_alias; + using namespace pybind11_tests::class_sh_with_alias; wrap<0>(m, "Abase0"); wrap<1>(m, "Abase1"); }