Skip to content

Commit

Permalink
Fix void pointers. Previously Rice could not send a wrap C++ object b…
Browse files Browse the repository at this point in the history
…ack to C++ via a void method parameter.
  • Loading branch information
cfis committed Nov 16, 2024
1 parent b58d751 commit 54e238c
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 4 deletions.
72 changes: 72 additions & 0 deletions rice/Data_Object.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,36 @@ namespace Rice::detail
Return* returnInfo_ = nullptr;
};

template <>
class To_Ruby<void*>
{
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<VALUE, rb_data_type_t*> 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 <typename T>
class To_Ruby<T*&>
{
Expand Down Expand Up @@ -452,5 +482,47 @@ namespace Rice::detail
return Data_Object<T>(value);
}
};

template<>
class From_Ruby<void*>
{
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<void>(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_
12 changes: 9 additions & 3 deletions rice/Data_Type.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
namespace Rice
{
template<typename T>
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();
Expand All @@ -30,17 +30,23 @@ namespace Rice
}

template<typename T>
void ruby_free_internal(detail::Wrapper* wrapper)
inline void ruby_free_internal(detail::Wrapper* wrapper)
{
delete wrapper;
}

template<typename T>
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<typename T>
template <typename Base_T>
inline Data_Type<T> Data_Type<T>::bind(const Module& klass)
Expand Down
2 changes: 1 addition & 1 deletion rice/detail/InstanceRegistry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace Rice::detail

template <typename T>
VALUE lookup(T* cppInstance);
VALUE lookup(void* cppInstance);

void add(void* cppInstance, VALUE rubyInstance);
void remove(void* cppInstance);
Expand All @@ -23,7 +24,6 @@ namespace Rice::detail
bool isEnabled = false;

private:
VALUE lookup(void* cppInstance);
std::map<void*, VALUE> objectMap_;
};
} // namespace Rice::detail
Expand Down
16 changes: 16 additions & 0 deletions rice/detail/TypeRegistry.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> is more consistent with Rice then
define_class<void*>. However, the types of void and void* are different so we need
this special case.
It is possible to support define_class<void*>, 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<void>(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 <typename T>
inline void TypeRegistry::remove()
{
Expand Down
87 changes: 87 additions & 0 deletions test/test_Data_Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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*>(helper);
}

void* returnVoidHelper()
{
if (!this->helper_)
this->helper_ = new Helper(4);

return static_cast<void*>(this->helper_);
}

bool checkVoidHelper(void* helper)
{
return helper == this->helper_;
}

private:
Helper* helper_ = nullptr;
};
} // namespace

TESTCASE(pointers)
{
Class voidClass = define_class<void>("Void");

Class helperClass = define_class<Helper>("Helper")
.define_constructor(Constructor<Helper, int>())
.define_method("value", &Helper::value);

Class myClass = define_class<MyClass2>("MyClass2")
.define_constructor(Constructor<MyClass2>())
.define_method<Helper*(MyClass2::*)(Helper*)>("pass_through", &MyClass2::passThrough)
.define_method<Helper*(MyClass2::*)(void*)>("pass_through_void", &MyClass2::passThrough)
.define_method<void*(MyClass2::*)()>("return_void_helper", &MyClass2::returnVoidHelper)
.define_method<bool(MyClass2::*)(void*)>("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<int>().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<int>().convert(value));

helper = object.call("return_void_helper");
result = object.call("check_void_helper", helper);
ASSERT_EQUAL(Qtrue, result.value());
}

namespace
{
class SomeClass
Expand Down

0 comments on commit 54e238c

Please sign in to comment.