Always display python type information in cast errors (#4463)

* Always display python type information in cast errors

* Address comments

* Update comment

Author: virtuald

include/pybind11/cast.h

@@ -1017,11 +1017,14 @@ type_caster<T, SFINAE> &load_type(type_caster<T, SFINAE> &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<std::string>()
+            + " 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<std::string>() + " to C++ type '"
                          + type_id<T>() + "'");
 #endif
     }
@@ -1085,12 +1088,13 @@ detail::enable_if_t<!detail::move_never<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<std::string>()
+            + " 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<T>()
-                         + " instance: instance has multiple references");
+        throw cast_error("Unable to move from Python "
+                         + str(type::handle_of(obj)).cast<std::string>() + " instance to C++ "
+                         + type_id<T>() + " 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<std::string, size> argtypes{{type_id<Args>()...}};
             throw cast_error_unable_to_convert_call_arg(std::to_string(i), argtypes[i]);
@@ -1510,7 +1515,7 @@ class unpacking_collector {
             detail::make_caster<T>::cast(std::forward<T>(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<T>());
@@ -1542,7 +1547,7 @@ class unpacking_collector {
         }
         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

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

tests/test_callbacks.py

@@ -5,6 +5,7 @@
 
 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 f(*args, **kwargs):
 
     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():

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<int>();
+    });
 }

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 <class 'NoneType'> to C++ type"
+    )

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)"
     )