Fixed stl casters to use the appropriate type_caster cast_op_type (py…

…bind#529) stl casters were using a value cast to (Value) or (Key), but that isn't always appropriate. This changes it to use the appropriate value converter's cast_op_type.
dvirtz · Nov 25, 2016 · f200493 · f200493
1 parent 8d396fc
commit f200493
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 16 deletions.
diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h
@@ -53,7 +53,7 @@ template <typename Type, typename Key> struct set_caster {
         for (auto entry : s) {
             if (!conv.load(entry, convert))
                 return false;
-            value.insert((Key) conv);
+            value.insert(conv.operator typename key_conv::template cast_op_type<Key>());
         }
         return true;
     }
@@ -87,7 +87,9 @@ template <typename Type, typename Key, typename Value> struct map_caster {
             if (!kconv.load(it.first.ptr(), convert) ||
                 !vconv.load(it.second.ptr(), convert))
                 return false;
-            value.emplace((Key) kconv, (Value) vconv);
+            value.emplace(
+                    kconv.operator typename   key_conv::template cast_op_type<Key>(),
+                    vconv.operator typename value_conv::template cast_op_type<Value>());
         }
         return true;
     }
@@ -121,7 +123,7 @@ template <typename Type, typename Value> struct list_caster {
         for (auto it : s) {
             if (!conv.load(it, convert))
                 return false;
-            value.push_back((Value) conv);
+            value.push_back(conv.operator typename value_conv::template cast_op_type<Value>());
         }
         return true;
     }
@@ -167,7 +169,7 @@ template <typename Type, size_t Size> struct type_caster<std::array<Type, Size>>
         for (auto it : l) {
             if (!conv.load(it, convert))
                 return false;
-            value[ctr++] = (Type) conv;
+            value[ctr++] = conv.operator typename value_conv::template cast_op_type<Type>();
         }
         return true;
     }
@@ -200,13 +202,12 @@ template <typename Key, typename Value, typename Hash, typename Equal, typename
 
 // This type caster is intended to be used for std::optional and std::experimental::optional
 template<typename T> struct optional_caster {
-    using value_type = typename intrinsic_type<typename T::value_type>::type;
-    using caster_type = type_caster<value_type>;
+    using value_conv = make_caster<typename T::value_type>;
 
     static handle cast(const T& src, return_value_policy policy, handle parent) {
         if (!src)
             return none().inc_ref();
-        return caster_type::cast(*src, policy, parent);
+        return value_conv::cast(*src, policy, parent);
     }
 
     bool load(handle src, bool convert) {
@@ -215,18 +216,16 @@ template<typename T> struct optional_caster {
         } else if (src.is_none()) {
             value = {};  // nullopt
             return true;
-        } else if (!inner.load(src, convert)) {
-            return false;
-        } else {
-            value.emplace(static_cast<const value_type&>(inner));
-            return true;
         }
-    }
+        value_conv inner_caster;
+        if (!inner_caster.load(src, convert))
+            return false;
 
-    PYBIND11_TYPE_CASTER(T, _("Optional[") + caster_type::name() + _("]"));
+        value.emplace(inner_caster.operator typename value_conv::template cast_op_type<typename T::value_type>());
+        return true;
+    }
 
-private:
-    caster_type inner;
+    PYBIND11_TYPE_CASTER(T, _("Optional[") + value_conv::name() + _("]"));
 };
 
 #if PYBIND11_HAS_OPTIONAL

diff --git a/tests/test_issues.cpp b/tests/test_issues.cpp
@@ -64,6 +64,15 @@ template <typename T> class custom_unique_ptr {
 };
 PYBIND11_DECLARE_HOLDER_TYPE(T, custom_unique_ptr<T>);
 
+/// Issue #528: templated constructor
+struct TplConstrClass {
+    template <typename T> TplConstrClass(const T &arg) : str{arg} {}
+    std::string str;
+    bool operator==(const TplConstrClass &t) const { return t.str == str; }
+};
+namespace std {
+template <> struct hash<TplConstrClass> { size_t operator()(const TplConstrClass &t) const { return std::hash<std::string>()(t.str); } };
+}
 
 
 void init_issues(py::module &m) {
@@ -371,6 +380,16 @@ void init_issues(py::module &m) {
     py::class_<MyDerived, MyBase>(m2, "MyDerived")
         .def_static("make", &MyDerived::make)
         .def_static("make2", &MyDerived::make);
+
+    /// Issue #528: templated constructor
+    m2.def("tpl_constr_vector", [](std::vector<TplConstrClass> &) {});
+    m2.def("tpl_constr_map", [](std::unordered_map<TplConstrClass, TplConstrClass> &) {});
+    m2.def("tpl_constr_set", [](std::unordered_set<TplConstrClass> &) {});
+#if defined(PYBIND11_HAS_OPTIONAL)
+    m2.def("tpl_constr_optional", [](std::optional<TplConstrClass> &) {});
+#elif defined(PYBIND11_HAS_EXP_OPTIONAL)
+    m2.def("tpl_constr_optional", [](std::experimental::optional<TplConstrClass> &) {});
+#endif
 }
 
 // MSVC workaround: trying to use a lambda here crashes MSCV