roundtrip test via reference passed to aliased class method

rhaschke · rhaschke · commit 8119419bf3e9 · 2021-03-21T22:01:28.000+01:00
Probably the test is failing, because it passes the arguments by value
instead of by reference.
diff --git a/tests/test_class_sh_basic.cpp b/tests/test_class_sh_basic.cpp
@@ -2,6 +2,7 @@
 
 #include <pybind11/smart_holder.h>
 
+#include <cstdint>
 #include <memory>
 #include <string>
 #include <vector>
@@ -21,17 +22,23 @@ struct consumer { // unique_ptr consumer
     std::unique_ptr<atyp> held;
     bool valid() const { return static_cast<bool>(held); }
 
-    std::string pass_uq_valu(std::unique_ptr<atyp> obj) { held = std::move(obj); return held->mtxt; }
-    std::string pass_uq_rref(std::unique_ptr<atyp> &&obj) { held = std::move(obj);  return held->mtxt; }
+    std::string pass_uq_valu(std::unique_ptr<atyp> obj) {
+        held = std::move(obj);
+        return held->mtxt;
+    }
+    std::string pass_uq_rref(std::unique_ptr<atyp> &&obj) {
+        held = std::move(obj);
+        return held->mtxt;
+    }
     std::string pass_uq_cref(const std::unique_ptr<atyp> &obj) { return obj->mtxt; }
     std::string pass_cptr(const atyp *obj) { return obj->mtxt; }
     std::string pass_cref(const atyp &obj) { return obj.mtxt; }
 
-    std::unique_ptr<atyp>        rtrn_uq_valu() { return std::move(held); }
-    std::unique_ptr<atyp>&       rtrn_uq_lref() { return held; }
-    const std::unique_ptr<atyp>& rtrn_uq_cref() { return held; }
-    const atyp* rtrn_cptr() { return held.get(); }
-    const atyp& rtrn_cref() { return *held; }
+    std::unique_ptr<atyp> rtrn_uq_valu() { return std::move(held); }
+    std::unique_ptr<atyp> &rtrn_uq_lref() { return held; }
+    const std::unique_ptr<atyp> &rtrn_uq_cref() { return held; }
+    const atyp *rtrn_cptr() { return held.get(); }
+    const atyp &rtrn_cref() { return *held; }
 };
 
 // clang-format off
