Next Release?

[cfis]

I've made a bunch of updates to Rice as I try to create Rice bindings for OpenCV. Currently this work is in this repo. Not quite sure of timing, the bindings still have a lot of work, and was going to wait until they are mostly complete.

Anyway, there are lot of new features and a couple breaking changes. Wondering if we should call this "5.0.0"?

@jasonroelofs thoughts?

Breaking changes:

• From_Ruby::is_convertible return type changed. I doubt this will impact anyone but maybe @ankane .
• I changed define_enum to define_enum_under to be consistent with the rest of the code base. Since enum support isn't even documented, not sure this will break anything either.

Changes:

• Support method overloading
• Support rvalues
• Add Data_Type#define_constant method
• Add Data_Type#define method to more easily support C++ template classes
• Updated FindRuby to support rbenv (this change is being merged upstream to CMake)
• Be more flexible on type verification by not throwing errors until all classes/methods have been defined and also allow a error message to be printed instead of thrown
• Add ability to transfer ownership of Ruby created objects to C++ (useful for sending wrapped pointers to APIs that take smart pointers)
• Add support for std::exception, std::type_index, std::sharted_ptr, std::vector, unsigned char*
• Add support for **char
• Fix std::variant to support references
• Improve C++ exception handling when called from Ruby
• Split NativeAttribute support to NativeAttributeGet and NativeAttributeSet
• Create base class for Native classes (NativeFunction, NativeIterator, NativeAttributeGet, NativeAttributeSet - make the type registry easier to deal with (thus using type erasure like the Wrapper classes do).
• Revamp type registry to support method overloading

@ankane would love to get your feedback on these changes.

Also, I have created yet another rubybindgen project to automaticaly create FFI and Rice bindings. I've been using it to create the opencv bindings. See https://github.com/ruby-rice/ruby-bindgen.

[jasonroelofs]

I don't think we need to go 5.0 for this, 4.4 should be fine. We can make the potential breaking changes clear in the changelog. And yeah we should get some documentation in place for define_enum and define_enum_under.

[ankane]

Just tried master with ORTools and am seeing a few errors:

include/rice/rice.hpp:5058:34: error: no member named 'is_convertible' in 'Rice::detail::From_Ruby<KnapsackSolver::SolverType>'
 5058 |           convertible = fromRuby.is_convertible(value);
      |                         ~~~~~~~~ ^
include/rice/rice.hpp:356:8: note: in instantiation of function template specialization 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::matches(int, VALUE *, VALUE)::(anonymous class)::operator()<Rice::detail::From_Ruby<KnapsackSolver::SolverType>>' requested here
  356 |       (callable(std::forward<decltype(args)>(args)), ...);
      |        ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__type_traits/invoke.h:341:10: note: in instantiation of function template specialization 'for_each_tuple(std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &, (lambda at include/rice/rice.hpp:5042:7) &&)::(anonymous class)::operator()<Rice::detail::From_Ruby<KnapsackSolver::SolverType> &, Rice::detail::From_Ruby<std::string> &>' requested here
  341 | decltype(std::declval<_Fp>()(std::declval<_Args>()...))
      |          ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1424:29: note: while substituting deduced template arguments into function template '__invoke' [with _Fp = (lambda at include/rice/rice.hpp:354:14), _Args = <typename tuple_element<0UL, tuple<From_Ruby<KnapsackSolver::SolverType>, From_Ruby<std::string>>>::type &, typename tuple_element<1UL, tuple<From_Ruby<KnapsackSolver::SolverType>, From_Ruby<std::string>>>::type &>]
 1424 |     _LIBCPP_NOEXCEPT_RETURN(std::__invoke(std::forward<_Fn>(__f), std::get<_Id>(std::forward<_Tuple>(__t))...))
      |                             ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1423:1: note: in instantiation of exception specification for '__apply_tuple_impl<(lambda at include/rice/rice.hpp:354:14), std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &, 0UL, 1UL>' requested here
 1423 | __apply_tuple_impl(_Fn&& __f, _Tuple&& __t, __tuple_indices<_Id...>)
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1428:34: note: in instantiation of function template specialization 'std::__apply_tuple_impl<(lambda at include/rice/rice.hpp:354:14), std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &, 0UL, 1UL>' requested here
 1428 |     _LIBCPP_NOEXCEPT_RETURN(std::__apply_tuple_impl(
      |                                  ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1427:55: note: (skipping 3 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all)
 1427 | inline _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) apply(_Fn&& __f, _Tuple&& __t)
      |                                                       ^
include/rice/rice.hpp:4904:5: note: in instantiation of member function 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::matches' requested here
 4904 |     NativeFunction(VALUE klass, std::string method_name, Function_T function, MethodInfo* methodInfo);
      |     ^
include/rice/rice.hpp:4973:44: note: in instantiation of member function 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::NativeFunction' requested here
 4973 |     NativeFunction_T* nativeFunction = new NativeFunction_T(klass, method_name, std::forward<Function_T>(function), methodInfo);
      |                                            ^
include/rice/rice.hpp:9043:54: note: in instantiation of member function 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::define' requested here
 9043 |     detail::NativeFunction<T, Function_T, IsMethod>::define(klass, name, std::forward<Function_T>(function), methodInfo);
      |                                                      ^
include/rice/rice.hpp:8572:9: note: in instantiation of function template specialization 'Rice::Data_Type<operations_research::KnapsackSolver>::wrap_native_call<true, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string)>' requested here
 8572 |   this->wrap_native_call<true>(this->value(), name, std::forward<Function_T>(func), methodInfo);
      |         ^
../../../../ext/or-tools/bin_packing.cpp:40:6: note: in instantiation of function template specialization 'Rice::Data_Type<operations_research::KnapsackSolver>::define_constructor<Rice::Constructor<operations_research::KnapsackSolver, operations_research::KnapsackSolver::SolverType, std::string>>' requested here
   40 |     .define_constructor(Rice::Constructor<KnapsackSolver, KnapsackSolver::SolverType, std::string>())

/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1424:29: error: attempt to use a deleted function
 1424 |     _LIBCPP_NOEXCEPT_RETURN(std::__invoke(std::forward<_Fn>(__f), std::get<_Id>(std::forward<_Tuple>(__t))...))
      |                             ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1423:1: note: in instantiation of exception specification for '__apply_tuple_impl<(lambda at include/rice/rice.hpp:354:14), std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &, 0UL, 1UL>' requested here
 1423 | __apply_tuple_impl(_Fn&& __f, _Tuple&& __t, __tuple_indices<_Id...>)
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1428:34: note: in instantiation of function template specialization 'std::__apply_tuple_impl<(lambda at include/rice/rice.hpp:354:14), std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &, 0UL, 1UL>' requested here
 1428 |     _LIBCPP_NOEXCEPT_RETURN(std::__apply_tuple_impl(
      |                                  ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/tuple:1427:55: note: in instantiation of exception specification for 'apply<(lambda at include/rice/rice.hpp:354:14), std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &>' requested here
 1427 | inline _LIBCPP_HIDE_FROM_ABI constexpr decltype(auto) apply(_Fn&& __f, _Tuple&& __t)
      |                                                       ^
include/rice/rice.hpp:354:8: note: in instantiation of function template specialization 'std::apply<(lambda at include/rice/rice.hpp:354:14), std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &>' requested here
  354 |   std::apply([&callable](auto&& ...args)
      |        ^
include/rice/rice.hpp:5041:5: note: in instantiation of function template specialization 'for_each_tuple<std::tuple<Rice::detail::From_Ruby<KnapsackSolver::SolverType>, Rice::detail::From_Ruby<std::string>> &, (lambda at include/rice/rice.hpp:5042:7)>' requested here
 5041 |     for_each_tuple(this->fromRubys_,
      |     ^
include/rice/rice.hpp:4904:5: note: in instantiation of member function 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::matches' requested here
 4904 |     NativeFunction(VALUE klass, std::string method_name, Function_T function, MethodInfo* methodInfo);
      |     ^
include/rice/rice.hpp:4973:44: note: in instantiation of member function 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::NativeFunction' requested here
 4973 |     NativeFunction_T* nativeFunction = new NativeFunction_T(klass, method_name, std::forward<Function_T>(function), methodInfo);
      |                                            ^
include/rice/rice.hpp:9043:54: note: in instantiation of member function 'Rice::detail::NativeFunction<operations_research::KnapsackSolver, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string), true>::define' requested here
 9043 |     detail::NativeFunction<T, Function_T, IsMethod>::define(klass, name, std::forward<Function_T>(function), methodInfo);
      |                                                      ^
include/rice/rice.hpp:8572:9: note: in instantiation of function template specialization 'Rice::Data_Type<operations_research::KnapsackSolver>::wrap_native_call<true, void (*)(unsigned long, operations_research::KnapsackSolver::SolverType, std::string)>' requested here
 8572 |   this->wrap_native_call<true>(this->value(), name, std::forward<Function_T>(func), methodInfo);
      |         ^
../../../../ext/or-tools/bin_packing.cpp:40:6: note: in instantiation of function template specialization 'Rice::Data_Type<operations_research::KnapsackSolver>::define_constructor<Rice::Constructor<operations_research::KnapsackSolver, operations_research::KnapsackSolver::SolverType, std::string>>' requested here
   40 |     .define_constructor(Rice::Constructor<KnapsackSolver, KnapsackSolver::SolverType, std::string>())
      |      ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__type_traits/nat.h:25:3: note: '~__nat' has been explicitly marked deleted here
   25 |   ~__nat()                       = delete;

and a few warnings:

include/rice/rice.hpp:2350:10: warning: private field 'arg_' is not used [-Wunused-private-field]
 2350 |     Arg* arg_ = nullptr;
      |          ^
include/rice/rice.hpp:10109:10: warning: private field 'arg_' is not used [-Wunused-private-field]
 10109 |     Arg* arg_ = nullptr;

[cfis]

@ankane - this repo?

https://github.com/ankane/or-tools-ruby/tree/master/ext/or-tools

From the errors, any custom From_Ruby classes will need to either add or update the is_convertible method. The deleted method sounds like someplace is trying to call a non-available copy constructor.

I'll take a look and post to that repo.

[cfis]

Sent a MR. Since you had custom From_Ruby implementations, the code no longer compiled since they must have a #is_convertible method.

That's an argument for calling this 5.0 since it not backwards compatible. But I doubt many users have implemented their own From_Ruby classes.

[ankane]

Thanks, can confirm it works once the header is regenerated.

[jasonroelofs]

My thinking right now is given Ruby 3.4 is coming out next week, we can make sure Rice is working fine there then put out a new release, 4.4 (I don't think we need to go to 5.0, there's a few things hard-coded to "4.x" that really don't need to change) in January. That sound good?

[cfis]

I'll definitely add 3.4 to the test mix. Not quite ready for a release...thinking early January? I'm ok with 4.4.

[cfis]

Update - there is one last missing piece for OpenCV support which is supporting C style callbacks. So was going to see if I can get that to work and then call this a release

[jasonroelofs]

Works for me!

[cfis]

So Ruby 3.4 does not work. There are two bugs. One embedding bug and one GC bug.

Embedding bug - https://bugs.ruby-lang.org/issues/20996

I haven't reported the GC bug yet.

[jasonroelofs]

Weird. From the stack traces my first guess would definitely be that something isn't getting initialized properly (and Ruby loves to change startup requirements, are there any core tests that actually document the embedding API?)

[cfis]

No idea - I'm not even sure there is official documentation on how to do it.

[cfis]

Haven't gotten any feedback on the issue. Maybe both of you could chime in (especially @ankane with your gems)?

[cfis]

Update - I have completed all the changes I would like to make for a new release. Lot and lots of new functionality. I now need to update the docs, which will take some time.

Unfortunately, no progress on Ruby 3.4 support.

[cfis]

Thanks for looking - good luck!

[jasonroelofs]

Ug, unfortunately it's GC related. Something we're tracking is getting collected under rb_gc_start().

I've opened a PR that should at least complete the tests without crashing, though there are some other failures related to 3.4 #242

[jasonroelofs]

And it looks like this is not due to any recent change. Grabbing the last release tag has the same crash under Ruby 3.4.

[cfis]

Yeah its a Ruby gc bug - see my comments on the PR. I was hoping the embedding bug would get fixed before reporting the GC bug to the Ruby bug tracker, but I guess not....

[jasonroelofs]

Incidentally I may have found where this broke: ruby/ruby@8077144

[cfis]

Thanks @jasonroelofs on getting Ruby 3.4 working. In the meantime, I have significantly updated the documentation. I went with version 4.5 to kinda sort of indicate this is a major release. I just pushed the documentation updates to the website (since they are mostly applicable to the current version of Rice too). I plan on updating the docs to discuss auto generating bindings using the new ruby-bindgen gem I have written, but I can do that after a release.

The one last thing I would like to do is enable FFI on MacOS. When I enabled it before it crashed :( Currently it is disabled, see:

https://github.com/ruby-rice/rice/blob/master/lib/mkmf-rice.rb#L70

I don't actually have a Mac to test on. So I have been using MacOS running in a virtual qemu/kvm but that is slow and painful. I can try to do that again, or find myself a Mac. Or if you have time Jason and can look at it that would be great. The crash happens in the test_Callbacks.cpp.

[cfis]

Thanks for investigating. I took the setup code from the ffi project. So maybe something here:

ffiStatus = ffi_prep_closure_loc(closure->pcl, &mh_cif, attached_method_invoke, closure, code);

The FFI code is a bit more complicated because it has a closure pool (https://github.com/ffi/ffi/blob/master/ext/ffi_c/ClosurePool.c)

[cfis]

By the way, I tried to explain why we are using libffi here - https://ruby-rice.github.io/4.x/bindings/callbacks.html#associating-callbacks-with-procs

See if that makes sense, I couldn't come up with a better solution after thinking about it quite a long time.

[jasonroelofs]

I think I got it, and while my guess that the nullptr was the problem, it took quite a bit of digging to figure out why and what was supposed to go there. #244

[ankane]

It'd be nice to make linking libffi opt-in (or at least optional), as I don't think most gems that use Rice should need this.

[cfis]

Yes, it is opt-in. It depends if HAVE_LIBFFI is defined.

That is here:

https://github.com/ruby-rice/rice/blob/master/lib/mkmf-rice.rb#L71

Then checked here when Rice comes across a C style callback:

https://github.com/ruby-rice/rice/blob/master/rice/detail/Proc.ipp#L61

The FFI specific code is here:

https://github.com/ruby-rice/rice/blob/master/rice/detail/NativeCallbackFFI.hpp#L4
https://github.com/ruby-rice/rice/blob/master/rice/detail/NativeCallbackFFI.ipp#L1

And tests here:

https://github.com/ruby-rice/rice/blob/master/test/test_Callback.cpp#L156

See if that works for you.

Although opt-in, my assumption is that it will always be installed because Ruby itself uses libFFI for fiddle. And since you are building a Ruby extension...that means you have libFFI.

I have tested with/without HAVE_LIBFFI defined and things work fine. Admittedly I haven't done that for a couple of week though, but none of that code has changed.

[cfis]

That was the last item on my list. We do need to update include/headers before a release.

@jasonroelofs anything else on yours (the code to disable GC was never checked in right)?

@ankane Do you have time to give this new version a spin first or should we just do a release?

I called it 4.5 in the changelog. Oh looking at the changelog, we should add dates, I always find that helpful on other projects...

[ankane]

Yeah, exactly, or a separate class if that's possible, like KwArg("some_name").

[cfis]

I like the KwArg idea better. Although maybe ArgKw? But do we need the added complexity of that? I don't think so. @jasonroelofs thoughts?

An unfortunate reality is the rb_scan_args format argument doesn't work well with keywords. For example, assume you have a method that takes two required keywords. The format argument would be 20: . But that blows up, because the required parameter only applies to positional arguments, not keywords. Eventually I gave up and just wrote my own code, see:

https://github.com/ruby-rice/rice/blob/master/rice/detail/NativeFunction.ipp#L155

Thinking through how this would work, this line needs to be updated:

https://github.com/ruby-rice/rice/blob/master/rice/detail/NativeFunction.ipp#L180

And then logic would have to be added to MethodInfo to throw an error if someone defines a Arg after a KwArg. So its doable to implement, but is it worth it?

[jasonroelofs]

Magnus uses the terms args and kwargs (https://github.com/matsadler/magnus/blob/main/src/into_value.rs#L101), and if I'm reading other code right, it goes through a lot of logic to handle the same cases we're talking about here.

I originally designed Arg(string) with the idea that it would hopefully Just Work when Ruby moved to keyword arguments, but reality and backwards compatibility have definitely, unfortunately, made that more complex.

I see where @ankane is coming from, but it does feel like a lot of extra work to adhere more closely to what has become a rather ridiculous number of options for passing in arguments with Ruby. I'd be ok leaving things as @cfis has them, possibly document this case, and if it ends up being more difficult in real use, revisit it later.

[cfis]

Ah interesting on Magnus - good find.

Ok, so let's go with what we have now but if this turns out to be an issue we will add specific keyword argument handling. Agreed on documenting this in the keywords argument section - I can do that.

[cfis]

Updated docs to mention this problem - 07d0bb6

[ankane]

Just tested with seven of my projects and everything seems to work fine, fwiw.

[cfis]

Planning on doing a 4.5 release this weekend...

[cfis]

And this is done! Thanks @jasonroelofs and @ankane for your help on this.

[ankane]

Great, congrats!