From c090c8c409feb6f7f89657b0677a46b3e2192493 Mon Sep 17 00:00:00 2001
From: Robert Haschke <rhaschke@users.noreply.github.com>
Date: Wed, 7 Jul 2021 00:13:13 +0200
Subject: [PATCH] Unify cast_error message thrown by
 [simple|unpacking]_collector (#3013)

* Unify cast_error message thrown by [simple|unpacking]_collector

simple_collector and unpacking_collector throw different error messages
when the casting of an argument failed: While the former mentions make_tuple(),
the latter emphasises the call argument (and its name/position).

* Consolidating "Unable to convert call argument" error reporting code to guarantee uniformity.

Co-authored-by: Ralf W. Grosse-Kunstleve <rwgk@google.com>
---
 include/pybind11/cast.h | 40 ++++++++++++++++++++++------------------
 tests/test_pytypes.py   |  6 +++---
 2 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
index 1ef1a9ce..4a219e2d 100644
--- a/include/pybind11/cast.h
+++ b/include/pybind11/cast.h
@@ -958,6 +958,21 @@ template <> inline void cast_safe<void>(object &&) {}
 
 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(NDEBUG)
+inline cast_error cast_error_unable_to_convert_call_arg() {
+    return cast_error(
+        "Unable to convert call argument to Python object (compile in debug mode for details)");
+}
+#else
+inline cast_error cast_error_unable_to_convert_call_arg(const std::string &name,
+                                                        const std::string &type) {
+    return cast_error("Unable to convert call argument '" + name + "' of type '" + type
+                      + "' to Python object");
+}
+#endif
+
 template <return_value_policy policy = return_value_policy::automatic_reference>
 tuple make_tuple() { return tuple(0); }
 
@@ -971,11 +986,10 @@ template <return_value_policy policy = return_value_policy::automatic_reference,
     for (size_t i = 0; i < args.size(); i++) {
         if (!args[i]) {
 #if defined(NDEBUG)
-            throw cast_error("make_tuple(): unable to convert arguments to Python object (compile in debug mode for details)");
+            throw cast_error_unable_to_convert_call_arg();
 #else
             std::array<std::string, size> argtypes { {type_id<Args>()...} };
-            throw cast_error("make_tuple(): unable to convert argument of type '" +
-                argtypes[i] + "' to Python object");
+            throw cast_error_unable_to_convert_call_arg(std::to_string(i), argtypes[i]);
 #endif
         }
     }
@@ -1230,9 +1244,10 @@ private:
         auto o = reinterpret_steal<object>(detail::make_caster<T>::cast(std::forward<T>(x), policy, {}));
         if (!o) {
 #if defined(NDEBUG)
-            argument_cast_error();
+            throw cast_error_unable_to_convert_call_arg();
 #else
-            argument_cast_error(std::to_string(args_list.size()), type_id<T>());
+            throw cast_error_unable_to_convert_call_arg(
+                std::to_string(args_list.size()), type_id<T>());
 #endif
         }
         args_list.append(o);
@@ -1260,9 +1275,9 @@ private:
         }
         if (!a.value) {
 #if defined(NDEBUG)
-            argument_cast_error();
+            throw cast_error_unable_to_convert_call_arg();
 #else
-            argument_cast_error(a.name, a.type);
+            throw cast_error_unable_to_convert_call_arg(a.name, a.type);
 #endif
         }
         m_kwargs[a.name] = a.value;
@@ -1301,17 +1316,6 @@ private:
         throw type_error("Got multiple values for keyword argument '" + name + "'");
     }
 
-    [[noreturn]] static void argument_cast_error() {
-        throw cast_error("Unable to convert call argument to Python object "
-                         "(compile in debug mode for details)");
-    }
-
-    [[noreturn]] static void argument_cast_error(const std::string &name,
-                                                 const std::string &type) {
-        throw cast_error("Unable to convert call argument '" + name
-                         + "' of type '" + type + "' to Python object");
-    }
-
 private:
     tuple m_args;
     dict m_kwargs;
diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
index f17bf76b..f234c773 100644
--- a/tests/test_pytypes.py
+++ b/tests/test_pytypes.py
@@ -372,10 +372,10 @@ def test_print(capture):
 
     with pytest.raises(RuntimeError) as excinfo:
         m.print_failure()
-    assert str(excinfo.value) == "make_tuple(): unable to convert " + (
-        "argument of type 'UnregisteredType' to Python object"
+    assert str(excinfo.value) == "Unable to convert call argument " + (
+        "'1' of type 'UnregisteredType' to Python object"
         if debug_enabled
-        else "arguments to Python object (compile in debug mode for details)"
+        else "to Python object (compile in debug mode for details)"
     )