From 8dcced29ae4c7654c25bc50742a72556ee3ae36c Mon Sep 17 00:00:00 2001 From: Dustin Spicuzza Date: Fri, 10 Feb 2023 00:21:17 -0500 Subject: [PATCH] Always display python type information in cast errors (#4463) * Always display python type information in cast errors * Address comments * Update comment --- include/pybind11/cast.h | 33 ++++++++++++++++++-------------- include/pybind11/detail/common.h | 5 +++-- tests/test_callbacks.py | 14 ++++++++++++-- tests/test_exceptions.cpp | 5 +++++ tests/test_exceptions.py | 9 +++++++++ tests/test_pytypes.py | 2 +- 6 files changed, 49 insertions(+), 19 deletions(-) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 3a4046027..9b013bc39 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -1017,11 +1017,14 @@ type_caster &load_type(type_caster &conv, const handle &ha "Internal error: type_caster should only be used for C++ types"); if (!conv.load(handle, true)) { #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) - throw cast_error("Unable to cast Python instance to C++ type (#define " - "PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)"); + throw cast_error( + "Unable to cast Python instance of type " + + str(type::handle_of(handle)).cast() + + " to C++ type '?' (#define " + "PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)"); #else throw cast_error("Unable to cast Python instance of type " - + (std::string) str(type::handle_of(handle)) + " to C++ type '" + + str(type::handle_of(handle)).cast() + " to C++ type '" + type_id() + "'"); #endif } @@ -1085,12 +1088,13 @@ detail::enable_if_t::value, T> move(object &&obj) { if (obj.ref_count() > 1) { #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) throw cast_error( - "Unable to cast Python instance to C++ rvalue: instance has multiple references" - " (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)"); + "Unable to cast Python " + str(type::handle_of(obj)).cast() + + " instance to C++ rvalue: instance has multiple references" + " (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)"); #else - throw cast_error("Unable to move from Python " + (std::string) str(type::handle_of(obj)) - + " instance to C++ " + type_id() - + " instance: instance has multiple references"); + throw cast_error("Unable to move from Python " + + str(type::handle_of(obj)).cast() + " instance to C++ " + + type_id() + " instance: instance has multiple references"); #endif } @@ -1195,9 +1199,10 @@ PYBIND11_NAMESPACE_END(detail) // The overloads could coexist, i.e. the #if is not strictly speaking needed, // but it is an easy minor optimization. #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) -inline cast_error cast_error_unable_to_convert_call_arg() { - return cast_error("Unable to convert call argument to Python object (#define " - "PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)"); +inline cast_error cast_error_unable_to_convert_call_arg(const std::string &name) { + return cast_error("Unable to convert call argument '" + name + + "' to Python object (#define " + "PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)"); } #else inline cast_error cast_error_unable_to_convert_call_arg(const std::string &name, @@ -1220,7 +1225,7 @@ tuple make_tuple(Args &&...args_) { for (size_t i = 0; i < args.size(); i++) { if (!args[i]) { #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) - throw cast_error_unable_to_convert_call_arg(); + throw cast_error_unable_to_convert_call_arg(std::to_string(i)); #else std::array argtypes{{type_id()...}}; throw cast_error_unable_to_convert_call_arg(std::to_string(i), argtypes[i]); @@ -1510,7 +1515,7 @@ private: detail::make_caster::cast(std::forward(x), policy, {})); if (!o) { #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) - throw cast_error_unable_to_convert_call_arg(); + throw cast_error_unable_to_convert_call_arg(std::to_string(args_list.size())); #else throw cast_error_unable_to_convert_call_arg(std::to_string(args_list.size()), type_id()); @@ -1542,7 +1547,7 @@ private: } if (!a.value) { #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) - throw cast_error_unable_to_convert_call_arg(); + throw cast_error_unable_to_convert_call_arg(a.name); #else throw cast_error_unable_to_convert_call_arg(a.name, a.type); #endif diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 0fc81e053..1032293d2 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1225,8 +1225,9 @@ constexpr #endif // Pybind offers detailed error messages by default for all builts that are debug (through the -// negation of ndebug). This can also be manually enabled by users, for any builds, through -// defining PYBIND11_DETAILED_ERROR_MESSAGES. +// negation of NDEBUG). This can also be manually enabled by users, for any builds, through +// defining PYBIND11_DETAILED_ERROR_MESSAGES. This information is primarily useful for those +// who are writing (as opposed to merely using) libraries that use pybind11. #if !defined(PYBIND11_DETAILED_ERROR_MESSAGES) && !defined(NDEBUG) # define PYBIND11_DETAILED_ERROR_MESSAGES #endif diff --git a/tests/test_callbacks.py b/tests/test_callbacks.py index 57b659988..4a652f53e 100644 --- a/tests/test_callbacks.py +++ b/tests/test_callbacks.py @@ -5,6 +5,7 @@ import pytest import env # noqa: F401 from pybind11_tests import callbacks as m +from pybind11_tests import detailed_error_messages_enabled def test_callbacks(): @@ -70,11 +71,20 @@ def test_keyword_args_and_generalized_unpacking(): with pytest.raises(RuntimeError) as excinfo: m.test_arg_conversion_error1(f) - assert "Unable to convert call argument" in str(excinfo.value) + assert str(excinfo.value) == "Unable to convert call argument " + ( + "'1' of type 'UnregisteredType' to Python object" + if detailed_error_messages_enabled + else "'1' to Python object (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)" + ) with pytest.raises(RuntimeError) as excinfo: m.test_arg_conversion_error2(f) - assert "Unable to convert call argument" in str(excinfo.value) + assert str(excinfo.value) == "Unable to convert call argument " + ( + "'expected_name' of type 'UnregisteredType' to Python object" + if detailed_error_messages_enabled + else "'expected_name' to Python object " + "(#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)" + ) def test_lambda_closure_cleanup(): diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index f57e09506..854c7e6f7 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -339,4 +339,9 @@ TEST_SUBMODULE(exceptions, m) { } return py::str("UNEXPECTED"); }); + + m.def("test_fn_cast_int", [](const py::function &fn) { + // function returns None instead of int, should give a useful error message + fn().cast(); + }); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 0d2c80814..8bcefe3de 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -381,3 +381,12 @@ def test_pypy_oserror_normalization(): # https://github.com/pybind/pybind11/issues/4075 what = m.test_pypy_oserror_normalization() assert "this_filename_must_not_exist" in what + + +def test_fn_cast_int_exception(): + with pytest.raises(RuntimeError) as excinfo: + m.test_fn_cast_int(lambda: None) + + assert str(excinfo.value).startswith( + "Unable to cast Python instance of type to C++ type" + ) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 8f9f2987e..a1a80a293 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -536,7 +536,7 @@ def test_print(capture): assert str(excinfo.value) == "Unable to convert call argument " + ( "'1' of type 'UnregisteredType' to Python object" if detailed_error_messages_enabled - else "to Python object (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)" + else "'1' to Python object (#define PYBIND11_DETAILED_ERROR_MESSAGES or compile in debug mode for details)" )