make implicit conversions non-reentrant (fixes #1035) (#1037)

docs/advanced/classes.rst

@@ -585,6 +585,10 @@ Python side:
     Implicit conversions from ``A`` to ``B`` only work when ``B`` is a custom
     Implicit conversions from ``A`` to ``B`` only work when ``B`` is a custom
     data type that is exposed to Python via pybind11.
     data type that is exposed to Python via pybind11.
 
 
+    To prevent runaway recursion, implicit conversions are non-reentrant: an
+    implicit conversion invoked as part of another implicit conversion of the
+    same type (i.e. from ``A`` to ``B``) will fail.
+
 .. _static_properties:
 .. _static_properties:
 
 
 Static properties
 Static properties

include/pybind11/pybind11.h

@@ -1536,7 +1536,16 @@ template <return_value_policy Policy = return_value_policy::reference_internal,
 }
 }
 
 
 template <typename InputType, typename OutputType> void implicitly_convertible() {
 template <typename InputType, typename OutputType> void implicitly_convertible() {
+    struct set_flag {
+        bool &flag;
+        set_flag(bool &flag) : flag(flag) { flag = true; }
+        ~set_flag() { flag = false; }
+    };
     auto implicit_caster = [](PyObject *obj, PyTypeObject *type) -> PyObject * {
     auto implicit_caster = [](PyObject *obj, PyTypeObject *type) -> PyObject * {
+        static bool currently_used = false;
+        if (currently_used) // implicit conversions are non-reentrant
+            return nullptr;
+        set_flag flag_helper(currently_used);
         if (!detail::make_caster<InputType>().load(obj, false))
         if (!detail::make_caster<InputType>().load(obj, false))
             return nullptr;
             return nullptr;
         tuple args(1);
         tuple args(1);

tests/test_class.cpp

@@ -291,6 +291,17 @@ TEST_SUBMODULE(class_, m) {
         .def(py::init<int, const std::string &>())
         .def(py::init<int, const std::string &>())
         .def_readwrite("field1", &BraceInitialization::field1)
         .def_readwrite("field1", &BraceInitialization::field1)
         .def_readwrite("field2", &BraceInitialization::field2);
         .def_readwrite("field2", &BraceInitialization::field2);
+
+    // test_reentrant_implicit_conversion_failure
+    // #1035: issue with runaway reentrant implicit conversion
+    struct BogusImplicitConversion {
+        BogusImplicitConversion(const BogusImplicitConversion &) { }
+    };
+
+    py::class_<BogusImplicitConversion>(m, "BogusImplicitConversion")
+        .def(py::init<const BogusImplicitConversion &>());
+
+    py::implicitly_convertible<int, BogusImplicitConversion>();
 }
 }
 
 
 template <int N> class BreaksBase { public: virtual ~BreaksBase() = default; };
 template <int N> class BreaksBase { public: virtual ~BreaksBase() = default; };

tests/test_class.py

@@ -223,3 +223,13 @@ def test_class_refcount():
 
 
         assert refcount_1 == refcount_3
         assert refcount_1 == refcount_3
         assert refcount_2 > refcount_1
         assert refcount_2 > refcount_1
+
+
+def test_reentrant_implicit_conversion_failure(msg):
+    # ensure that there is no runaway reentrant implicit conversion (#1035)
+    with pytest.raises(TypeError) as excinfo:
+        m.BogusImplicitConversion(0)
+    assert msg(excinfo.value) == '''__init__(): incompatible constructor arguments. The following argument types are supported:
+    1. m.class_.BogusImplicitConversion(arg0: m.class_.BogusImplicitConversion)
+
+Invoked with: 0'''