From 07accc69d16896974a2a12b688084fc7face3bc0 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Wed, 17 Mar 2021 15:35:54 -0700
Subject: [PATCH 1/7] Adaption of PyCLIF virtual_py_cpp_mix test.

---
 tests/test_class_sh_virtual_py_cpp_mix.cpp | 52 ++++++++++++++++++++++
 tests/test_class_sh_virtual_py_cpp_mix.py  | 41 +++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100644 tests/test_class_sh_virtual_py_cpp_mix.cpp
 create mode 100644 tests/test_class_sh_virtual_py_cpp_mix.py

diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp
new file mode 100644
index 0000000000..6e3aa57fd7
--- /dev/null
+++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp
@@ -0,0 +1,52 @@
+#include "pybind11_tests.h"
+
+#include <pybind11/smart_holder.h>
+
+#include <memory>
+
+namespace pybind11_tests {
+namespace test_class_sh_virtual_py_cpp_mix {
+
+class Base {
+public:
+    virtual ~Base() = default;
+    virtual int get() const { return 101; }
+};
+
+class CppDerived : public Base {
+public:
+    int get() const override { return 212; }
+};
+
+int get_from_cpp_plainc_ptr(const Base *b) { return b->get() + 4000; }
+
+int get_from_cpp_unique_ptr(std::unique_ptr<Base> b) { return b->get() + 5000; }
+
+class BaseVirtualOverrider : public Base, py::detail::virtual_overrider_self_life_support {
+public:
+    using Base::Base;
+
+    int get() const override { PYBIND11_OVERRIDE(int, Base, get); }
+};
+
+class CppDerivedVirtualOverrider : public CppDerived, BaseVirtualOverrider {
+public:
+    using CppDerived::CppDerived;
+};
+
+} // namespace test_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::CppDerived)
+
+TEST_SUBMODULE(class_sh_virtual_py_cpp_mix, m) {
+    using namespace pybind11_tests::test_class_sh_virtual_py_cpp_mix;
+
+    py::classh<Base, BaseVirtualOverrider>(m, "Base").def(py::init<>()).def("get", &Base::get);
+
+    py::classh<CppDerived, Base, CppDerivedVirtualOverrider>(m, "CppDerived").def(py::init<>());
+
+    m.def("get_from_cpp_plainc_ptr", get_from_cpp_plainc_ptr, py::arg("b"));
+    m.def("get_from_cpp_unique_ptr", get_from_cpp_unique_ptr, py::arg("b"));
+}
diff --git a/tests/test_class_sh_virtual_py_cpp_mix.py b/tests/test_class_sh_virtual_py_cpp_mix.py
new file mode 100644
index 0000000000..7e22e84e03
--- /dev/null
+++ b/tests/test_class_sh_virtual_py_cpp_mix.py
@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+
+from pybind11_tests import class_sh_virtual_py_cpp_mix as m
+
+
+class PyDerived(m.Base):
+    def __init__(self):
+        m.Base.__init__(self)
+
+    def get(self):
+        return 323
+
+
+def test_py_derived_get():
+    d = PyDerived()
+    assert d.get() == 323
+
+
+def test_get_from_cpp_plainc_ptr_passing_py_derived():
+    d = PyDerived()
+    assert m.get_from_cpp_plainc_ptr(d) == 4323
+
+
+def test_get_from_cpp_unique_ptr_passing_py_derived():
+    d = PyDerived()
+    assert m.get_from_cpp_unique_ptr(d) == 5323
+
+
+def test_cpp_derived_get():
+    d = m.CppDerived()
+    assert d.get() == 212
+
+
+def test_get_from_cpp_plainc_ptr_passing_cpp_derived():
+    d = m.CppDerived()
+    assert m.get_from_cpp_plainc_ptr(d) == 4212
+
+
+def test_get_from_cpp_unique_ptr_passing_cpp_derived():
+    d = m.CppDerived()
+    assert m.get_from_cpp_unique_ptr(d) == 5212

From 21c369432544d9075a441fdaf812daa2e0d1bc19 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Thu, 18 Mar 2021 11:05:06 -0700
Subject: [PATCH 2/7] Removing ValueError: Ownership of instance with virtual
 overrides in Python cannot be transferred to C++. TODO: static_assert alias
 class needs to inherit from virtual_overrider_self_life_support.

---
 .../detail/smart_holder_type_casters.h        |  4 -
 tests/test_class_sh_virtual_py_cpp_mix.cpp    |  8 +-
 tests/test_class_sh_virtual_py_cpp_mix.py     | 77 ++++++++++++-------
 tests/test_class_sh_with_alias.py             |  7 +-
 4 files changed, 54 insertions(+), 42 deletions(-)

diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h
index 5e6b0c016a..ab02872f71 100644
--- a/include/pybind11/detail/smart_holder_type_casters.h
+++ b/include/pybind11/detail/smart_holder_type_casters.h
@@ -392,10 +392,6 @@ struct smart_holder_type_caster_load {
 
         auto *self_life_support
             = dynamic_cast_virtual_overrider_self_life_support_ptr(raw_type_ptr);
-        if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) {
-            throw value_error("Ownership of instance with virtual overrides in Python cannot be "
-                              "transferred to C++.");
-        }
 
         // Critical transfer-of-ownership section. This must stay together.
         if (self_life_support != nullptr) {
diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp
index 6e3aa57fd7..e82ec5d854 100644
--- a/tests/test_class_sh_virtual_py_cpp_mix.cpp
+++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp
@@ -22,16 +22,16 @@ int get_from_cpp_plainc_ptr(const Base *b) { return b->get() + 4000; }
 
 int get_from_cpp_unique_ptr(std::unique_ptr<Base> b) { return b->get() + 5000; }
 
-class BaseVirtualOverrider : public Base, py::detail::virtual_overrider_self_life_support {
-public:
+struct BaseVirtualOverrider : Base, py::detail::virtual_overrider_self_life_support {
     using Base::Base;
 
     int get() const override { PYBIND11_OVERRIDE(int, Base, get); }
 };
 
-class CppDerivedVirtualOverrider : public CppDerived, BaseVirtualOverrider {
-public:
+struct CppDerivedVirtualOverrider : CppDerived, py::detail::virtual_overrider_self_life_support {
     using CppDerived::CppDerived;
+
+    int get() const override { PYBIND11_OVERRIDE(int, CppDerived, get); }
 };
 
 } // namespace test_class_sh_virtual_py_cpp_mix
diff --git a/tests/test_class_sh_virtual_py_cpp_mix.py b/tests/test_class_sh_virtual_py_cpp_mix.py
index 7e22e84e03..d9d04f2bfc 100644
--- a/tests/test_class_sh_virtual_py_cpp_mix.py
+++ b/tests/test_class_sh_virtual_py_cpp_mix.py
@@ -1,9 +1,10 @@
 # -*- coding: utf-8 -*-
+import pytest
 
 from pybind11_tests import class_sh_virtual_py_cpp_mix as m
 
 
-class PyDerived(m.Base):
+class PyBase(m.Base):  # Avoiding name PyDerived, for more systematic naming.
     def __init__(self):
         m.Base.__init__(self)
 
@@ -11,31 +12,51 @@ def get(self):
         return 323
 
 
-def test_py_derived_get():
-    d = PyDerived()
-    assert d.get() == 323
-
-
-def test_get_from_cpp_plainc_ptr_passing_py_derived():
-    d = PyDerived()
-    assert m.get_from_cpp_plainc_ptr(d) == 4323
-
-
-def test_get_from_cpp_unique_ptr_passing_py_derived():
-    d = PyDerived()
-    assert m.get_from_cpp_unique_ptr(d) == 5323
-
-
-def test_cpp_derived_get():
-    d = m.CppDerived()
-    assert d.get() == 212
-
-
-def test_get_from_cpp_plainc_ptr_passing_cpp_derived():
-    d = m.CppDerived()
-    assert m.get_from_cpp_plainc_ptr(d) == 4212
-
+class PyCppDerived(m.CppDerived):
+    def __init__(self):
+        m.CppDerived.__init__(self)
 
-def test_get_from_cpp_unique_ptr_passing_cpp_derived():
-    d = m.CppDerived()
-    assert m.get_from_cpp_unique_ptr(d) == 5212
+    def get(self):
+        return 434
+
+
+@pytest.mark.parametrize(
+    "ctor, expected",
+    [
+        (m.Base, 101),
+        (PyBase, 323),
+        (m.CppDerived, 212),
+        (PyCppDerived, 434),
+    ],
+)
+def test_base_get(ctor, expected):
+    obj = ctor()
+    assert obj.get() == expected
+
+
+@pytest.mark.parametrize(
+    "ctor, expected",
+    [
+        (m.Base, 4101),
+        (PyBase, 4323),
+        (m.CppDerived, 4212),
+        (PyCppDerived, 4434),
+    ],
+)
+def test_get_from_cpp_plainc_ptr(ctor, expected):
+    obj = ctor()
+    assert m.get_from_cpp_plainc_ptr(obj) == expected
+
+
+@pytest.mark.parametrize(
+    "ctor, expected",
+    [
+        (m.Base, 5101),
+        (PyBase, 5323),
+        (m.CppDerived, 5212),
+        (PyCppDerived, 5434),
+    ],
+)
+def test_get_from_cpp_unique_ptr(ctor, expected):
+    obj = ctor()
+    assert m.get_from_cpp_unique_ptr(obj) == expected
diff --git a/tests/test_class_sh_with_alias.py b/tests/test_class_sh_with_alias.py
index efd6af7a4e..2720f9c86d 100644
--- a/tests/test_class_sh_with_alias.py
+++ b/tests/test_class_sh_with_alias.py
@@ -40,13 +40,8 @@ def test_drvd0_add_in_cpp_shared_ptr():
 def test_drvd0_add_in_cpp_unique_ptr():
     while True:
         drvd = PyDrvd0(0)
-        with pytest.raises(ValueError) as exc_info:
+        with pytest.raises(RuntimeError):
             m.AddInCppUniquePtr(drvd, 0)
-        assert (
-            str(exc_info.value)
-            == "Ownership of instance with virtual overrides in Python"
-            " cannot be transferred to C++."
-        )
         return  # Comment out for manual leak checking (use `top` command).
 
 

From 894919488e855986a08578f5207cd1bd459ac54c Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Thu, 18 Mar 2021 16:40:18 -0700
Subject: [PATCH 3/7] Bringing back ValueError: "... instance cannot safely be
 transferred to C++.", but based on dynamic_cast<AliasType>.

---
 .../detail/smart_holder_type_casters.h        | 24 +++++++++++--------
 .../virtual_overrider_self_life_support.h     | 16 +++++++------
 include/pybind11/pybind11.h                   |  3 ++-
 ..._class_sh_trampoline_shared_ptr_cpp_arg.py | 16 ++++---------
 tests/test_class_sh_with_alias.py             |  8 ++++++-
 5 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h
index ab02872f71..49178ad29e 100644
--- a/include/pybind11/detail/smart_holder_type_casters.h
+++ b/include/pybind11/detail/smart_holder_type_casters.h
@@ -263,15 +263,13 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag
         return &modified_type_caster_generic_load_impl::local_load;
     }
 
-    template <typename T>
-    static void init_instance_for_type(detail::instance *inst,
-                                       const void *holder_const_void_ptr,
-                                       bool has_alias) {
+    template <typename WrappedType, typename AliasType>
+    static void init_instance_for_type(detail::instance *inst, const void *holder_const_void_ptr) {
         // Need for const_cast is a consequence of the type_info::init_instance type:
         // void (*init_instance)(instance *, const void *);
         auto holder_void_ptr = const_cast<void *>(holder_const_void_ptr);
 
-        auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(T)));
+        auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(WrappedType)));
         if (!v_h.instance_registered()) {
             register_instance(inst, v_h.value_ptr(), v_h.type);
             v_h.set_instance_registered();
@@ -282,13 +280,14 @@ struct smart_holder_type_caster_class_hooks : smart_holder_type_caster_base_tag
             auto holder_ptr = static_cast<holder_type *>(holder_void_ptr);
             new (std::addressof(v_h.holder<holder_type>())) holder_type(std::move(*holder_ptr));
         } else if (inst->owned) {
-            new (std::addressof(v_h.holder<holder_type>()))
-                holder_type(holder_type::from_raw_ptr_take_ownership(v_h.value_ptr<T>()));
+            new (std::addressof(v_h.holder<holder_type>())) holder_type(
+                holder_type::from_raw_ptr_take_ownership(v_h.value_ptr<WrappedType>()));
         } else {
             new (std::addressof(v_h.holder<holder_type>()))
-                holder_type(holder_type::from_raw_ptr_unowned(v_h.value_ptr<T>()));
+                holder_type(holder_type::from_raw_ptr_unowned(v_h.value_ptr<WrappedType>()));
         }
