Support Object#dup and Object#clone, disallow wrapping move construct…

…ors.

rice/Constructor.hpp

@@ -10,33 +10,13 @@ namespace Rice
           .define_constructor(Constructor<Test>());
       \endcode
   *
-  *  The first template type must be the type being wrapped.
-  *  Then any additional types must match the appropriate constructor
-  *  to be used in C++ when constructing the object.
+  *  The first template argument must be the type being wrapped.
+  *  Additional arguments must be the types of the parameters sent
+  *  to the constructor.
   *
   *  For more information, see Rice::Data_Type::define_constructor.
   */
   template<typename T, typename...Arg_Ts>
-  class Constructor
-  {
-  public:
-    static void construct(VALUE self, Arg_Ts...args)
-    {
-      T* data = new T(args...);
-      detail::replace<T>(self, Data_Type<T>::ruby_data_type(), data, true);
-    }
-  };
-
-  //! Special-case Constructor used when defining Directors.
-  template<typename T, typename...Arg_Ts>
-  class Constructor<T, Object, Arg_Ts...>
-  {
-    public:
-      static void construct(Object self, Arg_Ts...args)
-      {
-        T* data = new T(self, args...);
-        detail::replace<T>(self.value(), Data_Type<T>::ruby_data_type(), data, true);
-      }
-  };
+  class Constructor;
 }
 #endif // Rice__Constructor__hpp_

rice/Constructor.ipp

@@ -0,0 +1,79 @@
+namespace Rice
+{
+  template<typename T, typename...Arg_Ts>
+  class Constructor
+  {
+  public:
+    static constexpr std::size_t arity = sizeof...(Arg_Ts);
+
+    static constexpr bool isCopyConstrutor()
+    {
+      if constexpr (arity == 1)
+      {
+        using Arg_Types = std::tuple<Arg_Ts...>;
+        using First_Arg_T = std::tuple_element_t<0, Arg_Types>;
+        return (arity == 1 &&
+                std::is_lvalue_reference_v<First_Arg_T> &&
+                std::is_same_v<T, detail::intrinsic_type<First_Arg_T>>);
+      }
+      else
+      {
+        return false;
+      }
+    }
+
+    static constexpr bool isMoveConstrutor()
+    {
+      if constexpr (arity == 1)
+      {
+        using Arg_Types = std::tuple<Arg_Ts...>;
+        using First_Arg_T = std::tuple_element_t<0, Arg_Types>;
+        return (arity == 1 &&
+          std::is_rvalue_reference_v<First_Arg_T> &&
+          std::is_same_v<T, detail::intrinsic_type<First_Arg_T>>);
+      }
+      else
+      {
+        return false;
+      }
+    }
+
+    static void initialize(VALUE self, Arg_Ts...args)
+    {
+      // Call C++ constructor
+      T* data = new T(args...);
+      detail::replace<T>(self, Data_Type<T>::ruby_data_type(), data, true);
+    }
+
+    static void initialize_copy(VALUE self, const T& other)
+    {
+      // Call C++ copy constructor
+      T* data = new T(other);
+      detail::replace<T>(self, Data_Type<T>::ruby_data_type(), data, true);
+    }
+
+  };
+
+  //! Special-case Constructor used when defining Directors.
+  template<typename T, typename...Arg_Ts>
+  class Constructor<T, Object, Arg_Ts...>
+  {
+    public:
+      static constexpr bool isCopyConstrutor()
+      {
+        return false;
+      }
+
+      static constexpr bool isMoveConstrutor()
+      {
+        return false;
+      }
+
+      static void initialize(Object self, Arg_Ts...args)
+      {
+        // Call C++ constructor
+        T* data = new T(self, args...);
+        detail::replace<T>(self.value(), Data_Type<T>::ruby_data_type(), data, true);
+      }
+  };
+}

rice/Data_Type.ipp

@@ -137,8 +137,26 @@ namespace Rice
     // Define a Ruby allocator which creates the Ruby object
     detail::protect(rb_define_alloc_func, static_cast<VALUE>(*this), detail::default_allocation_func<T>);
 
