Skip to content

Commit

Permalink
Fix conversion from Ruby to char*. See ruby-rice#149.
Browse files Browse the repository at this point in the history
  • Loading branch information
cfis committed Mar 4, 2021
1 parent 35c1748 commit 595b351
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 14 deletions.
27 changes: 24 additions & 3 deletions include/rice/rice.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -962,7 +962,7 @@ namespace Rice
template<>
struct From_Ruby<char*>
{
static char const* convert(VALUE x)
static char* convert(VALUE x)
{
if (x == Qnil)
{
Expand All @@ -980,7 +980,14 @@ namespace Rice
{
static char const* convert(VALUE x)
{
return RSTRING_PTR(x);
if (x == Qnil)
{
return nullptr;
}
else
{
return RSTRING_PTR(x);
}
}
};

Expand Down Expand Up @@ -1302,7 +1309,8 @@ namespace Rice
// converted value so that a reference or a pointer to the value can be passed to
// the native function.
template <typename T>
class NativeArg<T, typename std::enable_if_t<is_primitive_v<T>>>
class NativeArg<T, typename std::enable_if_t<is_primitive_v<T> &&
!(std::is_same_v<char, intrinsic_type<T>> && std::is_pointer_v<T>)>>
{
public:
using Intrinsic_T = std::decay_t<std::remove_pointer_t<T>>;
Expand All @@ -1325,6 +1333,19 @@ namespace Rice
Intrinsic_T native_;
};

// Special case char which is a native type but if we have a pointer we
// want to pass through the underlying Ruby pointer
template <typename T>
class NativeArg<T, typename std::enable_if_t<std::is_same_v<char, intrinsic_type<T>> &&
std::is_pointer_v<T>>>
{
public:
T nativeValue(VALUE value)
{
return From_Ruby<T>::convert(value);
}
};

// NativeArg implementation that works on all other types. The primary use is for
// pointers wrapped by Data_Object where there is no reason to store a local copy.
// It is also used for converting to various Rice C++ wrappers such as Rice::Hash,
Expand Down
16 changes: 15 additions & 1 deletion rice/detail/NativeArg.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ namespace Rice
// converted value so that a reference or a pointer to the value can be passed to
// the native function.
template <typename T>
class NativeArg<T, typename std::enable_if_t<is_primitive_v<T>>>
class NativeArg<T, typename std::enable_if_t<is_primitive_v<T> &&
!(std::is_same_v<char, intrinsic_type<T>> && std::is_pointer_v<T>)>>
{
public:
using Intrinsic_T = std::decay_t<std::remove_pointer_t<T>>;
Expand All @@ -40,6 +41,19 @@ namespace Rice
Intrinsic_T native_;
};

// Special case char which is a native type but if we have a pointer we
// want to pass through the underlying Ruby pointer
template <typename T>
class NativeArg<T, typename std::enable_if_t<std::is_same_v<char, intrinsic_type<T>> &&
std::is_pointer_v<T>>>
{
public:
T nativeValue(VALUE value)
{
return From_Ruby<T>::convert(value);
}
};

// NativeArg implementation that works on all other types. The primary use is for
// pointers wrapped by Data_Object where there is no reason to store a local copy.
// It is also used for converting to various Rice C++ wrappers such as Rice::Hash,
Expand Down
11 changes: 9 additions & 2 deletions rice/detail/from_ruby.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ namespace Rice
template<>
struct From_Ruby<char*>
{
static char const* convert(VALUE x)
static char* convert(VALUE x)
{
if (x == Qnil)
{
Expand All @@ -143,7 +143,14 @@ namespace Rice
{
static char const* convert(VALUE x)
{
return RSTRING_PTR(x);
if (x == Qnil)
{
return nullptr;
}
else
{
return RSTRING_PTR(x);
}
}
};

Expand Down
16 changes: 8 additions & 8 deletions test/test_Data_Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,11 @@ namespace
return i;
}

std::string multiple_args(int i, bool b, float f, std::string s)
std::string multiple_args(int i, bool b, float f, std::string s, char* c)
{
multiple_args_called = true;
return "multiple_args(" + std::to_string(i) + ", " + std::to_string(b) + ", " +
std::to_string(f) + ", " + s + ")";
std::to_string(f) + ", " + s + ", " + std::string(c) + ")";
}

public:
Expand Down Expand Up @@ -109,9 +109,9 @@ TESTCASE(methods_with_member_pointers)
ASSERT(MyClass::int_arg_called);
ASSERT_EQUAL(42, detail::From_Ruby<int>::convert(result.value()));

result = o.call("multiple_args", 81, true, 7.0, "a string");
result = o.call("multiple_args", 81, true, 7.0, "a string", "a char");
ASSERT(MyClass::multiple_args_called);
ASSERT_EQUAL("multiple_args(81, 1, 7.000000, a string)", detail::From_Ruby<std::string>::convert(result.value()));
ASSERT_EQUAL("multiple_args(81, 1, 7.000000, a string, a char)", detail::From_Ruby<std::string>::convert(result.value()));
}

TESTCASE(incorrect_number_of_args)
Expand Down Expand Up @@ -166,9 +166,9 @@ TESTCASE(methods_with_lambdas)
return instance.int_arg(anInt);
})
.define_method("multiple_args",
[](MyClass& instance, int anInt, bool aBool, float aFloat, std::string aString)
[](MyClass& instance, int anInt, bool aBool, float aFloat, std::string aString, char* aChar)
{
return instance.multiple_args(anInt, aBool, aFloat, aString);
return instance.multiple_args(anInt, aBool, aFloat, aString, aChar);
});

MyClass::reset();
Expand All @@ -186,9 +186,9 @@ TESTCASE(methods_with_lambdas)
ASSERT(MyClass::int_arg_called);
ASSERT_EQUAL(42, detail::From_Ruby<int>::convert(result.value()));

result = o.call("multiple_args", 81, true, 7.0, "a string");
result = o.call("multiple_args", 81, true, 7.0, "a string", "a char");
ASSERT(MyClass::multiple_args_called);
ASSERT_EQUAL("multiple_args(81, 1, 7.000000, a string)", detail::From_Ruby<std::string>::convert(result.value()));
ASSERT_EQUAL("multiple_args(81, 1, 7.000000, a string, a char)", detail::From_Ruby<std::string>::convert(result.value()));
}

TESTCASE(static_singleton_method)
Expand Down

0 comments on commit 595b351

Please sign in to comment.