From bf54ecdf9ced78819f1bc4851241c314fe4b8677 Mon Sep 17 00:00:00 2001
From: "Ralf W. Grosse-Kunstleve" <rwgk@google.com>
Date: Sun, 25 Aug 2024 22:41:16 +0700
Subject: [PATCH] Split out non-functional changes from
 https://github.com/pybind/pybind11/pull/5332 (#5333)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PREPARATION for:

PR #5332 — Fix handling of const unique_ptr<T, D> & (do not disown).

Splitting out so that the functional changes under PR #5332 will be more obvious.
---
 include/pybind11/cast.h                          | 16 ++++++++--------
 ...test_class_sh_trampoline_shared_from_this.cpp |  6 ++++--
 .../test_class_sh_trampoline_shared_from_this.py |  3 ++-
 3 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index 0262296aa..0cba8a128 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -871,14 +871,14 @@ public:
             pybind11_fail("Passing `std::shared_ptr<T> *` from Python to C++ is not supported "
                           "(inherently unsafe).");
         }
-        return std::addressof(shared_ptr_holder);
+        return std::addressof(shared_ptr_storage);
     }
 
     explicit operator std::shared_ptr<type> &() {
         if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
-            shared_ptr_holder = sh_load_helper.load_as_shared_ptr(value);
+            shared_ptr_storage = sh_load_helper.load_as_shared_ptr(value);
         }
-        return shared_ptr_holder;
+        return shared_ptr_storage;
     }
 
     static handle
@@ -924,7 +924,7 @@ protected:
         }
         if (v_h.holder_constructed()) {
             value = v_h.value_ptr();
-            shared_ptr_holder = v_h.template holder<std::shared_ptr<type>>();
+            shared_ptr_storage = v_h.template holder<std::shared_ptr<type>>();
             return;
         }
         throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
@@ -953,8 +953,8 @@ protected:
                 if (typeinfo->holder_enum_v == detail::holder_enum_t::smart_holder) {
                     sh_load_helper.loaded_v_h = sub_caster.sh_load_helper.loaded_v_h;
                 } else {
-                    shared_ptr_holder
-                        = std::shared_ptr<type>(sub_caster.shared_ptr_holder, (type *) value);
+                    shared_ptr_storage
+                        = std::shared_ptr<type>(sub_caster.shared_ptr_storage, (type *) value);
                 }
                 return true;
             }
@@ -964,8 +964,8 @@ protected:
 
     static bool try_direct_conversions(handle) { return false; }
 
-    std::shared_ptr<type> shared_ptr_holder;
     smart_holder_type_caster_support::load_helper<remove_cv_t<type>> sh_load_helper; // Const2Mutbl
+    std::shared_ptr<type> shared_ptr_storage;
 };
 
 #endif // PYBIND11_HAS_INTERNALS_WITH_SMART_HOLDER_SUPPORT
@@ -1040,7 +1040,7 @@ public:
             policy = return_value_policy::reference_internal;
         }
         if (policy != return_value_policy::reference_internal) {
-            throw cast_error("Invalid return_value_policy for unique_ptr&");
+            throw cast_error("Invalid return_value_policy for const unique_ptr&");
         }
         return type_caster_base<type>::cast(src.get(), policy, parent);
     }
diff --git a/tests/test_class_sh_trampoline_shared_from_this.cpp b/tests/test_class_sh_trampoline_shared_from_this.cpp
index e5d688af6..f664cac51 100644
--- a/tests/test_class_sh_trampoline_shared_from_this.cpp
+++ b/tests/test_class_sh_trampoline_shared_from_this.cpp
@@ -87,7 +87,9 @@ long pass_shared_ptr(const std::shared_ptr<Sft> &obj) {
     return sft.use_count();
 }
 
-void pass_unique_ptr(const std::unique_ptr<Sft> &) {}
+void pass_unique_ptr_cref(const std::unique_ptr<Sft> &) {
+    throw std::runtime_error("Expected to not be reached.");
+}
 
 Sft *make_pure_cpp_sft_raw_ptr(const std::string &history_seed) { return new Sft{history_seed}; }
 
@@ -135,7 +137,7 @@ TEST_SUBMODULE(class_sh_trampoline_shared_from_this, m) {
 
     m.def("use_count", use_count);
     m.def("pass_shared_ptr", pass_shared_ptr);
-    m.def("pass_unique_ptr", pass_unique_ptr);
+    m.def("pass_unique_ptr_cref", pass_unique_ptr_cref);
     m.def("make_pure_cpp_sft_raw_ptr", make_pure_cpp_sft_raw_ptr);
     m.def("make_pure_cpp_sft_unq_ptr", make_pure_cpp_sft_unq_ptr);
     m.def("make_pure_cpp_sft_shd_ptr", make_pure_cpp_sft_shd_ptr);
diff --git a/tests/test_class_sh_trampoline_shared_from_this.py b/tests/test_class_sh_trampoline_shared_from_this.py
index a074b20a1..b112bb5d9 100644
--- a/tests/test_class_sh_trampoline_shared_from_this.py
+++ b/tests/test_class_sh_trampoline_shared_from_this.py
@@ -138,10 +138,11 @@ def test_pass_released_shared_ptr_as_unique_ptr():
     stash1 = m.SftSharedPtrStash(1)
     stash1.Add(obj)  # Releases shared_ptr to C++.
     with pytest.raises(ValueError) as exc_info:
-        m.pass_unique_ptr(obj)
+        m.pass_unique_ptr_cref(obj)
     assert str(exc_info.value) == (
         "Python instance is currently owned by a std::shared_ptr."
     )
+    assert obj.history == "PySft_Stash1Add"
 
 
 @pytest.mark.parametrize(