-    // Define an initialize function that will create the C++ object
-    this->define_method("initialize", &Constructor_T::construct, args...);
+    // We can't do anything with move constructors so blow up
+    static_assert(!Constructor_T::isMoveConstrutor(), "Rice does not support move constructors");
+
+    // If this is a copy constructor then use it to support Ruby's Object#dup and Object#clone methods.
+    // Otherwise if a user calls #dup or #clone an error will occur because the newly cloned Ruby
+    // object will have a NULL ptr because the C++ object is never copied. This also prevents having
+    // very unlike Ruby code of:
+    // 
+    //    my_object_copy = MyObject.new(my_ojbect_original).
+
+    if constexpr (Constructor_T::isCopyConstrutor())
+    {
+      // Define initialize_copy that will copy the C++ object
+      this->define_method("initialize_copy", &Constructor_T::initialize_copy, args...);
+    }
+    else
+    {
+      // Define an initialize function that will create the C++ object
+      this->define_method("initialize", &Constructor_T::initialize, args...);
+    }
 
     return *this;
   }

rice/rice.hpp

@@ -105,6 +105,7 @@
 #include "Data_Type.ipp"
 #include "detail/default_allocation_func.ipp"
 #include "Constructor.hpp"
+#include "Constructor.ipp"
 #include "Data_Object.hpp"
 #include "Data_Object.ipp"
 #include "Enum.hpp"

test/test_Constructor.cpp

@@ -17,7 +17,7 @@ TEARDOWN(Constructor)
   rb_gc_start();
 }
 
-namespace
+/*namespace
 {
   class Default_Constructible
   {
@@ -129,53 +129,63 @@ TESTCASE(constructor_supports_single_default_argument)
 
   klass.call("new", 6);
   ASSERT_EQUAL(6, withArgX);
-}
+}*/
 
 namespace
 {
   class MyClass
   {
-  public:
-    MyClass()
-    {
-    }
-
-    MyClass(const MyClass& other)
-    {
-    }
-
-    MyClass(MyClass&& other)
-    {
-    }
+  public: 
+    MyClass() = default;
+    MyClass(const MyClass& other) = default;
+    MyClass(MyClass&& other) = default;
+    int value;
   };
 }
 
-TESTCASE(constructor_copy)
+TESTCASE(constructor_clone)
 {
   Class c = define_class<MyClass>("MyClass")
     .define_constructor(Constructor<MyClass>())
-    .define_constructor(Constructor<MyClass, const MyClass&>());
+    .define_constructor(Constructor<MyClass, const MyClass&>())
+    .define_attr("value", &MyClass::value);
 
   // Default constructor
   Object o1 = c.call("new");
+  o1.call("value=", 7);
   ASSERT_EQUAL(c, o1.class_of());
 
-  // Copy constructor
-  Object o2 = c.call("new", o1);
+  // Clone
+  Object o2 = o1.call("clone");
+  Object value = o2.call("value");
   ASSERT_EQUAL(c, o2.class_of());
+  ASSERT_EQUAL(7, detail::From_Ruby<int>().convert(value));
 }
 
-TESTCASE(constructor_move)
+TESTCASE(constructor_dup)
 {
-  Class c = define_class<MyClass>("MyClass")
-    .define_constructor(Constructor<MyClass>())
-    .define_constructor(Constructor<MyClass, MyClass&&>());
+  Class c = define_class<MyClass>("MyClass").
+            define_constructor(Constructor<MyClass>()).
+            define_constructor(Constructor<MyClass, const MyClass&>()).
+            define_attr("value", &MyClass::value);
 
   // Default constructor
   Object o1 = c.call("new");
+  o1.call("value=", 7);
   ASSERT_EQUAL(c, o1.class_of());
 
-  // Move constructor
-  Object o2 = c.call("new", o1);
+  // Clone
+  Object o2 = o1.call("dup");
+  Object value = o2.call("value");
   ASSERT_EQUAL(c, o2.class_of());
+  ASSERT_EQUAL(7, detail::From_Ruby<int>().convert(value));
+}
+
+TESTCASE(constructor_move)
+{
+  Data_Type<MyClass> c = define_class<MyClass>("MyClass").
+            define_constructor(Constructor<MyClass>());
+
+  // This intentionally will not compile due to a static_assert
+  //c.define_constructor(Constructor<MyClass, MyClass&&>());
 }