From 54e238c3a30f3e98e07a58d565257181320ecb2f Mon Sep 17 00:00:00 2001 From: cfis Date: Fri, 15 Nov 2024 19:27:34 -0800 Subject: [PATCH] Fix void pointers. Previously Rice could not send a wrap C++ object back to C++ via a void method parameter. --- rice/Data_Object.ipp | 72 ++++++++++++++++++++++++++ rice/Data_Type.ipp | 12 +++-- rice/detail/InstanceRegistry.hpp | 2 +- rice/detail/TypeRegistry.ipp | 16 ++++++ test/test_Data_Type.cpp | 87 ++++++++++++++++++++++++++++++++ 5 files changed, 185 insertions(+), 4 deletions(-) diff --git a/rice/Data_Object.ipp b/rice/Data_Object.ipp index fa4b86e7..e017b9df 100644 --- a/rice/Data_Object.ipp +++ b/rice/Data_Object.ipp @@ -190,6 +190,36 @@ namespace Rice::detail Return* returnInfo_ = nullptr; }; + template <> + class To_Ruby + { + public: + To_Ruby() = default; + + explicit To_Ruby(Return* returnInfo) : returnInfo_(returnInfo) + { + } + + VALUE convert(void* data) + { + if (data) + { + // Note that T could be a pointer or reference to a base class while data is in fact a + // child class. Lookup the correct type so we return an instance of the correct Ruby class + std::pair rubyTypeInfo = detail::Registries::instance.types.figureType(data); + bool isOwner = this->returnInfo_ && this->returnInfo_->isOwner(); + return detail::wrap(rubyTypeInfo.first, rubyTypeInfo.second, data, isOwner); + } + else + { + return Qnil; + } + } + + private: + Return* returnInfo_ = nullptr; + }; + template class To_Ruby { @@ -452,5 +482,47 @@ namespace Rice::detail return Data_Object(value); } }; + + template<> + class From_Ruby + { + public: + Convertible is_convertible(VALUE value) + { + switch (rb_type(value)) + { + case RUBY_T_DATA: + // Hope for the best! + return Convertible::Exact; + break; + case RUBY_T_NIL: + return Convertible::Exact; + break; + default: + return Convertible::None; + } + } + + void* convert(VALUE value) + { + switch (rb_type(value)) + { + case RUBY_T_DATA: + { + // Since C++ is not telling us type information, we need to extract it + // from the Ruby object. + const rb_data_type_t* rb_type = RTYPEDDATA_TYPE(value); + return detail::unwrap(value, (rb_data_type_t*)rb_type); + break; + } + case RUBY_T_NIL: + return nullptr; + break; + default: + throw Exception(rb_eTypeError, "wrong argument type %s (expected % s)", + detail::protect(rb_obj_classname, value), "pointer"); + } + } + }; } #endif // Rice__Data_Object__ipp_ \ No newline at end of file diff --git a/rice/Data_Type.ipp b/rice/Data_Type.ipp index 2d1dcc31..41cb7d25 100644 --- a/rice/Data_Type.ipp +++ b/rice/Data_Type.ipp @@ -19,7 +19,7 @@ namespace Rice { template - void ruby_mark_internal(detail::Wrapper* wrapper) + inline void ruby_mark_internal(detail::Wrapper* wrapper) { // Tell the wrapper to mark the objects its keeping alive wrapper->ruby_mark(); @@ -30,17 +30,23 @@ namespace Rice } template - void ruby_free_internal(detail::Wrapper* wrapper) + inline void ruby_free_internal(detail::Wrapper* wrapper) { delete wrapper; } template - size_t ruby_size_internal(const T* data) + inline size_t ruby_size_internal(const T* data) { return sizeof(T); } + template<> + inline size_t ruby_size_internal(const void* data) + { + return sizeof(void*); + } + template template inline Data_Type Data_Type::bind(const Module& klass) diff --git a/rice/detail/InstanceRegistry.hpp b/rice/detail/InstanceRegistry.hpp index ff948f9f..c31731c5 100644 --- a/rice/detail/InstanceRegistry.hpp +++ b/rice/detail/InstanceRegistry.hpp @@ -14,6 +14,7 @@ namespace Rice::detail template VALUE lookup(T* cppInstance); + VALUE lookup(void* cppInstance); void add(void* cppInstance, VALUE rubyInstance); void remove(void* cppInstance); @@ -23,7 +24,6 @@ namespace Rice::detail bool isEnabled = false; private: - VALUE lookup(void* cppInstance); std::map objectMap_; }; } // namespace Rice::detail diff --git a/rice/detail/TypeRegistry.ipp b/rice/detail/TypeRegistry.ipp index 77eb6914..371b9de4 100644 --- a/rice/detail/TypeRegistry.ipp +++ b/rice/detail/TypeRegistry.ipp @@ -13,6 +13,22 @@ namespace Rice::detail registry_[key] = std::pair(klass, rbType); } + /* Special case void. Rice defines classes using the class name not a pointer to the + class. Thus define_class is more consistent with Rice then + define_class. However, the types of void and void* are different so we need + this special case. + + It is possible to support define_class, but it requires changing the static + assertions on Data_Type and Data_Object and thus seems less desirable (and less + consistent as mentioned above).*/ + template <> + inline void TypeRegistry::add(VALUE klass, rb_data_type_t* rbType) + { + // The special case, use void* + std::type_index key(typeid(void*)); + registry_[key] = std::pair(klass, rbType); + } + template inline void TypeRegistry::remove() { diff --git a/test/test_Data_Type.cpp b/test/test_Data_Type.cpp index be5281b0..b890c644 100644 --- a/test/test_Data_Type.cpp +++ b/test/test_Data_Type.cpp @@ -450,6 +450,93 @@ TESTCASE(null_ptrs) ASSERT_EQUAL(Qnil, result.value()); } +namespace +{ + class Helper + { + public: + + Helper(int value) : value_(value) + { + } + + int value() + { + return this->value_; + } + + private: + int value_; + }; + + class MyClass2 + { + public: + Helper* passThrough(Helper* helper) + { + return helper; + } + + Helper* passThrough(void* helper) + { + return static_cast(helper); + } + + void* returnVoidHelper() + { + if (!this->helper_) + this->helper_ = new Helper(4); + + return static_cast(this->helper_); + } + + bool checkVoidHelper(void* helper) + { + return helper == this->helper_; + } + + private: + Helper* helper_ = nullptr; + }; +} // namespace + +TESTCASE(pointers) +{ + Class voidClass = define_class("Void"); + + Class helperClass = define_class("Helper") + .define_constructor(Constructor()) + .define_method("value", &Helper::value); + + Class myClass = define_class("MyClass2") + .define_constructor(Constructor()) + .define_method("pass_through", &MyClass2::passThrough) + .define_method("pass_through_void", &MyClass2::passThrough) + .define_method("return_void_helper", &MyClass2::returnVoidHelper) + .define_method("check_void_helper", &MyClass2::checkVoidHelper); + + Object helper = helperClass.call("new", 5); + Object object = myClass.call("new"); + + Object result = object.call("pass_through", nullptr); + ASSERT_EQUAL(Qnil, result.value()); + + result = object.call("pass_through", helper); + Object value = result.call("value"); + ASSERT_EQUAL(5, detail::From_Ruby().convert(value)); + + result = object.call("pass_through_void", nullptr); + ASSERT_EQUAL(Qnil, result.value()); + + result = object.call("pass_through_void", helper); + value = result.call("value"); + ASSERT_EQUAL(5, detail::From_Ruby().convert(value)); + + helper = object.call("return_void_helper"); + result = object.call("check_void_helper", helper); + ASSERT_EQUAL(Qtrue, result.value()); +} + namespace { class SomeClass