From 07e6acb432bfa8f3f1a2da60ae861ae6f732be54 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Tue, 26 Mar 2024 23:58:07 -0700 Subject: [PATCH 1/2] Transfer bug fixes from #4888 wholesale. Full test coverage for all fixes is still missing. --- include/pybind11/cast.h | 95 ++++++++++++++++++++++ include/pybind11/detail/type_caster_base.h | 6 +- include/pybind11/numpy.h | 6 ++ include/pybind11/pybind11.h | 27 +++++- include/pybind11/pytypes.h | 1 + tests/test_exceptions.cpp | 3 + tests/test_exceptions.py | 6 ++ tests/test_pytypes.cpp | 9 ++ tests/test_pytypes.py | 2 +- 9 files changed, 150 insertions(+), 5 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 3f3f966d08..ae207758bf 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -881,10 +881,53 @@ struct is_holder_type template struct is_holder_type> : std::true_type {}; +#ifdef PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION // See PR #4888 + +// This leads to compilation errors if a specialization is missing. +template +struct handle_type_name; + +#else + template struct handle_type_name { static constexpr auto name = const_name(); }; + +#endif + +template <> +struct handle_type_name { + static constexpr auto name = const_name("object"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("list"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("dict"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("Union[set, frozenset]"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("set"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("frozenset"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("str"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("tuple"); +}; template <> struct handle_type_name { static constexpr auto name = const_name("bool"); @@ -930,6 +973,34 @@ struct handle_type_name { static constexpr auto name = const_name("Sequence"); }; template <> +struct handle_type_name { + static constexpr auto name = const_name("bytearray"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("memoryview"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("slice"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("type"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("capsule"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("ellipsis"); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name("weakref"); +}; +template <> struct handle_type_name { static constexpr auto name = const_name("*args"); }; @@ -937,6 +1008,30 @@ template <> struct handle_type_name { static constexpr auto name = const_name("**kwargs"); }; +template <> +struct handle_type_name { + static constexpr auto name = const_name(); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name(); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name(); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name(); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name(); +}; +template <> +struct handle_type_name { + static constexpr auto name = const_name(); +}; template struct pyobject_caster { diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 476646ee8f..518d3107ba 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -1201,13 +1201,17 @@ class type_caster_base : public type_caster_generic { static Constructor make_move_constructor(...) { return nullptr; } }; +inline std::string quote_cpp_type_name(const std::string &cpp_type_name) { + return cpp_type_name; // No-op for now. See PR #4888 +} + PYBIND11_NOINLINE std::string type_info_description(const std::type_info &ti) { if (auto *type_data = get_type_info(ti)) { handle th((PyObject *) type_data->type); return th.attr("__module__").cast() + '.' + th.attr("__qualname__").cast(); } - return clean_type_id(ti.name()); + return quote_cpp_type_name(clean_type_id(ti.name())); } PYBIND11_NAMESPACE_END(detail) diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 12a0490e29..4eb1e214ea 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -46,10 +46,16 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_WARNING_DISABLE_MSVC(4127) +class dtype; // Forward declaration class array; // Forward declaration PYBIND11_NAMESPACE_BEGIN(detail) +template <> +struct handle_type_name { + static constexpr auto name = const_name("numpy.dtype"); +}; + template <> struct handle_type_name { static constexpr auto name = const_name("numpy.ndarray"); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index b87fe66b27..429d2138d1 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -492,9 +492,7 @@ class cpp_function : public function { signature += rec->scope.attr("__module__").cast() + "." + rec->scope.attr("__qualname__").cast(); } else { - std::string tname(t->name()); - detail::clean_type_id(tname); - signature += tname; + signature += detail::quote_cpp_type_name(detail::clean_type_id(t->name())); } } else { signature += c; @@ -1192,6 +1190,15 @@ class cpp_function : public function { } }; +PYBIND11_NAMESPACE_BEGIN(detail) + +template <> +struct handle_type_name { + static constexpr auto name = const_name("Callable"); +}; + +PYBIND11_NAMESPACE_END(detail) + /// Wrapper for Python extension modules class module_ : public object { public: @@ -1319,6 +1326,15 @@ class module_ : public object { } }; +PYBIND11_NAMESPACE_BEGIN(detail) + +template <> +struct handle_type_name { + static constexpr auto name = const_name("module"); +}; + +PYBIND11_NAMESPACE_END(detail) + // When inside a namespace (or anywhere as long as it's not the first item on a line), // C++20 allows "module" to be used. This is provided for backward compatibility, and for // simplicity, if someone wants to use py::module for example, that is perfectly safe. @@ -2611,6 +2627,11 @@ class exception : public object { PYBIND11_NAMESPACE_BEGIN(detail) +template <> +struct handle_type_name> { + static constexpr auto name = const_name("Exception"); +}; + // Helper function for register_exception and register_local_exception template exception & diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index e7c7c8124d..d5f6af8e02 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -59,6 +59,7 @@ struct sequence_item; struct list_item; struct tuple_item; } // namespace accessor_policies +// PLEASE KEEP handle_type_name SPECIALIZATIONS IN SYNC. using obj_attr_accessor = accessor; using str_attr_accessor = accessor; using item_accessor = accessor; diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index d3af7ab728..c1d05bb241 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -382,4 +382,7 @@ TEST_SUBMODULE(exceptions, m) { // function returns None instead of int, should give a useful error message fn().cast(); }); + + // m.def("pass_exception_void", [](const py::exception&) {}); // Does not compile. + m.def("return_exception_void", []() { return py::exception(); }); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 6752285acd..01fcb918ec 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -424,3 +424,9 @@ def test_fn_cast_int_exception(): assert str(excinfo.value).startswith( "Unable to cast Python instance of type to C++ type" ) + + +def test_return_exception_void(): + with pytest.raises(TypeError) as excinfo: + m.return_exception_void() + assert "Exception" in str(excinfo.value) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index e840eb61f8..12e6151812 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -41,6 +41,15 @@ class float_ : public py::object { }; } // namespace external +namespace pybind11 { +namespace detail { +template <> +struct handle_type_name { + static constexpr auto name = const_name("float"); +}; +} // namespace detail +} // namespace pybind11 + namespace implicit_conversion_from_0_to_handle { // Uncomment to trigger compiler error. Note: Before PR #4008 this used to compile successfully. // void expected_to_trigger_compiler_error() { py::handle(0); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index cfb144523a..f6271a628a 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -121,7 +121,7 @@ def test_set(capture, doc): assert m.anyset_contains({"foo"}, "foo") assert doc(m.get_set) == "get_set() -> set" - assert doc(m.print_anyset) == "print_anyset(arg0: anyset) -> None" + assert doc(m.print_anyset) == "print_anyset(arg0: Union[set, frozenset]) -> None" def test_frozenset(capture, doc): From 5f4d0b15d99bd2cc88ec8738af75047d05ae2720 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 27 Mar 2024 00:22:34 -0700 Subject: [PATCH 2/2] Add cmake option(PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION) and use in some tests. --- .github/workflows/ci.yml | 1 + CMakeLists.txt | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd44c1faaf..5cc6c3515e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,6 +114,7 @@ jobs: run: > cmake -S . -B . -DPYBIND11_WERROR=ON + -DPYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION=ON -DPYBIND11_SIMPLE_GIL_MANAGEMENT=ON -DPYBIND11_NUMPY_1_ONLY=ON -DDOWNLOAD_CATCH=ON diff --git a/CMakeLists.txt b/CMakeLists.txt index 890440dc17..7db1bf668f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -107,6 +107,8 @@ endif() option(PYBIND11_INSTALL "Install pybind11 header files?" ${PYBIND11_MASTER_PROJECT}) option(PYBIND11_TEST "Build pybind11 test suite?" ${PYBIND11_MASTER_PROJECT}) option(PYBIND11_NOPYTHON "Disable search for Python" OFF) +option(PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION + "To enforce that a handle_type_name<> specialization exists" OFF) option(PYBIND11_SIMPLE_GIL_MANAGEMENT "Use simpler GIL management logic that does not support disassociation" OFF) option(PYBIND11_NUMPY_1_ONLY @@ -115,6 +117,9 @@ set(PYBIND11_INTERNALS_VERSION "" CACHE STRING "Override the ABI version, may be used to enable the unstable ABI.") +if(PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION) + add_compile_definitions(PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION) +endif() if(PYBIND11_SIMPLE_GIL_MANAGEMENT) add_compile_definitions(PYBIND11_SIMPLE_GIL_MANAGEMENT) endif()