From ec94c59d6d2dd5af4dc84843637cb3d427d46e17 Mon Sep 17 00:00:00 2001 From: cfis Date: Wed, 18 Feb 2026 23:28:13 -0700 Subject: [PATCH 1/2] Fix path that did not return value. --- rice/detail/Parameter.ipp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rice/detail/Parameter.ipp b/rice/detail/Parameter.ipp index c9f70209..d1ba42a5 100644 --- a/rice/detail/Parameter.ipp +++ b/rice/detail/Parameter.ipp @@ -165,6 +165,10 @@ namespace Rice::detail T defaultValue = this->arg()->template defaultValue(); return this->toRuby_.convert(defaultValue); } + else + { + throw std::runtime_error("Default value not allowed for parameter " + this->arg()->name); + } } else { From 1aaafcb4f204a7fa2eb7bb5d84b156d14e878865 Mon Sep 17 00:00:00 2001 From: cfis Date: Wed, 18 Feb 2026 23:29:39 -0700 Subject: [PATCH 2/2] Be a bit more lenient on wrapped Ruby VALUEs. Fix #391. --- rice/Data_Type.ipp | 2 +- rice/cpp_api/Object.hpp | 15 ++++--- rice/cpp_api/Object.ipp | 70 ++++++++++++++++++--------------- rice/cpp_api/String.ipp | 8 ++-- rice/cpp_api/shared_methods.hpp | 8 ++-- test/test_Object.cpp | 14 +------ 6 files changed, 60 insertions(+), 57 deletions(-) diff --git a/rice/Data_Type.ipp b/rice/Data_Type.ipp index dee22406..91b4a269 100644 --- a/rice/Data_Type.ipp +++ b/rice/Data_Type.ipp @@ -368,7 +368,7 @@ namespace Rice template inline Data_Type& Data_Type::define_singleton_attr(std::string name, Attribute_T attribute, Access_T access, const Arg_Ts&...args) { - VALUE singleton = detail::protect(rb_singleton_class, this->value()); + VALUE singleton = detail::protect(rb_singleton_class, this->validated_value()); return this->define_attr_internal(singleton, name, std::forward(attribute), access, args...); } diff --git a/rice/cpp_api/Object.hpp b/rice/cpp_api/Object.hpp index 3dc8a10f..2d84ee37 100644 --- a/rice/cpp_api/Object.hpp +++ b/rice/cpp_api/Object.hpp @@ -37,6 +37,12 @@ namespace Rice Object(Object&& other) = default; Object& operator=(Object&& other) = default; + //! Implicit conversion to VALUE. + operator VALUE() const; + + //! Explicitly get the encapsulated VALUE. + VALUE value() const; + //! Returns false if the object is nil or false; returns true //! otherwise. explicit operator bool() const; @@ -44,11 +50,6 @@ namespace Rice //! Returns true if the object is nil, false otherwise. bool is_nil() const; - //! Implicit conversion to VALUE. - operator VALUE() const; - - //! Explicitly get the encapsulated VALUE. - VALUE value() const; //! Get the class of an object. /*! \return the object's Class. @@ -249,6 +250,10 @@ namespace Rice void remove_const(Identifier name); protected: + //! Checks the encapsulated VALUE is not nil and returns it. If it is nil + //! an exception is thrown. + VALUE validated_value() const; + //! Set the encapsulated value. void set_value(VALUE value); diff --git a/rice/cpp_api/Object.ipp b/rice/cpp_api/Object.ipp index 294a19f0..0ad96b4b 100644 --- a/rice/cpp_api/Object.ipp +++ b/rice/cpp_api/Object.ipp @@ -8,16 +8,9 @@ namespace Rice { } - inline Object::operator bool() const - { - // Bypass getter to not raise exception - return RTEST(this->value_.value()); - } - - inline bool Object::is_nil() const + inline VALUE Object::value() const { - // Bypass getter to not raise exception - return NIL_P(this->value_.value()); + return this->value_.value(); } inline Object::operator VALUE() const @@ -25,9 +18,9 @@ namespace Rice return this->value(); } - inline VALUE Object::value() const + inline VALUE Object::validated_value() const { - VALUE result = this->value_.value(); + VALUE result = this->value(); if (result == Qnil) { @@ -38,6 +31,16 @@ namespace Rice return result; } + inline Object::operator bool() const + { + return RTEST(this->value()); + } + + inline bool Object::is_nil() const + { + return NIL_P(this->value()); + } + template inline Object Object::call(Identifier id, Parameter_Ts... args) const { @@ -49,7 +52,7 @@ namespace Rice easy to duplicate by setting GC.stress to true and calling a constructor that takes multiple values like a std::pair wrapper. */ std::array values = { detail::To_Ruby>().convert(std::forward(args))... }; - return detail::protect(rb_funcallv, value(), id.id(), (int)values.size(), (const VALUE*)values.data()); + return detail::protect(rb_funcallv, this->validated_value(), id.id(), (int)values.size(), (const VALUE*)values.data()); } template @@ -57,13 +60,13 @@ namespace Rice { /* IMPORTANT - See call() above */ std::array values = { detail::To_Ruby>().convert(args)... }; - return detail::protect(rb_funcallv_kw, value(), id.id(), (int)values.size(), (const VALUE*)values.data(), RB_PASS_KEYWORDS); + return detail::protect(rb_funcallv_kw, this->validated_value(), id.id(), (int)values.size(), (const VALUE*)values.data(), RB_PASS_KEYWORDS); } template inline void Object::iv_set(Identifier name, T const& value) { - detail::protect(rb_ivar_set, this->value(), name.id(), detail::To_Ruby().convert(value)); + detail::protect(rb_ivar_set, this->validated_value(), name.id(), detail::To_Ruby().convert(value)); } inline int Object::compare(Object const& other) const @@ -79,66 +82,71 @@ namespace Rice return this->is_nil() && other.is_nil(); } - VALUE result = detail::protect(rb_equal, this->value(), other.value()); + VALUE result = detail::protect(rb_equal, this->validated_value(), other.validated_value()); return RB_TEST(result); } inline bool Object::is_eql(const Object& other) const { - VALUE result = detail::protect(rb_eql, this->value(), other.value()); + if (this->is_nil() || other.is_nil()) + { + return this->is_nil() && other.is_nil(); + } + + VALUE result = detail::protect(rb_eql, this->validated_value(), other.validated_value()); return RB_TEST(result); } inline void Object::freeze() { - detail::protect(rb_obj_freeze, value()); + detail::protect(rb_obj_freeze, this->validated_value()); } inline bool Object::is_frozen() const { - return RB_OBJ_FROZEN(value()); + return RB_OBJ_FROZEN(this->validated_value()); } inline int Object::rb_type() const { - return ::rb_type(this->value()); + return ::rb_type(this->validated_value()); } inline VALUE Object::object_id() const { - return detail::protect(rb_obj_id, this->value()); + return detail::protect(rb_obj_id, this->validated_value()); } inline bool Object::is_a(Object klass) const { - VALUE result = detail::protect(rb_obj_is_kind_of, this->value(), klass.value()); + VALUE result = detail::protect(rb_obj_is_kind_of, this->validated_value(), klass.validated_value()); return RB_TEST(result); } inline void Object::extend(Module const& mod) { - detail::protect(rb_extend_object, this->value(), mod.value()); + detail::protect(rb_extend_object, this->validated_value(), mod.validated_value()); } inline bool Object::respond_to(Identifier id) const { - return bool(rb_respond_to(this->value(), id.id())); + return bool(rb_respond_to(this->validated_value(), id.id())); } inline bool Object::is_instance_of(Object klass) const { - VALUE result = detail::protect(rb_obj_is_instance_of, this->value(), klass.value()); + VALUE result = detail::protect(rb_obj_is_instance_of, this->validated_value(), klass.validated_value()); return RB_TEST(result); } inline Object Object::iv_get(Identifier name) const { - return detail::protect(rb_ivar_get, this->value(), name.id()); + return detail::protect(rb_ivar_get, this->validated_value(), name.id()); } inline Object Object::attr_get(Identifier name) const { - return detail::protect(rb_attr_get, this->value(), name.id()); + return detail::protect(rb_attr_get, this->validated_value(), name.id()); } inline void Object::set_value(VALUE value) @@ -148,20 +156,20 @@ namespace Rice inline Object Object::const_get(Identifier name) const { - return detail::protect(rb_const_get, this->value(), name.id()); + return detail::protect(rb_const_get, this->validated_value(), name.id()); } inline bool Object::const_defined(Identifier name) const { - size_t result = detail::protect(rb_const_defined, this->value(), name.id()); + size_t result = detail::protect(rb_const_defined, this->validated_value(), name.id()); return bool(result); } inline Object Object::const_set(Identifier name, Object value) { // We will allow setting constants to Qnil, or the decimal value of 4. This happens - // in C++ libraries with enums. Thus skip the value() method that raises excptions - detail::protect(rb_const_set, this->value(), name.id(), value.value_.value()); + // in C++ libraries with enums. Thus use value() instead of validated_value + detail::protect(rb_const_set, this->validated_value(), name.id(), value.value()); return value; } @@ -176,7 +184,7 @@ namespace Rice inline void Object::remove_const(Identifier name) { - detail::protect(rb_mod_remove_const, this->value(), name.to_sym()); + detail::protect(rb_mod_remove_const, this->validated_value(), name.to_sym()); } inline bool operator==(Object const& lhs, Object const& rhs) diff --git a/rice/cpp_api/String.ipp b/rice/cpp_api/String.ipp index a59b3c33..252c2b21 100644 --- a/rice/cpp_api/String.ipp +++ b/rice/cpp_api/String.ipp @@ -47,22 +47,22 @@ namespace Rice inline size_t String::length() const { - return RSTRING_LEN(value()); + return RSTRING_LEN(this->value()); } inline char String::operator[](ptrdiff_t index) const { - return RSTRING_PTR(value())[index]; + return RSTRING_PTR(this->value())[index]; } inline char const* String::c_str() const { - return RSTRING_PTR(value()); + return RSTRING_PTR(this->value()); } inline std::string String::str() const { - return std::string(RSTRING_PTR(value()), length()); + return std::string(RSTRING_PTR(this->value()), length()); } template diff --git a/rice/cpp_api/shared_methods.hpp b/rice/cpp_api/shared_methods.hpp index 65a4889e..e107e119 100644 --- a/rice/cpp_api/shared_methods.hpp +++ b/rice/cpp_api/shared_methods.hpp @@ -9,7 +9,7 @@ */ inline auto& include_module(Module const& inc) { - detail::protect(rb_include_module, this->value(), inc.value()); + detail::protect(rb_include_module, this->validated_value(), inc.value()); return *this; } @@ -33,7 +33,7 @@ inline auto& include_module(Module const& inc) template inline auto& define_method(std::string name, Method_T&& method, const Arg_Ts&...args) { - this->wrap_native_method(this->value(), name, std::forward(method), args...); + this->wrap_native_method(this->validated_value(), name, std::forward(method), args...); return *this; } @@ -51,7 +51,7 @@ inline auto& define_method(std::string name, Method_T&& method, const Arg_Ts&... template inline auto& define_function(std::string name, Function_T&& func, const Arg_Ts&...args) { - this->wrap_native_function(this->value(), name, std::forward(func), args...); + this->wrap_native_function(this->validated_value(), name, std::forward(func), args...); return *this; } @@ -125,6 +125,6 @@ template inline auto& define_constant(std::string name, Constant_T value) { using Base_T = detail::remove_cv_recursive_t; - detail::protect(rb_define_const, this->value(), name.c_str(), detail::To_Ruby().convert(value)); + detail::protect(rb_define_const, this->validated_value(), name.c_str(), detail::To_Ruby().convert(value)); return *this; } \ No newline at end of file diff --git a/test/test_Object.cpp b/test/test_Object.cpp index 3911c5fa..f366e874 100644 --- a/test/test_Object.cpp +++ b/test/test_Object.cpp @@ -97,12 +97,7 @@ TESTCASE(implicit_conversion_to_value) ASSERT_EQUAL(INT2NUM(42), (VALUE)Object(INT2NUM(42))); ASSERT_EQUAL(Qfalse, (VALUE)Object(Qfalse)); ASSERT_EQUAL(Qundef, (VALUE)Object(Qundef)); - - ASSERT_EXCEPTION_CHECK( - std::runtime_error, - (VALUE)Object(Qnil), - ASSERT_EQUAL("Rice Object does not wrap a Ruby object", ex.what()) - ); + ASSERT_EQUAL(Qnil, (VALUE)Object(Qnil)); } TESTCASE(explicit_conversion_to_value) @@ -111,12 +106,7 @@ TESTCASE(explicit_conversion_to_value) ASSERT_EQUAL(INT2NUM(42), Object(INT2NUM(42)).value()); ASSERT_EQUAL(Qfalse, Object(Qfalse).value()); ASSERT_EQUAL(Qundef, Object(Qundef).value()); - - ASSERT_EXCEPTION_CHECK( - std::runtime_error, - Object(Qnil).value(), - ASSERT_EQUAL("Rice Object does not wrap a Ruby object", ex.what()) - ); + ASSERT_EQUAL(Qnil, Object(Qnil).value()); } TESTCASE(class_of)