ruby-rice / rice

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

Automatic `frozen?` check #186

Closed uvlad7 closed 10 months ago

uvlad7 commented 1 year ago

I think it would be nice to add an ability to tell rice to automatically throw FrozenError inside define_method/define_function. May be check std::is_const inside invokeNativeMethod

jasonroelofs commented 10 months ago

I know it's been a while since you posted this but I'm not sure I follow what you're asking for here. Could you provide a more concrete example?

uvlad7 commented 10 months ago

Something like that

  template<typename From_Ruby_T, typename Function_T, bool IsMethod>
  VALUE NativeFunction<From_Ruby_T, Function_T, IsMethod>::invokeNativeMethod(VALUE self, const Arg_Ts& nativeArgs)
  {
    Class_T receiver = this->getReceiver(self);
    auto selfAndNativeArgs = std::tuple_cat(std::forward_as_tuple(receiver), nativeArgs);
    #ifdef AUTO_FROZEN_CHECK
    if constexpr (std::is_const<Class_T>)
    {
      if (selfAndNativeArgs.get<0>()->is_frozen())
      {
        throw Rice::Exception(rb_eFrozenError, "Cannot modify frozen object");
      }
    }
    #endif

(Not sure if it's gonna work like that, I don't really know C++)

jasonroelofs commented 10 months ago

It might be possible, I'm not sure, but I'm not sure how useful this would be. Is there a use case you're running into that led to this question?

uvlad7 commented 10 months ago

Well, I was working on this project and thought it would be nice and more Rubyish to check if the object is frozen in the methods that modify it without the need to wrap them into additional functions/macros.

I don't remember the details, it's been a while since I posted this and I've since abandoned the project (I'll probably continue to work on it in the future, but for now I've chosen to learn Rust over C++ and I play with Magnus instead of Rice).

jasonroelofs commented 10 months ago

Gotcha. Thanks for the suggestion, I'll keep it in mind but will probably wait for some other people to suggest similar before diving into this.

Also, Magnus looks slick, I'll definitely keep an eye on that one!