@@ -74,7 +81,7 @@ std::string pass_udcp(std::unique_ptr<atyp const, sddc> obj) { return "pass_udcp
 
 // Helpers for testing.
 std::string get_mtxt(atyp const &obj) { return obj.mtxt; }
-std::ptrdiff_t get_ptr(atyp const &obj) { return reinterpret_cast<std::ptrdiff_t>(&obj); }
+std::uintptr_t get_ptr(atyp const &obj) { return reinterpret_cast<std::uintptr_t>(&obj); }
 
 std::unique_ptr<atyp> unique_ptr_roundtrip(std::unique_ptr<atyp> obj) { return obj; }
 const std::unique_ptr<atyp> &unique_ptr_cref_roundtrip(const std::unique_ptr<atyp> &obj) {
@@ -150,21 +157,21 @@ TEST_SUBMODULE(class_sh_basic, m) {
         .def("rtrn_cptr", &consumer::rtrn_cptr, py::return_value_policy::reference_internal)
         .def("rtrn_cref", &consumer::rtrn_cref, py::return_value_policy::reference_internal);
 
-        // Helpers for testing.
-        // These require selected functions above to work first, as indicated:
-        m.def("get_mtxt", get_mtxt); // pass_cref
-        m.def("get_ptr", get_ptr);   // pass_cref
+    // Helpers for testing.
+    // These require selected functions above to work first, as indicated:
+    m.def("get_mtxt", get_mtxt); // pass_cref
+    m.def("get_ptr", get_ptr);   // pass_cref
 
-        m.def("unique_ptr_roundtrip", unique_ptr_roundtrip); // pass_uqmp, rtrn_uqmp
-        m.def("unique_ptr_cref_roundtrip", unique_ptr_cref_roundtrip);
+    m.def("unique_ptr_roundtrip", unique_ptr_roundtrip); // pass_uqmp, rtrn_uqmp
+    m.def("unique_ptr_cref_roundtrip", unique_ptr_cref_roundtrip);
 
-        py::classh<SharedPtrStash>(m, "SharedPtrStash")
-            .def(py::init<>())
-            .def("Add", &SharedPtrStash::Add, py::arg("obj"));
+    py::classh<SharedPtrStash>(m, "SharedPtrStash")
+        .def(py::init<>())
+        .def("Add", &SharedPtrStash::Add, py::arg("obj"));
 
-        m.def("py_type_handle_of_atyp", []() {
-            return py::type::handle_of<atyp>(); // Exercises static_cast in this function.
-        });
+    m.def("py_type_handle_of_atyp", []() {
+        return py::type::handle_of<atyp>(); // Exercises static_cast in this function.
+    });
 }
 
 } // namespace class_sh_basic
diff --git a/tests/test_class_sh_with_alias.cpp b/tests/test_class_sh_with_alias.cpp
@@ -2,6 +2,7 @@
 
 #include <pybind11/smart_holder.h>
 
+#include <cstdint>
 #include <memory>
 
 namespace pybind11_tests {
@@ -73,14 +74,92 @@ void wrap(py::module_ m, const char *py_class_name) {
     m.def("AddInCppUniquePtr", AddInCppUniquePtr<SerNo>, py::arg("obj"), py::arg("other_val"));
 }
 
+struct Passenger {
+    std::string mtxt;
+    // on construction: store pointer as an id
+    Passenger() : mtxt(id() + "_") {}
+    Passenger(const Passenger &other) { mtxt = other.mtxt + "Copy->" + id(); }
+    Passenger(Passenger &&other) { mtxt = other.mtxt + "Move->" + id(); }
+    std::string id() const { return std::to_string(reinterpret_cast<uintptr_t>(this)); }
+};
+struct ConsumerBase {
+    ConsumerBase()                     = default;
+    ConsumerBase(const ConsumerBase &) = default;
+    ConsumerBase(ConsumerBase &&)      = default;
+    virtual ~ConsumerBase()            = default;
+    virtual void pass_uq_cref(const std::unique_ptr<Passenger> &obj) { modify(*obj); };
+    virtual void pass_valu(Passenger obj) { modify(obj); };
+    virtual void pass_lref(Passenger &obj) { modify(obj); };
+    virtual void pass_cref(const Passenger &obj) { modify(const_cast<Passenger &>(obj)); };
+    void modify(Passenger &obj) {
+        // when base virtual method is called: append obj pointer again (should be same as before)
+        obj.mtxt.append("_");
+        obj.mtxt.append(std::to_string(reinterpret_cast<uintptr_t>(&obj)));
+    }
+};
+struct ConsumerBaseAlias : ConsumerBase {
+    using ConsumerBase::ConsumerBase;
+    void pass_uq_cref(const std::unique_ptr<Passenger> &obj) override {
+        PYBIND11_OVERRIDE(void, ConsumerBase, pass_uq_cref, obj);
+    }
+    void pass_valu(Passenger obj) override {
+        PYBIND11_OVERRIDE(void, ConsumerBase, pass_valu, obj);
+    }
+    void pass_lref(Passenger &obj) override {
+        PYBIND11_OVERRIDE(void, ConsumerBase, pass_lref, obj);
+    }
+    void pass_cref(const Passenger &obj) override {
+        PYBIND11_OVERRIDE(void, ConsumerBase, pass_cref, obj);
+    }
+};
+
+// check roundtrip of Passenger send to ConsumerBaseAlias
+// TODO: Find template magic to avoid code duplication
+std::string check_roundtrip_uq_cref(ConsumerBase &consumer) {
+    std::unique_ptr<Passenger> obj(new Passenger());
+    consumer.pass_uq_cref(obj);
+    return obj->mtxt;
+}
+std::string check_roundtrip_valu(ConsumerBase &consumer) {
+    Passenger obj;
+    consumer.pass_valu(obj);
+    return obj.mtxt;
+}
+std::string check_roundtrip_lref(ConsumerBase &consumer) {
+    Passenger obj;
+    consumer.pass_lref(obj);
+    return obj.mtxt;
+}
+std::string check_roundtrip_cref(ConsumerBase &consumer) {
+    Passenger obj;
+    consumer.pass_cref(obj);
+    return obj.mtxt;
+}
+
 } // namespace test_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::test_class_sh_with_alias::Passenger)
+PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::test_class_sh_with_alias::ConsumerBase)
 
 TEST_SUBMODULE(class_sh_with_alias, m) {
     using namespace pybind11_tests::test_class_sh_with_alias;
     wrap<0>(m, "Abase0");
     wrap<1>(m, "Abase1");
+
+    py::classh<Passenger>(m, "Passenger").def_readwrite("mtxt", &Passenger::mtxt);
+
+    py::classh<ConsumerBase, ConsumerBaseAlias>(m, "ConsumerBase")
+        .def(py::init<>())
+        .def("pass_uq_cref", &ConsumerBase::pass_uq_cref)
+        .def("pass_valu", &ConsumerBase::pass_valu)
+        .def("pass_lref", &ConsumerBase::pass_lref)
+        .def("pass_cref", &ConsumerBase::pass_cref);
+
+    m.def("check_roundtrip_uq_cref", check_roundtrip_uq_cref);
+    m.def("check_roundtrip_valu", check_roundtrip_valu);
+    m.def("check_roundtrip_lref", check_roundtrip_lref);
+    m.def("check_roundtrip_cref", check_roundtrip_cref);
 }
