fixup! fixup! Replace global map holders_seen with local holder_type in registered type_info

Runtime costs of checking holder compatibility for function return types during can be limited to
definition time (instead of calling time), if we require types to be registered before defining a function.
This allows to lookup the type_info record and validate the holder type as expected.
As we cannot call functions with an unregistered return type anyway, I think this is a good compromise.

Author: rhaschke

include/pybind11/cast.h

@@ -1492,13 +1492,13 @@ struct holder_helper {
 };
 
 template <typename holder>
-void check_for_holder_mismatch_impl() {
+bool check_for_holder_mismatch_impl(bool throw_if_missing = true) {
     using iholder = intrinsic_t<holder>;
     using base_type = decltype(*holder_helper<iholder>::get(std::declval<iholder>()));
     auto &holder_typeinfo = typeid(iholder);
-    auto base_info = detail::get_type_info(typeid(base_type));
+    auto base_info = detail::get_type_info(typeid(base_type), throw_if_missing);
     if (!base_info)
-        return;  // Don't complain if we see this type the first time
+        return false;  // Return false if the type is not yet registered
 
     auto debug = type_id<base_type>();
     if (!same_type(*base_info->holder_type, holder_typeinfo)) {
@@ -1512,6 +1512,7 @@ void check_for_holder_mismatch_impl() {
                 " was declared using holder type " + holder_name);
 #endif
     }
+    return true;
 }
 
 /// Type caster for holder types like std::shared_ptr, etc.
@@ -1538,7 +1539,6 @@ struct copyable_holder_caster : public type_caster_base<type> {
     explicit operator holder_type&() { return holder; }
 
     static handle cast(const holder_type &src, return_value_policy, handle) {
-        check_for_holder_mismatch_impl<holder_type>();
         const auto *ptr = holder_helper<holder_type>::get(src);
         return type_caster_base<type>::cast_holder(ptr, &src);
     }
@@ -1635,10 +1635,10 @@ template <typename holder> using is_holder = any_of<
     is_template_base_of<copyable_holder_caster, make_caster<holder>>>;
 
 template <typename holder>
-void check_for_holder_mismatch(enable_if_t<!is_holder<holder>::value, int> = 0) {}
+bool check_for_holder_mismatch(enable_if_t<!is_holder<holder>::value, int> = 0) { return true; }
 template <typename holder>
-void check_for_holder_mismatch(enable_if_t<is_holder<holder>::value, int> = 0) {
-    check_for_holder_mismatch_impl<holder>();
+bool check_for_holder_mismatch(enable_if_t<is_holder<holder>::value, int> = 0) {
+    return check_for_holder_mismatch_impl<holder>(false);
 }
 
 template <typename T> struct handle_type_name { static constexpr auto name = _<T>(); };

include/pybind11/pybind11.h

@@ -158,7 +158,12 @@ class cpp_function : public function {
                       "The number of argument annotations does not match the number of function arguments");
 
         // Fail if the type was previously registered with a different holder type
-        detail::check_for_holder_mismatch<Return>();
+        if (!detail::check_for_holder_mismatch<Return>()) {
+            // If Return type was not yet registered, check_for_holder_mismatch() returns false w/o raising
+            std::string tname(typeid(Return).name());
+            detail::clean_type_id(tname);
+            pybind11_fail("Cannot register function with not yet registered return type \"" + tname + "\"");
+        }
 
         /* Dispatch code which converts function arguments and performs the actual function call */
         rec->impl = [](function_call &call) -> handle {

tests/test_smart_ptr.cpp

@@ -377,7 +377,15 @@ TEST_SUBMODULE(smart_ptr, m) {
         // Fails: the class was already registered with a shared_ptr holder
         m.def("bad1", []() { return std::unique_ptr<HeldByShared>(new HeldByShared()); });
     });
-    m.def("return_shared", []() { return std::make_shared<HeldByUnique>(); });
+    m.def("register_mismatch_class", [](py::module m) {
+        // Fails: the class was already registered with a shared_ptr holder
+        py::class_<HeldByShared, std::unique_ptr<HeldByShared>>(m, "bad");
+    });
+
+    m.def("register_return_shared", [](py::module m) {
+        // Fails if HeldByUnique is not yet registered or, if registered, due to mismatching holder
+        m.def("return_shared", []() { return std::make_shared<HeldByUnique>(); });
+    });
     m.def("register_HeldByUnique", [](py::module m) {
         py::class_<HeldByUnique>(m, "HeldByUnique");
     });

tests/test_smart_ptr.py

@@ -312,12 +312,16 @@ def test_holder_mismatch():
     with pytest.raises(RuntimeError) as excinfo:
         m.register_mismatch_return(m)
     assert "Mismatched holders detected" in str(excinfo)
+    with pytest.raises(RuntimeError) as excinfo:
+        m.register_mismatch_class(m)
+    assert "is already registered" in str(excinfo)
 
-    with pytest.raises(TypeError) as excinfo:
-        m.return_shared()  # calling a function returning an unregistered type
+    with pytest.raises(RuntimeError) as excinfo:
+        m.register_return_shared(m)  # Cannot register function returning an unregistered type
+    assert "Cannot register function with not yet registered return type" in str(excinfo)
     m.register_HeldByUnique(m)  # register the type
     with pytest.raises(RuntimeError) as excinfo:
-        m.return_shared()  # calling a function returning a mismatching holder type
+        m.register_return_shared(m)  # Cannot register function returning a mismatching holder type
     assert "Mismatched holders detected" in str(excinfo)
 
     with pytest.raises(RuntimeError) as excinfo: