ruby-rice / rice

Ruby Interface for C++ Extensions
http://ruby-rice.github.io/
Other
378 stars 63 forks source link

Implict Casts #155

Closed cfis closed 3 years ago

cfis commented 3 years ago

The last issue to deal with is implicit casts.

In Rice 3.0x and earlier, casts used converting constructors, MyClass(SomeOtherClass& object), and worked on pointers. When a cast was invoked, a new instance was created (using new) and thus always resulted in a memory leak (because nothing ever freed the newly created C++ object).

Rice 4.0 adds in support for conversion operators, SomeOtherClass operator MyClass(), and adds support for values and references in addition to pointers.

However, when converting from pointers Rice 4.0 also causes memory leaks. And worse, for references, it returns a reference to a local object and will crash. So clearly this needs to be fixed.

The fundamental problem is that the casts are checked in From_Ruby. That results ins some code complexity and also makes it impossible to deal correctly with memory management. Say you have a Farenheit object, and the From_Ruby code casts it to a Celsius object. It is directly returned to C++ so no Ruby object wraps it.

So I don't have any great options, but here are some:

@jasonroelofs any better ideas?

jasonroelofs commented 3 years ago

I added define_implicit_cast when I was playing around with wrapping the Ogre rendering engine (Radians <=> Degrees). As far as I'm aware, I'm the only person who ever wanted such a thing, and I guess it's kind of showing my naivety of C++ and memory management!

So, lets just get rid of it. The added complexity isn't worth the effort.

cfis commented 3 years ago

Ok great, this functionality is now removed. Its not too hard to do one-offs when needed. Meaning:


define_method("to_radians", [](const Degree& degree)
{
   Radian* radian  = new Radian(degree);
   Data_Object<Radian> dataObject(radian, Data_Type<Radian>::klass(), true); // True to tell ruby to own this new object
   return dataObject;
});