diff --git a/tests/test_class_sh_with_alias.py b/tests/test_class_sh_with_alias.py
@@ -1,9 +1,17 @@
 # -*- coding: utf-8 -*-
+import re
 import pytest
+import env  # noqa: F401
 
 from pybind11_tests import class_sh_with_alias as m
 
 
+def check_regex(expected, actual):
+    result = re.match(expected + "$", actual)
+    if result is None:
+        pytest.fail("expected: '{}' != actual: '{}'".format(expected, actual))
+
+
 class PyDrvd0(m.Abase0):
     def __init__(self, val):
         super(PyDrvd0, self).__init__(val)
@@ -56,3 +64,89 @@ def test_drvd1_add_in_cpp_unique_ptr():
         drvd = PyDrvd1(25)
         assert m.AddInCppUniquePtr(drvd, 83) == ((25 * 10 + 3) * 200 + 83) * 100 + 13
         return  # Comment out for manual leak checking (use `top` command).
+
+
+class PyConsumer1(m.ConsumerBase):
+    def __init__(self):
+        m.ConsumerBase.__init__(self)
+
+    def pass_uq_cref(self, obj):
+        obj.mtxt = obj.mtxt + "pass_uq_cref"
+
+    def pass_valu(self, obj):
+        obj.mtxt = obj.mtxt + "pass_valu"
+
+    def pass_lref(self, obj):
+        obj.mtxt = obj.mtxt + "pass_lref"
+
+    def pass_cref(self, obj):
+        obj.mtxt = obj.mtxt + "pass_cref"
+
+
+class PyConsumer2(m.ConsumerBase):
+    """This one, additionally to PyConsumer1 calls the base methods.
+    This results in a second call to the trampoline override dispatcher.
+    Hence arguments have travelled a long way back and forth between C++
+    and Python: C++ -> Python (call #1) -> C++ (call #2)."""
+
+    def __init__(self):
+        m.ConsumerBase.__init__(self)
+
+    def pass_uq_cref(self, obj):
+        obj.mtxt = obj.mtxt + "pass_uq_cref"
+        m.ConsumerBase.pass_uq_cref(self, obj)
+
+    def pass_valu(self, obj):
+        obj.mtxt = obj.mtxt + "pass_valu"
+        m.ConsumerBase.pass_valu(self, obj)
+
+    def pass_lref(self, obj):
+        obj.mtxt = obj.mtxt + "pass_lref"
+        m.ConsumerBase.pass_lref(self, obj)
+
+    def pass_cref(self, obj):
+        obj.mtxt = obj.mtxt + "pass_cref"
+        m.ConsumerBase.pass_cref(self, obj)
+
+
+# roundtrip tests, creating an object in C++ that is passed by reference
+# to a virtual method of a class derived in Python. Thus:
+# C++ -> Python -> C++
+@pytest.mark.parametrize(
+    "f, expected",
+    [
+        (m.check_roundtrip_uq_cref, "([0-9]+)_pass_uq_cref"),
+        (m.check_roundtrip_valu, "([0-9]+)_"),  # modification not passed back to C++
+        (m.check_roundtrip_lref, "([0-9]+)_pass_lref"),
+        pytest.param(
+            m.check_roundtrip_cref,
+            "([0-9]+)_pass_cref",
+            marks=pytest.mark.skipif("env.PYPY"),
+        ),
+    ],
+)
+def test_unique_ptr_consumer1_roundtrip(f, expected):
+    c = PyConsumer1()
+    check_regex(expected, f(c))
+
+
+@pytest.mark.parametrize(
+    "f, expected",
+    [
+        pytest.param(  # cannot (yet) load unowned const unique_ptr& (for 2nd call)
+            m.check_roundtrip_uq_cref,
+            "([0-9]+)_pass_uq_cref_\\1",
+            marks=pytest.mark.xfail,
+        ),
+        (m.check_roundtrip_valu, "([0-9]+)_"),  # modification not passed back to C++
+        (m.check_roundtrip_lref, "([0-9]+)_pass_lref_\\1"),
+        pytest.param(  # PYPY always copies the argument instead of passing the reference
+            m.check_roundtrip_cref,
+            "([0-9]+)_pass_cref_\\1",
+            marks=pytest.mark.skipif("env.PYPY"),
+        ),
+    ],
+)
+def test_unique_ptr_consumer2_roundtrip(f, expected):
+    c = PyConsumer2()
+    check_regex(expected, f(c))
