core: Fix postcall for primitive type

Author: jorisv

include/eigenpy/variant.hpp

@@ -98,6 +98,15 @@ struct VariantValueToObject : VariantVisitorType<PyObject*, Variant> {
   using Base::operator();
 };
 
+template <typename T>
+struct is_class_or_union
+    : std::integral_constant<bool, std::is_class<T>::value ||
+                                       std::is_union<T>::value> {};
+template <typename T>
+struct is_class_or_union_remove_cvref
+    : is_class_or_union<typename std::remove_cv<
+          typename std::remove_reference<T>::type>::type> {};
+
 /// Convert {boost,std}::variant<class...> alternative reference to a Python
 /// object. This converter return the alternative reference. The code that
 /// create the reference holder is taken from \see
@@ -113,19 +122,15 @@ struct VariantRefToObject : VariantVisitorType<PyObject*, Variant> {
   }
 
   template <typename T,
-            typename std::enable_if<
-                std::is_arithmetic<typename std::remove_cv<
-                    typename std::remove_reference<T>::type>::type>::value,
-                bool>::type = true>
+            typename std::enable_if<!is_class_or_union_remove_cvref<T>::value,
+                                    bool>::type = true>
   result_type operator()(T t) const {
     return boost::python::incref(boost::python::object(t).ptr());
   }
 
   template <typename T,
-            typename std::enable_if<
-                !std::is_arithmetic<typename std::remove_cv<
-                    typename std::remove_reference<T>::type>::type>::value,
-                bool>::type = true>
+            typename std::enable_if<is_class_or_union_remove_cvref<T>::value,
+                                    bool>::type = true>
   result_type operator()(T& t) const {
     return boost::python::detail::make_reference_holder::execute(&t);
   }
@@ -180,6 +185,15 @@ struct ReturnInternalVariant : boost::python::return_internal_reference<> {
   typedef Variant variant_type;
 
   typedef details::VariantConverter<variant_type> result_converter;
+
+  template <class ArgumentPackage>
+  static PyObject* postcall(ArgumentPackage const& args_, PyObject* result) {
+    // Don't run return_internal_reference postcall on primitive type
+    if (PyLong_Check(result) || PyBool_Check(result) || PyFloat_Check(result)) {
+      return result;
+    }
+    return boost::python::return_internal_reference<>::postcall(args_, result);
+  }
 };
 
 /// Define a defaults converter to convert a {boost,std}::variant alternative to

unittest/python/test_variant.py.in

@@ -6,9 +6,11 @@ V2 = variant_module.V2
 VariantHolder = variant_module.VariantHolder
 VariantNoneHolder = variant_module.VariantNoneHolder
 VariantArithmeticHolder = variant_module.VariantArithmeticHolder
+VariantBoolHolder = variant_module.VariantBoolHolder
 make_variant = variant_module.make_variant
 make_variant_none = variant_module.make_variant_none
 make_variant_arithmetic = variant_module.make_variant_arithmetic
+make_variant_bool = variant_module.make_variant_bool
 
 variant = make_variant()
 assert isinstance(variant, V1)
@@ -60,19 +62,35 @@ assert variant_none_holder.variant.v == 1
 v1 = variant_none_holder.variant
 v1.v = 10
 assert variant_none_holder.variant.v == 10
+# TODO make this work
 # variant_none_holder.variant = None
 
 
 # Test variant that hold base type
 v_arithmetic = make_variant_arithmetic()
 assert isinstance(v_arithmetic, int)
 
-variant_arithemtic_holder = VariantArithmeticHolder()
-assert isinstance(variant_arithemtic_holder.variant, int)
-variant_arithemtic_holder.variant = 2.0
-assert variant_arithemtic_holder.variant == 2
-assert isinstance(variant_arithemtic_holder.variant, float)
-# Arithmetic type doesn't support reference
-v1 = variant_arithemtic_holder.variant
-v1 = 3
-assert variant_arithemtic_holder.variant != v1
+variant_arithmetic_holder = VariantArithmeticHolder()
+assert isinstance(variant_arithmetic_holder.variant, int)
+variant_arithmetic_holder.variant = 2
+# Raise an exception if return_internal_postcall is called
+assert variant_arithmetic_holder.variant == 2
+
+variant_arithmetic_holder.variant = 2.0
+assert isinstance(variant_arithmetic_holder.variant, float)
+# Raise an exception if return_internal_postcall is called
+assert variant_arithmetic_holder.variant == 2.0
+
+v_bool = make_variant_bool()
+assert isinstance(v_bool, bool)
+
+variant_bool_holder = VariantBoolHolder()
+assert isinstance(variant_bool_holder.variant, bool)
+variant_bool_holder.variant = False
+# Raise an exception if return_internal_postcall is called
+assert not variant_bool_holder.variant
+
+variant_bool_holder.variant = 2.0
+assert isinstance(variant_bool_holder.variant, float)
+# Raise an exception if return_internal_postcall is called
+assert variant_bool_holder.variant == 2.0

unittest/variant.cpp.in

@@ -36,12 +36,17 @@ typedef typename MyVariantNoneHelper<VARIANT<V1> >::type MyVariantNone;
 
 typedef VARIANT<int, double> MyVariantArithmetic;
 
+// There is a conversion conflict between int and bool
+typedef VARIANT<bool, double> MyVariantBool;
+
 MyVariant make_variant() { return V1(); }
 
 MyVariantNone make_variant_none() { return MyVariantNone(); }
 
 MyVariantArithmetic make_variant_arithmetic() { return MyVariantArithmetic(); }
 
+MyVariantBool make_variant_bool() { return MyVariantBool(); }
+
 struct VariantHolder {
   MyVariant variant;
 };
@@ -54,6 +59,10 @@ struct VariantArithmeticHolder {
   MyVariantArithmetic variant;
 };
 
+struct VariantBoolHolder {
+  MyVariantBool variant;
+};
+
 BOOST_PYTHON_MODULE(@MODNAME@) {
   using namespace eigenpy;
 
@@ -94,4 +103,16 @@ BOOST_PYTHON_MODULE(@MODNAME@) {
           bp::make_getter(&VariantArithmeticHolder::variant,
                           ConverterArithmetic::return_internal_reference()),
           bp::make_setter(&VariantArithmeticHolder::variant));
+
+  typedef eigenpy::VariantConverter<MyVariantBool> ConverterBool;
+  ConverterBool::registration();
+  bp::def("make_variant_bool", make_variant_bool);
+
+  boost::python::class_<VariantBoolHolder>("VariantBoolHolder",
+                                                 bp::init<>())
+      .add_property(
+          "variant",
+          bp::make_getter(&VariantBoolHolder::variant,
+                          ConverterBool::return_internal_reference()),
+          bp::make_setter(&VariantBoolHolder::variant));
 }