Skip to content

Commit

Permalink
Turns out Rice did not support passing rvalues as function/method par…
Browse files Browse the repository at this point in the history
…ameters. Probably the most common use case for this is wrapping move constructors and move assignments.
  • Loading branch information
cfis committed Nov 13, 2024
1 parent c23986c commit 3f80fb4
Show file tree
Hide file tree
Showing 7 changed files with 232 additions and 33 deletions.
42 changes: 42 additions & 0 deletions rice/Data_Object.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,48 @@ namespace Rice::detail
Arg* arg_ = nullptr;
};

template<typename T>
class From_Ruby<T&&>
{
static_assert(!std::is_fundamental_v<intrinsic_type<T>>,
"Data_Object cannot be used with fundamental types");
public:
From_Ruby() = default;

explicit From_Ruby(Arg * arg) : arg_(arg)
{
}

Convertible is_convertible(VALUE value)
{
switch (rb_type(value))
{
case RUBY_T_DATA:
return Data_Type<T>::is_descendant(value) ? Convertible::Exact : Convertible::None;
break;
default:
return Convertible::None;
}
}

T&& convert(VALUE value)
{
using Intrinsic_T = intrinsic_type<T>;

if (value == Qnil && this->arg_ && this->arg_->hasDefaultValue())
{
return std::move(this->arg_->template defaultValue<Intrinsic_T>());
}
else
{
return std::move(*Data_Object<Intrinsic_T>::from_ruby(value));
}
}

private:
Arg* arg_ = nullptr;
};

template<typename T>
class From_Ruby<T*>
{
Expand Down
1 change: 1 addition & 0 deletions rice/Data_Object_defn.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ namespace Rice
static_assert(!std::is_reference_v<T>);
static_assert(!std::is_const_v<T>);
static_assert(!std::is_volatile_v<T>);
static_assert(!std::is_void_v<T>);

public:
static T* from_ruby(VALUE value);
Expand Down
4 changes: 2 additions & 2 deletions rice/detail/NativeFunction.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ namespace Rice::detail
void checkKeepAlive(VALUE self, VALUE returnValue, std::vector<VALUE>& rubyValues);

// Call the underlying C++ function
VALUE invokeNativeFunction(const Arg_Ts& nativeArgs);
VALUE invokeNativeMethod(VALUE self, const Arg_Ts& nativeArgs);
VALUE invokeNativeFunction(Arg_Ts&& nativeArgs);
VALUE invokeNativeMethod(VALUE self, Arg_Ts&& nativeArgs);

private:
VALUE klass_;
Expand Down
18 changes: 9 additions & 9 deletions rice/detail/NativeFunction.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -196,37 +196,37 @@ namespace Rice::detail
}

template<typename Class_T, typename Function_T, bool IsMethod>
VALUE NativeFunction<Class_T, Function_T, IsMethod>::invokeNativeFunction(const Arg_Ts& nativeArgs)
VALUE NativeFunction<Class_T, Function_T, IsMethod>::invokeNativeFunction(Arg_Ts&& nativeArgs)
{
if constexpr (std::is_void_v<Return_T>)
{
std::apply(this->function_, nativeArgs);
std::apply(this->function_, std::forward<Arg_Ts>(nativeArgs));
return Qnil;
}
else
{
// Call the native method and get the result
Return_T nativeResult = std::apply(this->function_, nativeArgs);
Return_T nativeResult = std::apply(this->function_, std::forward<Arg_Ts>(nativeArgs));

// Return the result
return this->toRuby_.convert(nativeResult);
}
}

template<typename Class_T, typename Function_T, bool IsMethod>
VALUE NativeFunction<Class_T, Function_T, IsMethod>::invokeNativeMethod(VALUE self, const Arg_Ts& nativeArgs)
VALUE NativeFunction<Class_T, Function_T, IsMethod>::invokeNativeMethod(VALUE self, Arg_Ts&& nativeArgs)
{
Receiver_T receiver = this->getReceiver(self);
auto selfAndNativeArgs = std::tuple_cat(std::forward_as_tuple(receiver), nativeArgs);
auto selfAndNativeArgs = std::tuple_cat(std::forward_as_tuple(receiver), std::forward<Arg_Ts>(nativeArgs));

if constexpr (std::is_void_v<Return_T>)
{
std::apply(this->function_, selfAndNativeArgs);
std::apply(this->function_, std::forward<decltype(selfAndNativeArgs)>(selfAndNativeArgs));
return Qnil;
}
else
{
Return_T nativeResult = (Return_T)std::apply(this->function_, selfAndNativeArgs);
Return_T nativeResult = (Return_T)std::apply(this->function_, std::forward<decltype(selfAndNativeArgs)>(selfAndNativeArgs));

// Special handling if the method returns self. If so we do not want
// to create a new Ruby wrapper object and instead return self.
Expand Down Expand Up @@ -327,11 +327,11 @@ namespace Rice::detail
VALUE result = Qnil;
if constexpr (std::is_same_v<Receiver_T, std::nullptr_t>)
{
result = this->invokeNativeFunction(nativeValues);
result = this->invokeNativeFunction(std::forward<Arg_Ts>(nativeValues));
}
else
{
result = this->invokeNativeMethod(self, nativeValues);
result = this->invokeNativeMethod(self, std::forward<Arg_Ts>(nativeValues));
}

// Check if any function arguments or return values need to have their lifetimes tied to the receiver
Expand Down
59 changes: 54 additions & 5 deletions test/test_Constructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ using namespace Rice;

TESTSUITE(Constructor);

SETUP(Construtor)
{
embed_ruby();
}

namespace
{
class Default_Constructible
Expand All @@ -18,11 +23,6 @@ namespace
};
}

SETUP(Array)
{
embed_ruby();
}

TESTCASE(default_constructor)
{
Data_Type<Default_Constructible> rb_cDefault_Constructible(anonymous_class());
Expand Down Expand Up @@ -125,3 +125,52 @@ 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)
{
}
};
}

