diff --git a/rice/Constructor.hpp b/rice/Constructor.hpp index b1ff5d08..c9f9741e 100644 --- a/rice/Constructor.hpp +++ b/rice/Constructor.hpp @@ -10,33 +10,13 @@ namespace Rice .define_constructor(Constructor()); \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 - class Constructor - { - public: - static void construct(VALUE self, Arg_Ts...args) - { - T* data = new T(args...); - detail::replace(self, Data_Type::ruby_data_type(), data, true); - } - }; - - //! Special-case Constructor used when defining Directors. - template - class Constructor - { - public: - static void construct(Object self, Arg_Ts...args) - { - T* data = new T(self, args...); - detail::replace(self.value(), Data_Type::ruby_data_type(), data, true); - } - }; + class Constructor; } #endif // Rice__Constructor__hpp_ \ No newline at end of file diff --git a/rice/Constructor.ipp b/rice/Constructor.ipp new file mode 100644 index 00000000..f43613a4 --- /dev/null +++ b/rice/Constructor.ipp @@ -0,0 +1,79 @@ +namespace Rice +{ + template + 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; + using First_Arg_T = std::tuple_element_t<0, Arg_Types>; + return (arity == 1 && + std::is_lvalue_reference_v && + std::is_same_v>); + } + else + { + return false; + } + } + + static constexpr bool isMoveConstrutor() + { + if constexpr (arity == 1) + { + using Arg_Types = std::tuple; + using First_Arg_T = std::tuple_element_t<0, Arg_Types>; + return (arity == 1 && + std::is_rvalue_reference_v && + std::is_same_v>); + } + else + { + return false; + } + } + + static void initialize(VALUE self, Arg_Ts...args) + { + // Call C++ constructor + T* data = new T(args...); + detail::replace(self, Data_Type::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(self, Data_Type::ruby_data_type(), data, true); + } + + }; + + //! Special-case Constructor used when defining Directors. + template + class Constructor + { + 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(self.value(), Data_Type::ruby_data_type(), data, true); + } + }; +} \ No newline at end of file diff --git a/rice/Data_Type.ipp b/rice/Data_Type.ipp index e8d13e79..5c6f597f 100644 --- a/rice/Data_Type.ipp +++ b/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(*this), detail::default_allocation_func); - // 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; } diff --git a/rice/rice.hpp b/rice/rice.hpp index ee684ee6..ba287e59 100644 --- a/rice/rice.hpp +++ b/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" diff --git a/test/test_Constructor.cpp b/test/test_Constructor.cpp index a5bd1cb0..d4dc11a5 100644 --- a/test/test_Constructor.cpp +++ b/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") .define_constructor(Constructor()) - .define_constructor(Constructor()); + .define_constructor(Constructor()) + .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().convert(value)); } -TESTCASE(constructor_move) +TESTCASE(constructor_dup) { - Class c = define_class("MyClass") - .define_constructor(Constructor()) - .define_constructor(Constructor()); + Class c = define_class("MyClass"). + define_constructor(Constructor()). + define_constructor(Constructor()). + 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().convert(value)); +} + +TESTCASE(constructor_move) +{ + Data_Type c = define_class("MyClass"). + define_constructor(Constructor()); + + // This intentionally will not compile due to a static_assert + //c.define_constructor(Constructor()); } \ No newline at end of file