-        v_h.holder<holder_type>().pointee_depends_on_holder_owner = has_alias;
+        v_h.holder<holder_type>().pointee_depends_on_holder_owner
+            = dynamic_raw_ptr_cast_if_possible<AliasType>(v_h.value_ptr<WrappedType>()) != nullptr;
         v_h.set_holder_constructed();
     }
 
@@ -391,7 +390,12 @@ struct smart_holder_type_caster_load {
         T *raw_type_ptr = convert_type(raw_void_ptr);
 
         auto *self_life_support
-            = dynamic_cast_virtual_overrider_self_life_support_ptr(raw_type_ptr);
+            = dynamic_raw_ptr_cast_if_possible<virtual_overrider_self_life_support>(raw_type_ptr);
+        if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) {
+            throw value_error("Alias class (also known as trampoline) does not inherit from "
+                              "py::detail::virtual_overrider_self_life_support, therefore the "
+                              "ownership of this instance cannot safely be transferred to C++.");
+        }
 
         // Critical transfer-of-ownership section. This must stay together.
         if (self_life_support != nullptr) {
diff --git a/include/pybind11/detail/virtual_overrider_self_life_support.h b/include/pybind11/detail/virtual_overrider_self_life_support.h
index 524bfcf545..19a009e450 100644
--- a/include/pybind11/detail/virtual_overrider_self_life_support.h
+++ b/include/pybind11/detail/virtual_overrider_self_life_support.h
@@ -46,16 +46,18 @@ struct virtual_overrider_self_life_support {
         = default;
 };
 
-template <typename T, detail::enable_if_t<!std::is_polymorphic<T>::value, int> = 0>
-virtual_overrider_self_life_support *
-dynamic_cast_virtual_overrider_self_life_support_ptr(T * /*raw_type_ptr*/) {
+template <typename To,
+          typename From,
+          detail::enable_if_t<!(std::is_polymorphic<From>::value), int> = 0>
+To *dynamic_raw_ptr_cast_if_possible(From * /*ptr*/) {
     return nullptr;
 }
 
-template <typename T, detail::enable_if_t<std::is_polymorphic<T>::value, int> = 0>
-virtual_overrider_self_life_support *
-dynamic_cast_virtual_overrider_self_life_support_ptr(T *raw_type_ptr) {
-    return dynamic_cast<virtual_overrider_self_life_support *>(raw_type_ptr);
+template <typename To,
+          typename From,
+          detail::enable_if_t<std::is_polymorphic<From>::value, int> = 0>
+To *dynamic_raw_ptr_cast_if_possible(From *ptr) {
+    return dynamic_cast<To *>(ptr);
 }
 
 PYBIND11_NAMESPACE_END(detail)
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 90d37972f7..7ffadcc51b 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -1612,9 +1612,10 @@ class class_ : public detail::generic_type {
 
     // clang-format on
     template <typename T = type,
+              typename A = type_alias,
               detail::enable_if_t<detail::type_uses_smart_holder_type_caster<T>::value, int> = 0>
     static void init_instance(detail::instance *inst, const void *holder_ptr) {
-        detail::type_caster<T>::template init_instance_for_type<type>(inst, holder_ptr, has_alias);
+        detail::type_caster<T>::template init_instance_for_type<T, A>(inst, holder_ptr);
     }
     // clang-format off
 
diff --git a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py
index 9304aa0b7f..09e72172b6 100644
--- a/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py
+++ b/tests/test_class_sh_trampoline_shared_ptr_cpp_arg.py
@@ -58,15 +58,10 @@ def test_shared_ptr_arg_identity():
     del obj
     pytest.gc_collect()
 
-    # python reference is still around since C++ has it
-    assert objref() is not None
-    assert tester.get_object() is objref()
-    assert tester.has_python_instance() is True
-
-    # python reference disappears once the C++ object releases it
-    tester.set_object(None)
-    pytest.gc_collect()
+    # SMART_HOLDER_WIP: the behavior below is DIFFERENT from PR #2839
+    # python reference is gone because it is not an Alias instance
     assert objref() is None
+    assert tester.has_python_instance() is False
 
 
 def test_shared_ptr_alias_nonpython():
@@ -90,7 +85,6 @@ def test_shared_ptr_alias_nonpython():
     assert tester.has_instance() is True
     assert tester.has_python_instance() is False
 
-    # SMART_HOLDER_WIP: the behavior below is DIFFERENT from PR #2839
     # When we pass it as an arg to a new tester the python instance should
     # disappear because it wasn't created with an alias
     new_tester = m.SpBaseTester()
@@ -107,9 +101,9 @@ def test_shared_ptr_alias_nonpython():
 
     # Gone!
     assert tester.has_instance() is True
-    assert tester.has_python_instance() is True  # False in PR #2839
+    assert tester.has_python_instance() is False
     assert new_tester.has_instance() is True
-    assert new_tester.has_python_instance() is True  # False in PR #2839
+    assert new_tester.has_python_instance() is False
 
 
 def test_shared_ptr_goaway():
diff --git a/tests/test_class_sh_with_alias.py b/tests/test_class_sh_with_alias.py
index 2720f9c86d..d93b3b5745 100644
--- a/tests/test_class_sh_with_alias.py
+++ b/tests/test_class_sh_with_alias.py
@@ -40,8 +40,14 @@ def test_drvd0_add_in_cpp_shared_ptr():
 def test_drvd0_add_in_cpp_unique_ptr():
     while True:
         drvd = PyDrvd0(0)
-        with pytest.raises(RuntimeError):
+        with pytest.raises(ValueError) as exc_info:
             m.AddInCppUniquePtr(drvd, 0)
+        assert (
+            str(exc_info.value)
+            == "Alias class (also known as trampoline) does not inherit from"
+            " py::detail::virtual_overrider_self_life_support, therefore the ownership of this"
+            " instance cannot safely be transferred to C++."
+        )
         return  # Comment out for manual leak checking (use `top` command).
 
 

From 5bdaf9647b1d8fad25cde3fbc0c085018dd53ae4 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Thu, 18 Mar 2021 17:45:19 -0700
Subject: [PATCH 4/7] Fixing oversight: adding
 test_class_sh_virtual_py_cpp_mix.cpp to cmake file.

---
 tests/CMakeLists.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index efc548c41c..7388589e53 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -106,6 +106,7 @@ set(PYBIND11_TEST_FILES
     test_class_sh_inheritance.cpp
     test_class_sh_trampoline_shared_ptr_cpp_arg.cpp
     test_class_sh_unique_ptr_member.cpp
+    test_class_sh_virtual_py_cpp_mix.cpp
     test_class_sh_with_alias.cpp
     test_constants_and_functions.cpp
     test_copy_move.cpp

From cac54baa54125a0e6d98efded8f013d54c686c94 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Thu, 18 Mar 2021 20:00:29 -0700
Subject: [PATCH 5/7] clang <= 3.6 compatibility.

---
 tests/test_class_sh_virtual_py_cpp_mix.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp
index e82ec5d854..d11628990d 100644
--- a/tests/test_class_sh_virtual_py_cpp_mix.cpp
+++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp
@@ -11,6 +11,10 @@ class Base {
 public:
     virtual ~Base() = default;
     virtual int get() const { return 101; }
+
+    // Some compilers complain about implicitly defined versions of some of the following:
+    Base()             = default;
+    Base(const Base &) = default;
 };
 
 class CppDerived : public Base {

From 82c1c127b9cc134420344fd48e74e7c3e1139e8b Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 19 Mar 2021 10:47:35 -0700
Subject: [PATCH 6/7] Fixing oversight: dynamic_raw_ptr_cast_if_possible needs
 special handling for To = void. Adding corresponding missing test in
 test_class_sh_virtual_py_cpp_mix. Moving dynamic_raw_ptr_cast_if_possible to
 separate header.

---
 CMakeLists.txt                                |  1 +
 .../detail/dynamic_raw_ptr_cast_if_possible.h | 39 +++++++++++++++++++
 .../detail/smart_holder_type_casters.h        |  1 +
 .../virtual_overrider_self_life_support.h     | 16 --------
 tests/extra_python_package/test_files.py      |  1 +
 tests/test_class_sh_virtual_py_cpp_mix.cpp    |  9 +++++
 tests/test_class_sh_virtual_py_cpp_mix.py     |  3 ++
 7 files changed, 54 insertions(+), 16 deletions(-)
 create mode 100644 include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0a52493b2e..6d74a29716 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -103,6 +103,7 @@ set(PYBIND11_HEADERS
     include/pybind11/detail/class.h
     include/pybind11/detail/common.h
     include/pybind11/detail/descr.h
+    include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h
     include/pybind11/detail/init.h
     include/pybind11/detail/internals.h
     include/pybind11/detail/smart_holder_poc.h
diff --git a/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h b/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h
new file mode 100644
index 0000000000..7c00fe98c1
--- /dev/null
+++ b/include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h
@@ -0,0 +1,39 @@
+// 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.
+
+#pragma once
+
+#include "common.h"
+
+#include <type_traits>
+
+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+PYBIND11_NAMESPACE_BEGIN(detail)
+
+template <typename To, typename From, typename SFINAE = void>
+struct dynamic_raw_ptr_cast_is_possible : std::false_type {};
+
+template <typename To, typename From>
+struct dynamic_raw_ptr_cast_is_possible<
+    To,
+    From,
+    detail::enable_if_t<!std::is_same<To, void>::value && std::is_polymorphic<From>::value>>
+    : std::true_type {};
+
+template <typename To,
+          typename From,
+          detail::enable_if_t<!dynamic_raw_ptr_cast_is_possible<To, From>::value, int> = 0>
+To *dynamic_raw_ptr_cast_if_possible(From * /*ptr*/) {
+    return nullptr;
+}
+
+template <typename To,
+          typename From,
+          detail::enable_if_t<dynamic_raw_ptr_cast_is_possible<To, From>::value, int> = 0>
+To *dynamic_raw_ptr_cast_if_possible(From *ptr) {
+    return dynamic_cast<To *>(ptr);
+}
+
+PYBIND11_NAMESPACE_END(detail)
+PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h
index 49178ad29e..f31f0f42f7 100644
--- a/include/pybind11/detail/smart_holder_type_casters.h
+++ b/include/pybind11/detail/smart_holder_type_casters.h
@@ -8,6 +8,7 @@
 #include "../pytypes.h"
 #include "common.h"
 #include "descr.h"
+#include "dynamic_raw_ptr_cast_if_possible.h"
 #include "internals.h"
 #include "smart_holder_poc.h"
 #include "smart_holder_sfinae_hooks_only.h"
diff --git a/include/pybind11/detail/virtual_overrider_self_life_support.h b/include/pybind11/detail/virtual_overrider_self_life_support.h
index 19a009e450..f5294e9e1f 100644
--- a/include/pybind11/detail/virtual_overrider_self_life_support.h
+++ b/include/pybind11/detail/virtual_overrider_self_life_support.h
@@ -8,8 +8,6 @@
 #include "smart_holder_poc.h"
 #include "type_caster_base.h"
 
-#include <type_traits>
-
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 PYBIND11_NAMESPACE_BEGIN(detail)
 
@@ -46,19 +44,5 @@ struct virtual_overrider_self_life_support {
         = default;
 };
 
-template <typename To,
-          typename From,
-          detail::enable_if_t<!(std::is_polymorphic<From>::value), int> = 0>
-To *dynamic_raw_ptr_cast_if_possible(From * /*ptr*/) {
-    return nullptr;
-}
-
-template <typename To,
-          typename From,
-          detail::enable_if_t<std::is_polymorphic<From>::value, int> = 0>
-To *dynamic_raw_ptr_cast_if_possible(From *ptr) {
-    return dynamic_cast<To *>(ptr);
-}
-
 PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py
index cfa8d9ca4c..1d19001aef 100644
--- a/tests/extra_python_package/test_files.py
+++ b/tests/extra_python_package/test_files.py
@@ -41,6 +41,7 @@
     "include/pybind11/detail/class.h",
     "include/pybind11/detail/common.h",
     "include/pybind11/detail/descr.h",
+    "include/pybind11/detail/dynamic_raw_ptr_cast_if_possible.h",
     "include/pybind11/detail/init.h",
     "include/pybind11/detail/internals.h",
     "include/pybind11/detail/smart_holder_poc.h",
diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp
index d11628990d..7262e31ac2 100644
--- a/tests/test_class_sh_virtual_py_cpp_mix.cpp
+++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp
@@ -17,6 +17,11 @@ class Base {
     Base(const Base &) = default;
 };
 
+class CppDerivedPlain : public Base {
+public:
+    int get() const override { return 202; }
+};
+
 class CppDerived : public Base {
 public:
     int get() const override { return 212; }
@@ -42,6 +47,8 @@ struct CppDerivedVirtualOverrider : CppDerived, py::detail::virtual_overrider_se
 } // 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)
 
 TEST_SUBMODULE(class_sh_virtual_py_cpp_mix, m) {
@@ -49,6 +56,8 @@ TEST_SUBMODULE(class_sh_virtual_py_cpp_mix, m) {
 
     py::classh<Base, BaseVirtualOverrider>(m, "Base").def(py::init<>()).def("get", &Base::get);
 
+    py::classh<CppDerivedPlain, Base>(m, "CppDerivedPlain").def(py::init<>());
+
     py::classh<CppDerived, Base, CppDerivedVirtualOverrider>(m, "CppDerived").def(py::init<>());
 
     m.def("get_from_cpp_plainc_ptr", get_from_cpp_plainc_ptr, py::arg("b"));
diff --git a/tests/test_class_sh_virtual_py_cpp_mix.py b/tests/test_class_sh_virtual_py_cpp_mix.py
index d9d04f2bfc..e85be05fe8 100644
--- a/tests/test_class_sh_virtual_py_cpp_mix.py
+++ b/tests/test_class_sh_virtual_py_cpp_mix.py
@@ -25,6 +25,7 @@ def get(self):
     [
         (m.Base, 101),
         (PyBase, 323),
+        (m.CppDerivedPlain, 202),
         (m.CppDerived, 212),
         (PyCppDerived, 434),
     ],
@@ -39,6 +40,7 @@ def test_base_get(ctor, expected):
     [
         (m.Base, 4101),
         (PyBase, 4323),
+        (m.CppDerivedPlain, 4202),
         (m.CppDerived, 4212),
         (PyCppDerived, 4434),
     ],
@@ -53,6 +55,7 @@ def test_get_from_cpp_plainc_ptr(ctor, expected):
     [
         (m.Base, 5101),
         (PyBase, 5323),
+        (m.CppDerivedPlain, 5202),
         (m.CppDerived, 5212),
         (PyCppDerived, 5434),
     ],

From 0d3f9e2caa05699cb472b009de391a010893a7bc Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Fri, 19 Mar 2021 11:04:10 -0700
Subject: [PATCH 7/7] Changing py::detail::virtual_overrider_self_life_support
 to py::virtual_overrider_self_life_support.

---
 CMakeLists.txt                                     |  4 ++--
 .../pybind11/detail/smart_holder_type_casters.h    |  4 ++--
 .../virtual_overrider_self_life_support.h          | 14 +++++++-------
 tests/extra_python_package/test_files.py           |  2 +-
 tests/test_class_sh_virtual_py_cpp_mix.cpp         |  4 ++--
 tests/test_class_sh_with_alias.cpp                 |  2 +-
 tests/test_class_sh_with_alias.py                  |  2 +-
 7 files changed, 16 insertions(+), 16 deletions(-)
 rename include/pybind11/{detail => }/virtual_overrider_self_life_support.h (89%)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6d74a29716..b698b016da 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -111,7 +111,6 @@ set(PYBIND11_HEADERS
     include/pybind11/detail/smart_holder_type_casters.h
     include/pybind11/detail/type_caster_base.h
     include/pybind11/detail/typeid.h
-    include/pybind11/detail/virtual_overrider_self_life_support.h
     include/pybind11/attr.h
     include/pybind11/buffer_info.h
     include/pybind11/cast.h
@@ -131,7 +130,8 @@ set(PYBIND11_HEADERS
     include/pybind11/pytypes.h
     include/pybind11/smart_holder.h
     include/pybind11/stl.h
-    include/pybind11/stl_bind.h)
+    include/pybind11/stl_bind.h
+    include/pybind11/virtual_overrider_self_life_support.h)
 
 # Compare with grep and warn if mismatched
 if(PYBIND11_MASTER_PROJECT AND NOT CMAKE_VERSION VERSION_LESS 3.12)
diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h
index f31f0f42f7..14b3ea9979 100644
--- a/include/pybind11/detail/smart_holder_type_casters.h
+++ b/include/pybind11/detail/smart_holder_type_casters.h
@@ -6,6 +6,7 @@
 
 #include "../gil.h"
 #include "../pytypes.h"
+#include "../virtual_overrider_self_life_support.h"
 #include "common.h"
 #include "descr.h"
 #include "dynamic_raw_ptr_cast_if_possible.h"
@@ -14,7 +15,6 @@
 #include "smart_holder_sfinae_hooks_only.h"
 #include "type_caster_base.h"
 #include "typeid.h"
-#include "virtual_overrider_self_life_support.h"
 
 #include <cstddef>
 #include <memory>
@@ -394,7 +394,7 @@ struct smart_holder_type_caster_load {
             = dynamic_raw_ptr_cast_if_possible<virtual_overrider_self_life_support>(raw_type_ptr);
         if (self_life_support == nullptr && holder().pointee_depends_on_holder_owner) {
             throw value_error("Alias class (also known as trampoline) does not inherit from "
-                              "py::detail::virtual_overrider_self_life_support, therefore the "
+                              "py::virtual_overrider_self_life_support, therefore the "
                               "ownership of this instance cannot safely be transferred to C++.");
         }
 
diff --git a/include/pybind11/detail/virtual_overrider_self_life_support.h b/include/pybind11/virtual_overrider_self_life_support.h
similarity index 89%
rename from include/pybind11/detail/virtual_overrider_self_life_support.h
rename to include/pybind11/virtual_overrider_self_life_support.h
index f5294e9e1f..b4c2437d0f 100644
--- a/include/pybind11/detail/virtual_overrider_self_life_support.h
+++ b/include/pybind11/virtual_overrider_self_life_support.h
@@ -4,22 +4,23 @@
 
 #pragma once
 
-#include "common.h"
-#include "smart_holder_poc.h"
-#include "type_caster_base.h"
+#include "detail/common.h"
+#include "detail/smart_holder_poc.h"
+#include "detail/type_caster_base.h"
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
-PYBIND11_NAMESPACE_BEGIN(detail)
 
+PYBIND11_NAMESPACE_BEGIN(detail)
 // SMART_HOLDER_WIP: Needs refactoring of existing pybind11 code.
 inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo);
+PYBIND11_NAMESPACE_END(detail)
 
 // The original core idea for this struct goes back to PyCLIF:
 // https://github.com/google/clif/blob/07f95d7e69dca2fcf7022978a55ef3acff506c19/clif/python/runtime.cc#L37
 // URL provided here mainly to give proper credit. To fully explain the `HoldPyObj` feature, more
 // context is needed (SMART_HOLDER_WIP).
 struct virtual_overrider_self_life_support {
-    value_and_holder loaded_v_h;
+    detail::value_and_holder loaded_v_h;
     ~virtual_overrider_self_life_support() {
         if (loaded_v_h.inst != nullptr && loaded_v_h.vh != nullptr) {
             void *value_void_ptr = loaded_v_h.value_ptr();
@@ -28,7 +29,7 @@ struct virtual_overrider_self_life_support {
                 Py_DECREF((PyObject *) loaded_v_h.inst);
                 loaded_v_h.value_ptr() = nullptr;
                 loaded_v_h.holder<pybindit::memory::smart_holder>().release_disowned();
-                deregister_instance(loaded_v_h.inst, value_void_ptr, loaded_v_h.type);
+                detail::deregister_instance(loaded_v_h.inst, value_void_ptr, loaded_v_h.type);
                 PyGILState_Release(threadstate);
             }
         }
@@ -44,5 +45,4 @@ struct virtual_overrider_self_life_support {
         = default;
 };
 
-PYBIND11_NAMESPACE_END(detail)
 PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
diff --git a/tests/extra_python_package/test_files.py b/tests/extra_python_package/test_files.py
index 1d19001aef..7d2cfd8142 100644
--- a/tests/extra_python_package/test_files.py
+++ b/tests/extra_python_package/test_files.py
@@ -35,6 +35,7 @@
     "include/pybind11/smart_holder.h",
     "include/pybind11/stl.h",
     "include/pybind11/stl_bind.h",
+    "include/pybind11/virtual_overrider_self_life_support.h",
 }
 
 detail_headers = {
@@ -49,7 +50,6 @@
     "include/pybind11/detail/smart_holder_type_casters.h",
     "include/pybind11/detail/type_caster_base.h",
     "include/pybind11/detail/typeid.h",
-    "include/pybind11/detail/virtual_overrider_self_life_support.h",
 }
 
 cmake_files = {
diff --git a/tests/test_class_sh_virtual_py_cpp_mix.cpp b/tests/test_class_sh_virtual_py_cpp_mix.cpp
index 7262e31ac2..74d2db4176 100644
--- a/tests/test_class_sh_virtual_py_cpp_mix.cpp
+++ b/tests/test_class_sh_virtual_py_cpp_mix.cpp
@@ -31,13 +31,13 @@ int get_from_cpp_plainc_ptr(const Base *b) { return b->get() + 4000; }
 
 int get_from_cpp_unique_ptr(std::unique_ptr<Base> b) { return b->get() + 5000; }
 
-struct BaseVirtualOverrider : Base, py::detail::virtual_overrider_self_life_support {
+struct BaseVirtualOverrider : Base, py::virtual_overrider_self_life_support {
     using Base::Base;
 
     int get() const override { PYBIND11_OVERRIDE(int, Base, get); }
 };
 
-struct CppDerivedVirtualOverrider : CppDerived, py::detail::virtual_overrider_self_life_support {
+struct CppDerivedVirtualOverrider : CppDerived, py::virtual_overrider_self_life_support {
     using CppDerived::CppDerived;
 
     int get() const override { PYBIND11_OVERRIDE(int, CppDerived, get); }
diff --git a/tests/test_class_sh_with_alias.cpp b/tests/test_class_sh_with_alias.cpp
index 37372f116e..846533ddfc 100644
--- a/tests/test_class_sh_with_alias.cpp
+++ b/tests/test_class_sh_with_alias.cpp
@@ -35,7 +35,7 @@ struct AbaseAlias : Abase<SerNo> {
 };
 
 template <>
-struct AbaseAlias<1> : Abase<1>, py::detail::virtual_overrider_self_life_support {
+struct AbaseAlias<1> : Abase<1>, py::virtual_overrider_self_life_support {
     using Abase<1>::Abase;
 
     int Add(int other_val) const override {
diff --git a/tests/test_class_sh_with_alias.py b/tests/test_class_sh_with_alias.py
index d93b3b5745..4650eaa5e1 100644
--- a/tests/test_class_sh_with_alias.py
+++ b/tests/test_class_sh_with_alias.py
@@ -45,7 +45,7 @@ def test_drvd0_add_in_cpp_unique_ptr():
         assert (
             str(exc_info.value)
             == "Alias class (also known as trampoline) does not inherit from"
-            " py::detail::virtual_overrider_self_life_support, therefore the ownership of this"
+            " py::virtual_overrider_self_life_support, therefore the ownership of this"
             " instance cannot safely be transferred to C++."
         )
         return  # Comment out for manual leak checking (use `top` command).