Skip to content

Commit

Permalink
Bug fixes: Add missing handle_type_name specializations. (#5073)
Browse files Browse the repository at this point in the history
* Transfer bug fixes from #4888 wholesale. Full test coverage for all fixes is still missing.

* Add cmake option(PYBIND11_DISABLE_HANDLE_TYPE_NAME_DEFAULT_IMPLEMENTATION) and use in some tests.
  • Loading branch information
rwgk committed Mar 27, 2024
1 parent 705efcc commit 0efff79
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 5 deletions.
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions CMakeLists.txt
Expand Up @@ -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
Expand All @@ -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()
Expand Down
95 changes: 95 additions & 0 deletions include/pybind11/cast.h
Expand Up @@ -881,10 +881,53 @@ struct is_holder_type
template <typename base, typename deleter>
struct is_holder_type<base, std::unique_ptr<base, deleter>> : 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 <typename T>
struct handle_type_name;

#else

template <typename T>
struct handle_type_name {
static constexpr auto name = const_name<T>();
};

#endif

template <>
struct handle_type_name<object> {
static constexpr auto name = const_name("object");
};
template <>
struct handle_type_name<list> {
static constexpr auto name = const_name("list");
};
template <>
struct handle_type_name<dict> {
static constexpr auto name = const_name("dict");
};
template <>
struct handle_type_name<anyset> {
static constexpr auto name = const_name("Union[set, frozenset]");
};
template <>
struct handle_type_name<set> {
static constexpr auto name = const_name("set");
};
template <>
struct handle_type_name<frozenset> {
static constexpr auto name = const_name("frozenset");
};
template <>
struct handle_type_name<str> {
static constexpr auto name = const_name("str");
};
template <>
struct handle_type_name<tuple> {
static constexpr auto name = const_name("tuple");
};
template <>
struct handle_type_name<bool_> {
static constexpr auto name = const_name("bool");
Expand Down Expand Up @@ -930,13 +973,65 @@ struct handle_type_name<sequence> {
static constexpr auto name = const_name("Sequence");
};
template <>
struct handle_type_name<bytearray> {
static constexpr auto name = const_name("bytearray");
};
template <>
struct handle_type_name<memoryview> {
static constexpr auto name = const_name("memoryview");
};
template <>
struct handle_type_name<slice> {
static constexpr auto name = const_name("slice");
};
template <>
struct handle_type_name<type> {
static constexpr auto name = const_name("type");
};
template <>
struct handle_type_name<capsule> {
static constexpr auto name = const_name("capsule");
};
template <>
struct handle_type_name<ellipsis> {
static constexpr auto name = const_name("ellipsis");
};
template <>
struct handle_type_name<weakref> {
static constexpr auto name = const_name("weakref");
};
template <>
struct handle_type_name<args> {
static constexpr auto name = const_name("*args");
};
template <>
struct handle_type_name<kwargs> {
static constexpr auto name = const_name("**kwargs");
};
template <>
struct handle_type_name<obj_attr_accessor> {
static constexpr auto name = const_name<obj_attr_accessor>();
};
template <>
struct handle_type_name<str_attr_accessor> {
static constexpr auto name = const_name<str_attr_accessor>();
};
template <>
struct handle_type_name<item_accessor> {
static constexpr auto name = const_name<item_accessor>();
};
template <>
struct handle_type_name<sequence_accessor> {
static constexpr auto name = const_name<sequence_accessor>();
};
template <>
struct handle_type_name<list_accessor> {
static constexpr auto name = const_name<list_accessor>();
};
template <>
struct handle_type_name<tuple_accessor> {
static constexpr auto name = const_name<tuple_accessor>();
};

template <typename type>
struct pyobject_caster {
Expand Down
6 changes: 5 additions & 1 deletion include/pybind11/detail/type_caster_base.h
Expand Up @@ -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<std::string>() + '.'
+ th.attr("__qualname__").cast<std::string>();
}
return clean_type_id(ti.name());
return quote_cpp_type_name(clean_type_id(ti.name()));
}

PYBIND11_NAMESPACE_END(detail)
Expand Down
6 changes: 6 additions & 0 deletions include/pybind11/numpy.h
Expand Up @@ -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<dtype> {
static constexpr auto name = const_name("numpy.dtype");
};

template <>
struct handle_type_name<array> {
static constexpr auto name = const_name("numpy.ndarray");
Expand Down
27 changes: 24 additions & 3 deletions include/pybind11/pybind11.h
Expand Up @@ -492,9 +492,7 @@ class cpp_function : public function {
signature += rec->scope.attr("__module__").cast<std::string>() + "."
+ rec->scope.attr("__qualname__").cast<std::string>();
} 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;
Expand Down Expand Up @@ -1192,6 +1190,15 @@ class cpp_function : public function {
}
};

PYBIND11_NAMESPACE_BEGIN(detail)

template <>
struct handle_type_name<cpp_function> {
static constexpr auto name = const_name("Callable");
};

PYBIND11_NAMESPACE_END(detail)

/// Wrapper for Python extension modules
class module_ : public object {
public:
Expand Down Expand Up @@ -1319,6 +1326,15 @@ class module_ : public object {
}
};

PYBIND11_NAMESPACE_BEGIN(detail)

template <>
struct handle_type_name<module_> {
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.
Expand Down Expand Up @@ -2611,6 +2627,11 @@ class exception : public object {

PYBIND11_NAMESPACE_BEGIN(detail)

template <>
struct handle_type_name<exception<void>> {
static constexpr auto name = const_name("Exception");
};

// Helper function for register_exception and register_local_exception
template <typename CppException>
exception<CppException> &
Expand Down
1 change: 1 addition & 0 deletions include/pybind11/pytypes.h
Expand Up @@ -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<accessor_policies::obj_attr>;
using str_attr_accessor = accessor<accessor_policies::str_attr>;
using item_accessor = accessor<accessor_policies::generic_item>;
Expand Down
3 changes: 3 additions & 0 deletions tests/test_exceptions.cpp
Expand Up @@ -382,4 +382,7 @@ TEST_SUBMODULE(exceptions, m) {
// function returns None instead of int, should give a useful error message
fn().cast<int>();
});

// m.def("pass_exception_void", [](const py::exception<void>&) {}); // Does not compile.
m.def("return_exception_void", []() { return py::exception<void>(); });
}
6 changes: 6 additions & 0 deletions tests/test_exceptions.py
Expand Up @@ -424,3 +424,9 @@ def test_fn_cast_int_exception():
assert str(excinfo.value).startswith(
"Unable to cast Python instance of type <class 'NoneType'> to C++ type"
)


def test_return_exception_void():
with pytest.raises(TypeError) as excinfo:
m.return_exception_void()
assert "Exception" in str(excinfo.value)
9 changes: 9 additions & 0 deletions tests/test_pytypes.cpp
Expand Up @@ -41,6 +41,15 @@ class float_ : public py::object {
};
} // namespace external

namespace pybind11 {
namespace detail {
template <>
struct handle_type_name<external::float_> {
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); }
Expand Down
2 changes: 1 addition & 1 deletion tests/test_pytypes.py
Expand Up @@ -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):
Expand Down

0 comments on commit 0efff79

Please sign in to comment.