TESTCASE(constructor_copy)
{
Class c = define_class<MyClass>("MyClass")
.define_constructor(Constructor<MyClass>())
.define_constructor(Constructor<MyClass, const MyClass&>());

// Default constructor
Object o1 = c.call("new");
ASSERT_EQUAL(c, o1.class_of());

// Copy constructor
Object o2 = c.call("new", o1);
ASSERT_EQUAL(c, o2.class_of());
}

TESTCASE(constructor_move)
{
Class c = define_class<MyClass>("MyClass")
.define_constructor(Constructor<MyClass>())
.define_constructor(Constructor<MyClass, MyClass&&>());

// Default constructor
Object o1 = c.call("new");
ASSERT_EQUAL(c, o1.class_of());

// Move constructor
Object o2 = c.call("new", o1);
ASSERT_EQUAL(c, o2.class_of());
}
51 changes: 51 additions & 0 deletions test/test_Data_Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,57 @@ TESTCASE(define_singleton_method_returning_reference)
ASSERT_EQUAL(result, String("foo"));
}

namespace
{
class RValue
{
public:
RValue() {}

RValue(RValue&& other) = default;

// Move assignment operator.
RValue& operator=(RValue&& other) noexcept
{
return *this;
}

bool takesRValue(RValue&& rvalue)
{
return true;
}
};
}

TESTCASE(rvalue_parameter)
{
Class c = define_class<RValue>("RValue")
.define_constructor(Constructor<RValue>())
.define_method("takes_r_value", &RValue::takesRValue);

Module m(anonymous_module());
std::string code = R"(rvalue = RValue.new
rvalue.takes_r_value(rvalue))";

Object result = m.module_eval(code);
ASSERT_EQUAL(Qtrue, result.value());
}

TESTCASE(move_assignment)
{
Class c = define_class<RValue>("RValue")
.define_constructor(Constructor<RValue>())
.define_method<RValue&(RValue::*)(RValue && other) noexcept>("=", &RValue::operator=);

Module m(anonymous_module());
std::string code = R"(object1 = RValue.new
object2 = RValue.new
object1 = object2)";

Object result = m.module_eval(code);
ASSERT_EQUAL(c, result.class_of());
}

namespace
{
struct MyStruct
Expand Down
90 changes: 73 additions & 17 deletions test/test_Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,27 +59,51 @@ TESTCASE(add_handler)

namespace
{
class MyClass
{
public:
MyClass() = default;
};

bool some_function()
{
return true;
}
bool some_function()
{
return true;
}

Object some_method(Object o)
{
return o;
}
Object some_method(Object o)
{
return o;
}

int function_int(int i)
{
return i;
}
int function_int(int i)
{
return i;
}

int method_int(Object object, int i)
{
return i;
}
int method_int(Object object, int i)
{
return i;
}

bool method_lvalue(MyClass& myClass)
{
return true;
}

bool method_rvalue(MyClass&& myClass)
{
return true;
}

void method_lvalue_return_void(int a, MyClass& myClass)
{
// Do nothing
}

void method_rvalue_return_void(int b, MyClass&& myClass)
{
// Do nothing
}
} // namespace

TESTCASE(define_method)
Expand Down Expand Up @@ -515,4 +539,36 @@ TESTCASE(references)
ASSERT_EQUAL(boolValue, true);
ASSERT_EQUAL(floatValue, 43.0);
ASSERT_EQUAL(doubleValue, 44.0);
}
}

TESTCASE(lvalue_function)
{
Module m(anonymous_module());
Class c = define_class<MyClass>("MyClass").
define_constructor(Constructor<MyClass>());

m.define_module_function("method_lvalue", &method_lvalue);
m.define_module_function("method_lvalue_return_void", &method_lvalue_return_void);

Object result = m.module_eval(R"(o = MyClass.new
method_lvalue_return_void(1, o)
method_lvalue(o))");

ASSERT_EQUAL(Qtrue, result.value());
}

TESTCASE(rvalue_function)
{
Module m(anonymous_module());
Class c = define_class<MyClass>("MyClass").
define_constructor(Constructor<MyClass>());

m.define_module_function("method_rvalue", &method_rvalue);
m.define_module_function("method_rvalue_return_void", &method_rvalue_return_void);

Object result = m.module_eval(R"(o = MyClass.new
method_rvalue_return_void(1, o)
method_rvalue(o))");

ASSERT_EQUAL(Qtrue, result.value());
}

0 comments on commit 3f80fb4

Please sign in to comment.