Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug fixes: Add missing handle_type_name specializations. #5073

Merged
merged 2 commits into from Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)
henryiii marked this conversation as resolved.
Show resolved Hide resolved
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