Addressing reviewer requests.

Author: rwgk

tests/CMakeLists.txt

@@ -123,7 +123,6 @@ set(PYBIND11_TEST_FILES
     test_opaque_types.cpp
     test_operator_overloading.cpp
     test_pickling.cpp
-    test_pickling_trampoline.cpp
     test_pytypes.cpp
     test_sequences_and_iterators.cpp
     test_smart_ptr.cpp

tests/test_pickling.cpp

@@ -1,14 +1,65 @@
+// clang-format off
 /*
     tests/test_pickling.cpp -- pickle support
 
     Copyright (c) 2016 Wenzel Jakob <[email protected]>
+    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_tests.h"
 
+// clang-format on
+
+#include <memory>
+#include <stdexcept>
+#include <utility>
+
+namespace exercise_trampoline {
+
+struct SimpleBase {
+    int num               = 0;
+    virtual ~SimpleBase() = default;
+
+    // For compatibility with old clang versions:
+    SimpleBase()                   = default;
+    SimpleBase(const SimpleBase &) = default;
+};
+
+struct SimpleBaseTrampoline : SimpleBase {};
+
+struct SimpleCppDerived : SimpleBase {};
+
+void wrap(py::module m) {
+    py::class_<SimpleBase, SimpleBaseTrampoline>(m, "SimpleBase")
+        .def(py::init<>())
+        .def_readwrite("num", &SimpleBase::num)
+        .def(py::pickle(
+            [](py::object self) {
+                py::dict d;
+                if (py::hasattr(self, "__dict__"))
+                    d = self.attr("__dict__");
+                return py::make_tuple(self.attr("num"), d);
+            },
+            [](py::tuple t) {
+                if (t.size() != 2)
+                    throw std::runtime_error("Invalid state!");
+                auto cpp_state = std::unique_ptr<SimpleBase>(new SimpleBaseTrampoline);
+                cpp_state->num = t[0].cast<int>();
+                auto py_state  = t[1].cast<py::dict>();
+                return std::make_pair(std::move(cpp_state), py_state);
+            }));
+
+    m.def("make_SimpleCppDerivedAsBase",
+          []() { return std::unique_ptr<SimpleBase>(new SimpleCppDerived); });
+}
+
+} // namespace exercise_trampoline
+
+// clang-format off
+
 TEST_SUBMODULE(pickling, m) {
     // test_roundtrip
     class Pickleable {
@@ -130,4 +181,6 @@ TEST_SUBMODULE(pickling, m) {
                 return std::make_pair(cpp_state, py_state);
             }));
 #endif
+
+    exercise_trampoline::wrap(m);
 }

tests/test_pickling.py

@@ -45,3 +45,35 @@ def test_enum_pickle():
 
     data = pickle.dumps(e.EOne, 2)
     assert e.EOne == pickle.loads(data)
+
+
+#
+# exercise_trampoline
+#
+class SimplePyDerived(m.SimpleBase):
+    pass
+
+
+def test_roundtrip_simple_py_derived():
+    p = SimplePyDerived()
+    p.num = 202
+    p.stored_in_dict = 303
+    data = pickle.dumps(p, pickle.HIGHEST_PROTOCOL)
+    p2 = pickle.loads(data)
+    assert isinstance(p2, SimplePyDerived)
+    assert p2.num == 202
+    assert p2.stored_in_dict == 303
+
+
+def test_roundtrip_simple_cpp_derived():
+    p = m.make_SimpleCppDerivedAsBase()
+    p.num = 404
+    if not env.PYPY:
+        # To ensure that this unit test is not accidentally invalidated.
+        with pytest.raises(AttributeError):
+            # Mimics the `setstate` C++ implementation.
+            setattr(p, "__dict__", {})  # noqa: B010
+    data = pickle.dumps(p, pickle.HIGHEST_PROTOCOL)
+    p2 = pickle.loads(data)
+    assert isinstance(p2, m.SimpleBase)
+    assert p2.num == 404