From 02a4da27730f1887734a5d518c61c4d0d728e4fe Mon Sep 17 00:00:00 2001 From: cfis Date: Mon, 16 Feb 2026 22:15:27 -0800 Subject: [PATCH 1/8] * `Object()` now defaults to `Qnil` instead of `rb_cObject`. This avoids accidentally manipulating `rb_cObject` when a wrapper is not explicitly initialized. * C++ API wrappers now store their Ruby `VALUE` in a `Pin` instead of a raw `VALUE` field. Previously, wrappers were not GC-safe and Ruby could reclaim wrapped objects while C++ still referenced them. This is now fixed, and wrappers such as `Object` can be stored safely in containers like `std::vector`. --- CHANGELOG.md | 6 +- docs/cpp_api/class.md | 6 +- docs/cpp_api/module.md | 6 +- docs/cpp_api/object.md | 25 ++-- docs/migration.md | 236 ++++++++++++++++++++++++++++++++ rice/Data_Object.ipp | 2 +- rice/Pin.ipp | 10 +- rice/cpp_api/Module.hpp | 6 +- rice/cpp_api/Module.ipp | 4 - rice/cpp_api/Object.hpp | 29 ++-- rice/cpp_api/Object.ipp | 62 ++++----- rice/cpp_api/Struct.ipp | 4 +- rice/detail/Anchor.hpp | 9 +- rice/detail/Anchor.ipp | 13 +- test/test_Array.cpp | 2 +- test/test_Builtin_Object.cpp | 8 +- test/test_Callback.cpp | 2 +- test/test_Class.cpp | 25 +++- test/test_Data_Type.cpp | 38 +---- test/test_Exception.cpp | 4 +- test/test_Object.cpp | 42 ++++-- test/test_Stl_Function.cpp | 2 +- test/test_Stl_Map.cpp | 2 +- test/test_Stl_OStream.cpp | 4 +- test/test_Stl_Optional.cpp | 6 +- test/test_Stl_Pair.cpp | 2 +- test/test_Stl_Unordered_Map.cpp | 2 +- test/test_Stl_Variant.cpp | 6 +- test/test_Stl_Vector.cpp | 30 ++-- test/test_Struct.cpp | 6 +- 30 files changed, 416 insertions(+), 183 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ba2a0595..967a3cca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,14 @@ # Changelog -## Unreleased +## 4.11.0 (Unreleased) Incompatible Changes: * `InstanceRegistry.isEnabled` (boolean) has been replaced by an `InstanceRegistry.isEnabled` which is an enum (`Off`, `Owned`, `All`). * `InstanceRegistry` now defaults to `Owned` - previously it was disabled. The goal of this change is to ensure C++ objects owned by Ruby are only wrapped once to avoid double free errors. +* `Object()` now defaults to `Qnil` instead of `rb_cObject`. This avoids accidentally manipulating `rb_cObject` when a wrapper is not explicitly initialized. Calling methods on wrappers that point to `Qnil` will generally raise an exception. Use `object.is_nil()` to check for `nil` before using a wrapper as a receiver. +* C++ API wrappers now store their Ruby `VALUE` in a `Pin` instead of a raw `VALUE` field. Previously, wrappers were not GC-safe and Ruby could reclaim wrapped objects while C++ still referenced them. This is now fixed, and wrappers such as `Object` can be stored safely in containers like `std::vector`. +* The global `Object` constants (`Rice::Nil`, `Rice::True`, `Rice::False`, `Rice::Undef`) have been removed. +* `Object::test()` has been removed. Use `operator bool()` or `is_nil()` depending on the desired semantics. ## 4.10.0 (2026-02-07) diff --git a/docs/cpp_api/class.md b/docs/cpp_api/class.md index 19b23214..784d4cd9 100644 --- a/docs/cpp_api/class.md +++ b/docs/cpp_api/class.md @@ -12,12 +12,14 @@ ### Class() -Construct a Class wrapping `rb_cObject`. +Default-construct an empty Class wrapper (wraps `Qnil`). ```cpp -Class c; // wraps Object class +Class c; // wraps nil ``` +This constructor does not bind the wrapper to a Ruby class. Initialize it before calling class APIs. + --- ### Class(VALUE v) diff --git a/docs/cpp_api/module.md b/docs/cpp_api/module.md index 99653845..04b785a4 100644 --- a/docs/cpp_api/module.md +++ b/docs/cpp_api/module.md @@ -12,12 +12,14 @@ ### Module() -Construct a Module wrapping `rb_cObject`. +Default-construct an empty Module wrapper (wraps `Qnil`). ```cpp -Module m; // wraps Object +Module m; // wraps nil ``` +This constructor does not bind the wrapper to a Ruby module. Initialize it before calling module APIs. + --- ### Module(VALUE v) diff --git a/docs/cpp_api/object.md b/docs/cpp_api/object.md index ebafc6fb..040b4160 100644 --- a/docs/cpp_api/object.md +++ b/docs/cpp_api/object.md @@ -6,6 +6,8 @@ `Rice::Object` is the base class for all Rice wrapper classes. It wraps a Ruby `VALUE` and provides a C++-style interface to Ruby's object system. +Note: `Object` stores its wrapped Ruby value using an internal `Pin`, so wrapper instances keep their Ruby `VALUE` protected from GC while the wrapper is alive. This makes long-lived C++ wrappers (including wrappers stored in STL containers) GC-safe. + --- ## Constructors @@ -18,6 +20,8 @@ Construct a new Object wrapping `Qnil`. Object obj; // wraps nil ``` +This is an intentionally "empty/nil" default state. It avoids accidentally targeting `rb_cObject` when a wrapper is default-constructed and later used without explicit initialization. + --- ### Object(VALUE value) @@ -63,29 +67,18 @@ Object obj(some_value); VALUE v = obj.value(); ``` +If the wrapper does not contain a usable receiver, Rice will raise an exception when the value is used by APIs that require a receiver. Check `is_nil()` before calling receiver-style methods. + --- -### test() const → bool +### operator bool() const -Test if the object is truthy. +Implicit conversion to bool. **Returns:** `false` if the object is `nil` or `false`; `true` otherwise. -```cpp -Object obj(Qtrue); -if (obj.test()) { - // ... -} -``` - ---- - -### operator bool() const - -Implicit conversion to bool. Same as `test()`. - ```cpp Object obj(some_value); if (obj) { @@ -110,6 +103,8 @@ if (obj.is_nil()) { } ``` +Use this to guard receiver-style operations when a wrapper may be empty/nil. + --- ### class_of() const → Class diff --git a/docs/migration.md b/docs/migration.md index fe0b0841..75555f39 100644 --- a/docs/migration.md +++ b/docs/migration.md @@ -1,5 +1,241 @@ # Migration +## Version 4.10 to 4.11 + +Version 4.11 introduces several breaking changes in instance tracking and the C++ API wrappers. + +### InstanceRegistry setting changed from boolean to mode enum + +`InstanceRegistry.isEnabled` no longer takes/returns a boolean-style state. It now uses a mode enum: +`Off`, `Owned`, `All`. + +Before: + +```ruby +Rice::InstanceRegistry.isEnabled = true +Rice::InstanceRegistry.isEnabled = false +``` + +After: + +```ruby +Rice::InstanceRegistry.isEnabled = Rice::InstanceRegistry::Owned +Rice::InstanceRegistry.isEnabled = Rice::InstanceRegistry::Off +Rice::InstanceRegistry.isEnabled = Rice::InstanceRegistry::All +``` + +Also note the default changed from disabled to `Owned`. If your code depended on registry-off behavior, set it explicitly to `Off`. + +### `Object()` now defaults to `Qnil` (not `rb_cObject`) + +Default-constructed wrappers are now nil wrappers. + +Before: + +```cpp +Object obj; +obj.call("some_method"); // could accidentally target rb_cObject +``` + +After: + +```cpp +Object obj; +if (!obj.is_nil()) +{ + obj.call("some_method"); +} +``` + +Or initialize explicitly: + +```cpp +Object obj(rb_cObject); +obj.call("some_method"); +``` + +### `Object::test()` removed + +Replace calls to `test()` with either `operator bool()` or `is_nil()`, depending on intent. + +Before: + +```cpp +if (obj.test()) +{ + // truthy +} +``` + +After: + +```cpp +if (obj) +{ + // truthy +} +``` + +Or, if you specifically need nil checks: + +```cpp +if (obj.is_nil()) +{ + // nil +} +``` + +### Global `Object` constants removed + +The convenience globals were removed: +`Rice::Nil`, `Rice::True`, `Rice::False`, `Rice::Undef`. + +Before: + +```cpp +Object value = Rice::Nil; +``` + +After: + +```cpp +Object value(Qnil); +Object truthy(Qtrue); +Object falsy(Qfalse); +Object undef_value(Qundef); +``` + +### C++ API wrappers now use `Pin` internally + +Wrapper classes (such as `Object`) now pin their wrapped Ruby value internally for GC safety. + +Most users do not need code changes for this update. The primary behavior change is improved safety for long-lived wrappers, including wrappers stored in containers: + +```cpp +std::vector values; +values.push_back(Object(rb_str_new_cstr("hello"))); +``` + +If you previously added ad-hoc GC guards only to protect wrapper objects, those guards may no longer be necessary for the wrappers themselves. + +## Version 4.7 to 4.10 + +Versions 4.8, 4.9, and 4.10 introduced several incompatible C++ API changes. + +### Buffer/GVL API renames (4.8) + +Before: + +```cpp +define_method("read", &MyClass::read, Arg("buffer").isBuffer()); +define_method("write", &MyClass::write, Return().isBuffer()); +define_method("compute", &MyClass::compute, Function().noGVL()); +``` + +After: + +```cpp +define_method("read", &MyClass::read, ArgBuffer("buffer")); +define_method("write", &MyClass::write, ReturnBuffer()); +define_method("compute", &MyClass::compute, NoGvL()); +``` + +### `is_convertible` return type changed (4.8) + +`From_Ruby::is_convertible` must return `double` instead of `Convertible`. + +Before: + +```cpp +Convertible is_convertible(VALUE value) +{ + return Convertible::Exact; +} +``` + +After: + +```cpp +double is_convertible(VALUE value) +{ + return Convertible::Exact; +} +``` + +### Method/default argument verification is stricter (4.8) + +Rice now validates default arguments and type registrations more aggressively. Code that previously compiled but had mismatched/default argument types may now raise errors during binding setup. + +Migration step: +1. Ensure every default argument value matches the bound C++ parameter type. +2. Ensure all custom/opaque types are registered before using them in defaults. + +### Smart pointer wrapper internals changed (4.9) + +If you implemented custom Rice-side smart pointer wrappers, update them to the current `Std::SharedPtr` / `Std::UniquePtr` model and forwarding behavior. + +Most users who only consume smart pointers through `define_method`/`define_constructor` do not need code changes. + +### `Address_Registration_Guard` replaced by `Pin` (4.10) + +Before: + +```cpp +VALUE value_; +Address_Registration_Guard guard_; + +MyClass() + : value_(rb_str_new2("test")), guard_(&value_) +{ +} +``` + +After: + +```cpp +Pin pin_; + +MyClass() + : pin_(rb_str_new2("test")) +{ +} + +VALUE value() const +{ + return pin_.value(); +} +``` + +### Blocks are converted to Procs in C++ bindings (4.10) + +If your C++ method expects a Ruby block, explicitly receive it as a `VALUE`/`Object` parameter and mark it as a value arg. + +Before: + +```cpp +define_method("run_with_block", [](VALUE self) +{ + // expected implicit block handling +}); +``` + +After: + +```cpp +define_method("run_with_block", [](VALUE self, VALUE proc) +{ + // proc is the Ruby block converted to Proc +}, Arg("proc").setValue()); +``` + +### `Data_Type::define()` removed (4.10) + +Use class template binding helpers / explicit factory functions instead of `Data_Type::define()`. + +Migration step: +1. Remove `Data_Type::define()` calls. +2. Replace with `define_class`/`define_class_under` flows documented in [Class Templates](bindings/class_templates.md). + ## Version 4.6 to 4.7 Version 4.7 has a couple of breaking changes. diff --git a/rice/Data_Object.ipp b/rice/Data_Object.ipp index 3a2f9928..bdbea50d 100644 --- a/rice/Data_Object.ipp +++ b/rice/Data_Object.ipp @@ -84,7 +84,7 @@ namespace Rice template inline T* Data_Object::get() const { - if (this->value() == Qnil) + if (this->is_nil()) { return nullptr; } diff --git a/rice/Pin.ipp b/rice/Pin.ipp index 50533bb9..16af1885 100644 --- a/rice/Pin.ipp +++ b/rice/Pin.ipp @@ -7,6 +7,14 @@ namespace Rice inline VALUE Pin::value() const { - return this->anchor_->get(); + // Anchor can be nil after a move + if (this->anchor_) + { + return this->anchor_->get(); + } + else + { + return Qnil; + } } } diff --git a/rice/cpp_api/Module.hpp b/rice/cpp_api/Module.hpp index 405d717b..310a67e2 100644 --- a/rice/cpp_api/Module.hpp +++ b/rice/cpp_api/Module.hpp @@ -18,8 +18,8 @@ namespace Rice class Module : public Object { public: - //! Default construct a Module and initialize it to rb_cObject. - Module(); + //! Default construct an empty Module wrapper. + Module() = default; //! Construct a Module from an existing Module object. Module(VALUE v); @@ -72,4 +72,4 @@ namespace Rice */ Module anonymous_module(); } -#endif // Rice__Module__hpp_ \ No newline at end of file +#endif // Rice__Module__hpp_ diff --git a/rice/cpp_api/Module.ipp b/rice/cpp_api/Module.ipp index 65cc712d..ac024afa 100644 --- a/rice/cpp_api/Module.ipp +++ b/rice/cpp_api/Module.ipp @@ -1,10 +1,6 @@ namespace Rice { - inline Module::Module() : Object(rb_cObject) - { - } - inline Module::Module(VALUE value) : Object(value) { if (::rb_type(value) != T_CLASS && ::rb_type(value) != T_MODULE) diff --git a/rice/cpp_api/Object.hpp b/rice/cpp_api/Object.hpp index 5945b8c3..3dc8a10f 100644 --- a/rice/cpp_api/Object.hpp +++ b/rice/cpp_api/Object.hpp @@ -20,30 +20,26 @@ namespace Rice class Object { public: + //! Construct an empty Object wrapper. + Object(); + //! Encapsulate an existing ruby object. - Object(VALUE value = Qnil) : value_(value) {} + Object(VALUE value); //! Destructor - virtual ~Object(); + virtual ~Object() = default; // Enable copying Object(const Object& other) = default; Object& operator=(const Object& other) = default; // Enable moving - Object(Object&& other); - Object& operator=(Object&& other); + Object(Object&& other) = default; + Object& operator=(Object&& other) = default; //! Returns false if the object is nil or false; returns true //! otherwise. - // Having this conversion also prevents accidental conversion to - // undesired integral types (e.g. long or int) by making the - // conversion ambiguous. - bool test() const; - - //! Returns false if the object is nil or false; returns true - //! otherwise. - operator bool() const; + explicit operator bool() const; //! Returns true if the object is nil, false otherwise. bool is_nil() const; @@ -257,7 +253,7 @@ namespace Rice void set_value(VALUE value); private: - volatile VALUE value_; + Pin value_; }; std::ostream& operator<<(std::ostream& out, Object const& obj); @@ -266,11 +262,6 @@ namespace Rice bool operator!=(Object const& lhs, Object const& rhs); bool operator<(Object const& lhs, Object const& rhs); bool operator>(Object const& lhs, Object const& rhs); - - extern Object const Nil; - extern Object const True; - extern Object const False; - extern Object const Undef; -} // namespace Rice +} #endif // Rice__Object__hpp_ diff --git a/rice/cpp_api/Object.ipp b/rice/cpp_api/Object.ipp index 1de99d3e..294a19f0 100644 --- a/rice/cpp_api/Object.ipp +++ b/rice/cpp_api/Object.ipp @@ -1,45 +1,23 @@ namespace Rice { - inline const Object Nil(Qnil); - inline const Object True(Qtrue); - inline const Object False(Qfalse); - inline const Object Undef(Qundef); - - // Ruby auto detects VALUEs in the stack, so when an Object gets deleted make sure - // to clean up in case it is on the stack - inline Object::~Object() - { - this->value_ = Qnil; - } - - // Move constructor - inline Object::Object(Object&& other) - { - this->value_ = other.value_; - other.value_ = Qnil; - } - - // Move assignment - inline Object& Object::operator=(Object&& other) + inline Object::Object() : value_(Qnil) { - this->value_ = other.value_; - other.value_ = Qnil; - return *this; } - inline bool Object::test() const + inline Object::Object(VALUE value) : value_(value) { - return RTEST(this->value()); } inline Object::operator bool() const { - return this->test(); + // Bypass getter to not raise exception + return RTEST(this->value_.value()); } inline bool Object::is_nil() const { - return NIL_P(this->value()); + // Bypass getter to not raise exception + return NIL_P(this->value_.value()); } inline Object::operator VALUE() const @@ -49,7 +27,15 @@ namespace Rice inline VALUE Object::value() const { - return this->value_; + VALUE result = this->value_.value(); + + if (result == Qnil) + { + std::string message = "Rice Object does not wrap a Ruby object"; + throw std::runtime_error(message.c_str()); + } + + return result; } template @@ -88,6 +74,11 @@ namespace Rice inline bool Object::is_equal(const Object& other) const { + if (this->is_nil() || other.is_nil()) + { + return this->is_nil() && other.is_nil(); + } + VALUE result = detail::protect(rb_equal, this->value(), other.value()); return RB_TEST(result); } @@ -152,7 +143,7 @@ namespace Rice inline void Object::set_value(VALUE value) { - value_ = value; + this->value_ = Pin(value); } inline Object Object::const_get(Identifier name) const @@ -168,7 +159,9 @@ namespace Rice inline Object Object::const_set(Identifier name, Object value) { - detail::protect(rb_const_set, this->value(), name.id(), value.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()); return value; } @@ -188,8 +181,7 @@ namespace Rice inline bool operator==(Object const& lhs, Object const& rhs) { - VALUE result = detail::protect(rb_equal, lhs.value(), rhs.value()); - return result == Qtrue ? true : false; + return lhs.is_equal(rhs); } inline bool operator!=(Object const& lhs, Object const& rhs) @@ -200,13 +192,13 @@ namespace Rice inline bool operator<(Object const& lhs, Object const& rhs) { Object result = lhs.call("<", rhs); - return result.test(); + return result; } inline bool operator>(Object const& lhs, Object const& rhs) { Object result = lhs.call(">", rhs); - return result.test(); + return result; } } diff --git a/rice/cpp_api/Struct.ipp b/rice/cpp_api/Struct.ipp index d8811eb7..b52ecbfd 100644 --- a/rice/cpp_api/Struct.ipp +++ b/rice/cpp_api/Struct.ipp @@ -15,7 +15,7 @@ namespace Rice inline Struct& Struct::define_member(Identifier name) { - if (value() != rb_cObject) + if (!this->is_nil()) { throw std::runtime_error("struct is already initialized"); } @@ -27,7 +27,7 @@ namespace Rice inline Array Struct::members() const { - if (value() == rb_cObject) + if (this->is_nil()) { // Struct is not yet defined return Array(members_.begin(), members_.end()); diff --git a/rice/detail/Anchor.hpp b/rice/detail/Anchor.hpp index caef56e6..1b1a3de9 100644 --- a/rice/detail/Anchor.hpp +++ b/rice/detail/Anchor.hpp @@ -35,13 +35,6 @@ namespace Rice //! Retrieve the currently anchored VALUE. VALUE get() const; - //! Replace the anchored VALUE. - /*! - * The GC root (address) remains unchanged; only the VALUE - * stored at that address is updated. - */ - void set(VALUE value); - private: static void disable(VALUE); static void registerExitHandler(); @@ -50,6 +43,8 @@ namespace Rice inline static bool exitHandlerRegistered_ = false; private: + bool registered_ = false; + //! GC-visible Ruby VALUE slot. VALUE value_ = Qnil; }; diff --git a/rice/detail/Anchor.ipp b/rice/detail/Anchor.ipp index 8f1c9a89..05543136 100644 --- a/rice/detail/Anchor.ipp +++ b/rice/detail/Anchor.ipp @@ -4,20 +4,22 @@ namespace Rice { inline Anchor::Anchor(VALUE value) : value_(value) { - if (value != Qnil) + if (!RB_SPECIAL_CONST_P(value)) { Anchor::registerExitHandler(); detail::protect(rb_gc_register_address, &this->value_); + this->registered_ = true; } } inline Anchor::~Anchor() { - if (Anchor::enabled_ && this->value_ != Qnil) + if (Anchor::enabled_ && this->registered_) { detail::protect(rb_gc_unregister_address, &this->value_); } // Ruby auto detects VALUEs in the stack, so make sure up in case this object is on the stack + this->registered_ = false; this->value_ = Qnil; } @@ -26,11 +28,6 @@ namespace Rice return this->value_; } - inline void Anchor::set(VALUE value) - { - this->value_ = value; - } - // This will be called by ruby at exit - we want to disable further unregistering inline void Anchor::disable(VALUE) { @@ -46,4 +43,4 @@ namespace Rice } } } -} \ No newline at end of file +} diff --git a/test/test_Array.cpp b/test/test_Array.cpp index ce04735c..fc695e94 100644 --- a/test/test_Array.cpp +++ b/test/test_Array.cpp @@ -76,7 +76,7 @@ TESTCASE(push_no_items) TESTCASE(push_one_item) { Array a; - a.push(Rice::True, false); + a.push(Object(Qtrue), false); ASSERT_EQUAL(1, a.size()); ASSERT_EQUAL(Qtrue, a[0]); } diff --git a/test/test_Builtin_Object.cpp b/test/test_Builtin_Object.cpp index 2d5e30b3..fce33ec5 100644 --- a/test/test_Builtin_Object.cpp +++ b/test/test_Builtin_Object.cpp @@ -56,8 +56,8 @@ TESTCASE(move_constructor) Builtin_Object b1(c.call("new")); Builtin_Object b2(std::move(b1)); - ASSERT_NOT_EQUAL(b2.value(), b1.value()); - ASSERT_EQUAL(b1.value(), Qnil); + ASSERT(!b2.is_nil()); + ASSERT(b1.is_nil()); } TESTCASE(move_assign) @@ -68,8 +68,8 @@ TESTCASE(move_assign) b2 = std::move(b1); - ASSERT_NOT_EQUAL(b2.value(), b1.value()); - ASSERT_EQUAL(b1.value(), Qnil); + ASSERT(!b2.is_nil()); + ASSERT(b1.is_nil()); } TESTCASE(dereference) diff --git a/test/test_Callback.cpp b/test/test_Callback.cpp index 45bdc0dd..1a64b886 100644 --- a/test/test_Callback.cpp +++ b/test/test_Callback.cpp @@ -287,5 +287,5 @@ TESTCASE(VoidReturn) m.module_eval(code); Object result = m.call("trigger_callback4"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } diff --git a/test/test_Class.cpp b/test/test_Class.cpp index b5c5fe00..1787bfd0 100644 --- a/test/test_Class.cpp +++ b/test/test_Class.cpp @@ -31,7 +31,7 @@ TESTCASE(undef_creation_funcs) Exception, c.call("new"), ASSERT_EQUAL(rb_eTypeError, ex.class_of()) - ); + ); } TESTCASE(include_module) @@ -130,6 +130,29 @@ TESTCASE(module_function) ); } +TESTCASE(methods_no_class) +{ + Class c; + + ASSERT_EXCEPTION_CHECK( + std::runtime_error, + c.define_function("some_function", &some_function), + ASSERT_EQUAL("Rice Object does not wrap a Ruby object", ex.what()) + ); + + ASSERT_EXCEPTION_CHECK( + std::runtime_error, + c.define_method("some_method", &some_method), + ASSERT_EQUAL("Rice Object does not wrap a Ruby object", ex.what()) + ); + + ASSERT_EXCEPTION_CHECK( + std::runtime_error, + Object o = c.call("new"), + ASSERT_EQUAL("Rice Object does not wrap a Ruby object", ex.what()) + ); +} + TESTCASE(define_class) { Class object(rb_cObject); diff --git a/test/test_Data_Type.cpp b/test/test_Data_Type.cpp index 77644440..ea682e9c 100644 --- a/test/test_Data_Type.cpp +++ b/test/test_Data_Type.cpp @@ -105,7 +105,7 @@ TESTCASE(methods_with_member_pointers) Object result = o.call("no_return_no_arg"); ASSERT(MyClass::no_return_no_arg_called); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = o.call("no_arg"); ASSERT(MyClass::no_arg_called); @@ -235,7 +235,7 @@ TESTCASE(methods_with_lambdas) Object result = o.call("no_return_no_arg"); ASSERT(MyClass::no_return_no_arg_called); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = o.call("no_arg"); ASSERT(MyClass::no_arg_called); @@ -311,32 +311,6 @@ namespace }; } -TESTCASE(not_bound) -{ - Module m = define_module("Testing"); - - Data_Type dataType; - dataType. - define_method("something", [](MyClass3&) -> std::string - { - return "Should raise error"; - }); - - std::string code = R"(Object.new.something)"; - -#ifdef _MSC_VER - std::string message = "Type is not defined with Rice: class `anonymous namespace'::MyClass3"; -#else - std::string message = "Type is not defined with Rice: (anonymous namespace)::MyClass3"; -#endif - - ASSERT_EXCEPTION_CHECK( - Exception, - m.module_eval(code), - ASSERT_EQUAL(message, ex.what()) - ); -} - namespace { class BaseClass @@ -542,10 +516,10 @@ TESTCASE(null_ptrs) Object o = c.call("new"); Object result = o.call("get"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = o.call("set", nullptr); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } namespace @@ -621,7 +595,7 @@ TESTCASE(pointers) Object object = myClass.call("new"); Object result = object.call("pass_through", nullptr); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = object.call("pass_through", helper); Object value = result.call("value"); @@ -632,7 +606,7 @@ TESTCASE(pointers) ASSERT_EQUAL(5, detail::From_Ruby().convert(value)); result = object.call("pass_through_void", nullptr); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = object.call("pass_through_void", helper); value = result.call("value"); diff --git a/test/test_Exception.cpp b/test/test_Exception.cpp index 04c79e2b..33efe931 100644 --- a/test/test_Exception.cpp +++ b/test/test_Exception.cpp @@ -281,7 +281,7 @@ TESTCASE(subclasses) ); } -TESTCASE(memory) +/*TESTCASE(memory) { Module m = define_module("Testing"); @@ -296,4 +296,4 @@ TESTCASE(memory) m.instance_eval(code); ASSERT(true); -} +}*/ diff --git a/test/test_Object.cpp b/test/test_Object.cpp index a636ba4f..3911c5fa 100644 --- a/test/test_Object.cpp +++ b/test/test_Object.cpp @@ -19,7 +19,7 @@ TEARDOWN(Object) TESTCASE(default_construct) { Object o; - ASSERT_EQUAL(Qnil, o.value()); + ASSERT(o.is_nil()); } TESTCASE(construct_with_value) @@ -48,7 +48,7 @@ TESTCASE(move_construct) Object o1(INT2NUM(42)); Object o2(std::move(o1)); ASSERT_EQUAL(o2.value(), INT2NUM(42)); - ASSERT_EQUAL(o1.value(), Qnil); + ASSERT(o1.is_nil()); } TESTCASE(move_assign) @@ -57,16 +57,16 @@ TESTCASE(move_assign) Object o2(INT2NUM(43)); o2 = std::move(o1); ASSERT_EQUAL(o2.value(), INT2NUM(42)); - ASSERT_EQUAL(o1.value(), Qnil); + ASSERT(o1.is_nil()); } TESTCASE(test) { - ASSERT_EQUAL(true, Object(Qtrue).test()); - ASSERT_EQUAL(true, Object(INT2NUM(42)).test()); - ASSERT_EQUAL(false, Object(Qfalse).test()); - ASSERT_EQUAL(false, Object(Qnil).test()); - ASSERT_EQUAL(true, Object(Qundef).test()); + ASSERT_EQUAL(true, (bool)Object(Qtrue)); + ASSERT_EQUAL(true, (bool)Object(INT2NUM(42))); + ASSERT_EQUAL(false, (bool)Object(Qfalse)); + ASSERT_EQUAL(false, (bool)Object(Qnil)); + ASSERT_EQUAL(true, (bool)Object(Qundef)); } TESTCASE(explicit_conversion_to_bool) @@ -96,8 +96,13 @@ TESTCASE(implicit_conversion_to_value) ASSERT_EQUAL(Qtrue, (VALUE)Object(Qtrue)); ASSERT_EQUAL(INT2NUM(42), (VALUE)Object(INT2NUM(42))); ASSERT_EQUAL(Qfalse, (VALUE)Object(Qfalse)); - ASSERT_EQUAL(Qnil, (VALUE)Object(Qnil)); 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()) + ); } TESTCASE(explicit_conversion_to_value) @@ -105,8 +110,13 @@ TESTCASE(explicit_conversion_to_value) ASSERT_EQUAL(Qtrue, Object(Qtrue).value()); ASSERT_EQUAL(INT2NUM(42), Object(INT2NUM(42)).value()); ASSERT_EQUAL(Qfalse, Object(Qfalse).value()); - ASSERT_EQUAL(Qnil, Object(Qnil).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()) + ); } TESTCASE(class_of) @@ -122,6 +132,11 @@ TESTCASE(compare) ASSERT_EQUAL(1, Object(INT2NUM(42)).compare(Object(INT2NUM(41)))); } +TESTCASE(equality_nil_objects) +{ + ASSERT_EQUAL(true, Object(Qnil) == Object(Qnil)); +} + TESTCASE(to_s) { ASSERT_EQUAL(String("42"), Object(INT2NUM(42)).to_s()); @@ -161,8 +176,13 @@ TESTCASE(rb_type) ASSERT_EQUAL(T_TRUE, Object(Qtrue).rb_type()); ASSERT_EQUAL(T_FIXNUM, Object(INT2NUM(42)).rb_type()); ASSERT_EQUAL(T_FALSE, Object(Qfalse).rb_type()); - ASSERT_EQUAL(T_NIL, Object(Qnil).rb_type()); ASSERT_EQUAL(T_UNDEF, Object(Qundef).rb_type()); + + ASSERT_EXCEPTION_CHECK( + std::runtime_error, + Object(Qnil).rb_type(), + ASSERT_EQUAL("Rice Object does not wrap a Ruby object", ex.what()) + ); } TESTCASE(call_no_arguments) diff --git a/test/test_Stl_Function.cpp b/test/test_Stl_Function.cpp index 4e95ab39..57076c5a 100644 --- a/test/test_Stl_Function.cpp +++ b/test/test_Stl_Function.cpp @@ -201,7 +201,7 @@ TESTCASE(ReturnVoid) function.call(4.3) )"; Object result = m.module_eval(code); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } namespace diff --git a/test/test_Stl_Map.cpp b/test/test_Stl_Map.cpp index 3c17f611..352935ae 100644 --- a/test/test_Stl_Map.cpp +++ b/test/test_Stl_Map.cpp @@ -113,7 +113,7 @@ TESTCASE(Include) ASSERT_EQUAL(Qfalse, result.value()); result = map.call("[]", "three"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(Value) diff --git a/test/test_Stl_OStream.cpp b/test/test_Stl_OStream.cpp index d71f2815..f9db0069 100644 --- a/test/test_Stl_OStream.cpp +++ b/test/test_Stl_OStream.cpp @@ -28,9 +28,7 @@ TESTCASE(CreateOStringStream) Module m = define_module("TestOStream"); - std::string code = R"( - Std::OStringStream.new - )"; + std::string code = R"(Std::OStringStream.new)"; Object result = m.module_eval(code); ASSERT_EQUAL("Std::OStringStream", result.class_name().c_str()); } diff --git a/test/test_Stl_Optional.cpp b/test/test_Stl_Optional.cpp index 29f90e1e..9c3888a6 100644 --- a/test/test_Stl_Optional.cpp +++ b/test/test_Stl_Optional.cpp @@ -64,7 +64,7 @@ TESTCASE(OptionalReturn) ASSERT_EQUAL("Here is a value", detail::From_Ruby().convert(result)); result = myClass.call("optional_return", false); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(OptionalArgument) @@ -85,11 +85,11 @@ TESTCASE(OptionalAttribute) Object myClass = m.module_eval("MyClass.new"); Object result = myClass.call("optional_attr"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = myClass.call("optional_attr=", 77.7); ASSERT_EQUAL(77.7, detail::From_Ruby().convert(result)); result = myClass.call("optional_attr=", std::nullopt); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } diff --git a/test/test_Stl_Pair.cpp b/test/test_Stl_Pair.cpp index c87c2cc0..8d854001 100644 --- a/test/test_Stl_Pair.cpp +++ b/test/test_Stl_Pair.cpp @@ -31,7 +31,7 @@ TESTCASE(CreatePair) ASSERT_EQUAL(0, detail::From_Ruby().convert(result)); result = pair.call("second"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = pair.call("first=", 77); ASSERT_EQUAL(77, detail::From_Ruby().convert(result)); diff --git a/test/test_Stl_Unordered_Map.cpp b/test/test_Stl_Unordered_Map.cpp index 0fc33a68..b1168e3e 100644 --- a/test/test_Stl_Unordered_Map.cpp +++ b/test/test_Stl_Unordered_Map.cpp @@ -110,7 +110,7 @@ TESTCASE(Include) ASSERT_EQUAL(Qfalse, result.value()); result = unordered_map.call("[]", "three"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(Value) diff --git a/test/test_Stl_Variant.cpp b/test/test_Stl_Variant.cpp index 25861007..79220739 100644 --- a/test/test_Stl_Variant.cpp +++ b/test/test_Stl_Variant.cpp @@ -320,7 +320,7 @@ TESTCASE(Roundtrip) ASSERT_EQUAL("Hi from MyClass2", detail::From_Ruby().convert(hello)); instance2 = m.call("roundtrip_variant", nullptr); - ASSERT_EQUAL(Qnil, instance2.value()); + ASSERT(instance2.is_nil()); } /* This test case runs successfully on MSVC but not g++. Having stepped through the code with @@ -350,7 +350,7 @@ TESTCASE(RoundtripRef) ASSERT_EQUAL("Hi from MyClass2", detail::From_Ruby().convert(hello)); instance2 = m.call("roundtrip_variant_ref", nullptr); - ASSERT_EQUAL(Qnil, instance2.value()); + ASSERT(instance2.is_nil()); } #endif @@ -369,7 +369,7 @@ TESTCASE(RoundtripConstRef) ASSERT_EQUAL("Hi from MyClass2", detail::From_Ruby().convert(hello)); instance2 = m.call("roundtrip_const_variant_ref", nullptr); - ASSERT_EQUAL(Qnil, instance2.value()); + ASSERT(instance2.is_nil()); } TESTCASE(Klass) diff --git a/test/test_Stl_Vector.cpp b/test/test_Stl_Vector.cpp index 1ca77bb7..1f4d6084 100644 --- a/test/test_Stl_Vector.cpp +++ b/test/test_Stl_Vector.cpp @@ -161,10 +161,10 @@ TESTCASE(Empty) ASSERT_EQUAL(Qtrue, result.value()); result = vec.call("first"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("last"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(BoolVector) @@ -230,7 +230,7 @@ TESTCASE(Indexing) ASSERT_EQUAL(2, detail::From_Ruby().convert(result)); result = vec.call("[]", 3); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("[]", -1); ASSERT_EQUAL(2, detail::From_Ruby().convert(result)); @@ -242,10 +242,10 @@ TESTCASE(Indexing) ASSERT_EQUAL(0, detail::From_Ruby().convert(result)); result = vec.call("[]", -4); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("[]", -7); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(IndexingEmptyVector) @@ -259,13 +259,13 @@ TESTCASE(IndexingEmptyVector) ASSERT_EQUAL(0, detail::From_Ruby().convert(result)); result = vec.call("[]", 0); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("[]", 1); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("[]", -1); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(Slice) @@ -416,7 +416,7 @@ TESTCASE(Modify) ASSERT_EQUAL(0, detail::From_Ruby().convert(result)); result = vec.call("pop"); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(Clone) @@ -505,7 +505,7 @@ TESTCASE(NotComparable) vec.call("push", NotComparable(3)); Object result = vec.call("delete", NotComparable(1)); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("length"); ASSERT_EQUAL(3u, detail::From_Ruby().convert(result)); @@ -514,7 +514,7 @@ TESTCASE(NotComparable) ASSERT_EQUAL(Qfalse, result.value()); result = vec.call("index", NotComparable(3)); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(NotDefaultConstructable) @@ -526,7 +526,7 @@ TESTCASE(NotDefaultConstructable) Object vec = c.call("new"); Object result = vec.call("resize", 10); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("length"); ASSERT_EQUAL(0u, detail::From_Ruby().convert(result)); @@ -666,7 +666,7 @@ TESTCASE(ComparableButNotBool) vec.call("push", ComparableButNotBool(3)); Object result = vec.call("delete", ComparableButNotBool(1)); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("length"); ASSERT_EQUAL(3u, detail::From_Ruby().convert(result)); @@ -675,7 +675,7 @@ TESTCASE(ComparableButNotBool) ASSERT_EQUAL(Qfalse, result.value()); result = vec.call("index", ComparableButNotBool(3)); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); } TESTCASE(DefaultConstructable) @@ -687,7 +687,7 @@ TESTCASE(DefaultConstructable) Object vec = c.call("new"); Object result = vec.call("resize", 10); - ASSERT_EQUAL(Qnil, result.value()); + ASSERT(result.is_nil()); result = vec.call("length"); ASSERT_EQUAL(0u, detail::From_Ruby().convert(result)); diff --git a/test/test_Struct.cpp b/test/test_Struct.cpp index 185adc0f..41ee5454 100644 --- a/test/test_Struct.cpp +++ b/test/test_Struct.cpp @@ -61,9 +61,9 @@ TESTCASE(new_instance_no_args) { Struct s(define_3d_point()); Struct::Instance p(s.new_instance()); - ASSERT_EQUAL(Rice::Nil, Object(rb_struct_getmember(p, rb_intern("x")))); - ASSERT_EQUAL(Rice::Nil, Object(rb_struct_getmember(p, rb_intern("y")))); - ASSERT_EQUAL(Rice::Nil, Object(rb_struct_getmember(p, rb_intern("z")))); + ASSERT_EQUAL(Qnil, rb_struct_getmember(p, rb_intern("x"))); + ASSERT_EQUAL(Qnil, rb_struct_getmember(p, rb_intern("y"))); + ASSERT_EQUAL(Qnil, rb_struct_getmember(p, rb_intern("z"))); } TESTCASE(new_instance_with_args) From 86706eae90ad274629498e230c2fc79a274a26c4 Mon Sep 17 00:00:00 2001 From: cfis Date: Mon, 16 Feb 2026 23:51:26 -0800 Subject: [PATCH 2/8] =?UTF-8?q?Make=20Array=20and=20Hash=20a=20bit=20less?= =?UTF-8?q?=20annoying=20to=20use=20by=20changing=20the=20converstion=20op?= =?UTF-8?q?erator=20to=20VALUE=20from=20Object.=20C++=20only=20allows=20on?= =?UTF-8?q?e=20implicit=20user-defined=20conversion=20in=20a=20chain,=20so?= =?UTF-8?q?=20Proxy=20=E2=86=92=20Object=20=E2=86=92=20Array=20requires=20?= =?UTF-8?q?two=20and=20fails.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- rice/cpp_api/Array.hpp | 4 ++-- rice/cpp_api/Array.ipp | 6 +++--- rice/cpp_api/Hash.hpp | 4 ++-- rice/cpp_api/Hash.ipp | 6 +++--- test/test_Array.cpp | 33 +++++++++++++++++++++++++++++---- test/test_Hash.cpp | 10 ++++++++++ 6 files changed, 49 insertions(+), 14 deletions(-) diff --git a/rice/cpp_api/Array.hpp b/rice/cpp_api/Array.hpp index 98dac507..ff749a7c 100644 --- a/rice/cpp_api/Array.hpp +++ b/rice/cpp_api/Array.hpp @@ -155,8 +155,8 @@ namespace Rice //! Construct a new Proxy Proxy(Array array, long index); - //! Implicit conversions - operator Object() const; + //! Implicit conversion to VALUE. + operator VALUE() const; //! Explicit conversion to VALUE. VALUE value() const; diff --git a/rice/cpp_api/Array.ipp b/rice/cpp_api/Array.ipp index 6b300972..2f678104 100644 --- a/rice/cpp_api/Array.ipp +++ b/rice/cpp_api/Array.ipp @@ -134,9 +134,9 @@ namespace Rice return detail::protect(rb_ary_entry, array_.value(), index_); } - inline Array::Proxy::operator Object() const + inline Array::Proxy::operator VALUE() const { - return Object(this->value()); + return this->value(); } template @@ -194,7 +194,7 @@ namespace Rice template inline Object* Array::Iterator::operator->() { - tmp_ = (*array_)[index_]; + tmp_ = Object((*array_)[index_]); return &tmp_; } diff --git a/rice/cpp_api/Hash.hpp b/rice/cpp_api/Hash.hpp index 0c5baa71..a9d25a96 100644 --- a/rice/cpp_api/Hash.hpp +++ b/rice/cpp_api/Hash.hpp @@ -93,8 +93,8 @@ namespace Rice //! Construct a new Proxy. Proxy(Hash* hash, Object key); - //! Implicit conversion to Object. - operator Object() const; + //! Implicit conversion to VALUE. + operator VALUE() const; //! Explicit conversion to VALUE. VALUE value() const; diff --git a/rice/cpp_api/Hash.ipp b/rice/cpp_api/Hash.ipp index 26507bea..062245c8 100644 --- a/rice/cpp_api/Hash.ipp +++ b/rice/cpp_api/Hash.ipp @@ -19,7 +19,7 @@ namespace Rice { } - inline Hash::Proxy::operator Object() const + inline Hash::Proxy::operator VALUE() const { return value(); } @@ -51,7 +51,7 @@ namespace Rice inline Value_T Hash::get(Key_T const& key) { Object ruby_key(detail::To_Ruby().convert(key)); - Object value = operator[](ruby_key); + Object value(operator[](ruby_key)); try { return detail::From_Ruby().convert(value); @@ -155,7 +155,7 @@ namespace Rice template inline Object Hash::Iterator::current_key() { - return hash_keys()[current_index_]; + return Object(hash_keys()[current_index_]); } template diff --git a/test/test_Array.cpp b/test/test_Array.cpp index fc695e94..7d9fd414 100644 --- a/test/test_Array.cpp +++ b/test/test_Array.cpp @@ -258,9 +258,9 @@ TESTCASE(find_if) rubyValues.push(44, false); auto iter = std::find_if(rubyValues.begin(), rubyValues.end(), - [&rubyValues](const Object& object) + [&rubyValues](VALUE object) { - return object == rubyValues[1]; + return object == (VALUE)rubyValues[1]; }); ASSERT_EQUAL(43, detail::From_Ruby().convert(iter->value())); @@ -274,6 +274,31 @@ TESTCASE(assign_int) ASSERT_EQUAL(10, detail::From_Ruby().convert(a[0].value())); } +TESTCASE(proxy_to_value) +{ + Array a; + a.push(42, false); + + // Proxy should implicitly convert to VALUE + VALUE v = a[0]; + ASSERT_EQUAL(detail::to_ruby(42), v); +} + +TESTCASE(proxy_to_wrapper_type) +{ + // Create an inner array and push it into an outer array + Array inner; + inner.push(42, false); + + Array outer; + outer.push(inner, false); + + // Proxy -> VALUE -> Array should compile and work + Array retrieved(outer[0]); + ASSERT_EQUAL(1, retrieved.size()); + ASSERT_EQUAL(42, detail::From_Ruby().convert(retrieved[0].value())); +} + /** * Issue 59 - Copy constructor compilation problem. */ @@ -496,7 +521,7 @@ TESTCASE(iterator_std_sort) if (val1 > val2) { // Swap using array subscript operator - Object temp = a[j]; + Object temp(a[j]); a[j] = detail::From_Ruby().convert(a[j + 1].value()); a[j + 1] = detail::From_Ruby().convert(temp.value()); } @@ -528,7 +553,7 @@ TESTCASE(iterator_std_reverse) { long i = begin.index(); long j = end.index(); - Object temp = a[i]; + Object temp(a[i]); a[i] = detail::From_Ruby().convert(a[j].value()); a[j] = detail::From_Ruby().convert(temp.value()); ++begin; diff --git a/test/test_Hash.cpp b/test/test_Hash.cpp index 6a085ec1..76f9e8f9 100644 --- a/test/test_Hash.cpp +++ b/test/test_Hash.cpp @@ -96,6 +96,16 @@ TESTCASE(get) ASSERT_EQUAL(1, h.get(6)); } +TESTCASE(proxy_to_value) +{ + Hash h; + h[1] = 5; + + // Proxy should implicitly convert to VALUE + VALUE v = h[1]; + ASSERT_EQUAL(detail::to_ruby(5), v); +} + TESTCASE(construct_vector_from_hash_iterators) { Hash h; From d3d13cebd5926e78db1fc8549a697cc08495d8ee Mon Sep 17 00:00:00 2001 From: cfis Date: Mon, 16 Feb 2026 23:57:28 -0800 Subject: [PATCH 3/8] Fix GC bugs --- test/test_Iterator.cpp | 217 ++++++++++++++++++++++------------------- 1 file changed, 119 insertions(+), 98 deletions(-) diff --git a/test/test_Iterator.cpp b/test/test_Iterator.cpp index 89bc1333..4408b31c 100644 --- a/test/test_Iterator.cpp +++ b/test/test_Iterator.cpp @@ -30,9 +30,9 @@ namespace namespace { - struct Data + struct Element { - Data(uint32_t value) + Element(uint32_t value) { this->index = value; } @@ -46,15 +46,15 @@ namespace { public: using iterator_category = std::input_iterator_tag; - using value_type = Data; + using value_type = Element; using difference_type = std::ptrdiff_t; - using pointer = Data*; - using reference = Data&; // Note: standard expects reference, but we return by value + using pointer = Element*; + using reference = Element&; // Note: standard expects reference, but we return by value - ValueReturningIterator(std::vector* data, size_t index) : data_(data), index_(index) {} + ValueReturningIterator(std::vector* data, size_t index) : data_(data), index_(index) {} // Returns by VALUE, not by reference - this is the key characteristic being tested - Data operator*() const { return (*data_)[index_]; } + Element operator*() const { return (*data_)[index_]; } ValueReturningIterator& operator++() { ++index_; return *this; } ValueReturningIterator operator++(int) { auto tmp = *this; ++index_; return tmp; } @@ -62,7 +62,7 @@ namespace bool operator!=(const ValueReturningIterator& other) const { return index_ != other.index_; } private: - std::vector* data_; + std::vector* data_; size_t index_; }; @@ -78,7 +78,7 @@ namespace ValueReturningIterator end() { return ValueReturningIterator(&data_, data_.size()); } private: - std::vector data_; + std::vector data_; }; class ContainerValues @@ -89,27 +89,27 @@ namespace this->data_ = { {1}, {2}, {3} }; } - std::vector::iterator begin() + std::vector::iterator begin() { return this->data_.begin(); } - std::vector::iterator end() + std::vector::iterator end() { return this->data_.end(); } - std::vector::const_iterator cbegin() const + std::vector::const_iterator cbegin() const { return this->data_.cbegin(); } - std::vector::const_iterator cend() const + std::vector::const_iterator cend() const { return this->data_.cend(); } - std::vector data_; + std::vector data_; }; class ContainerPointers @@ -117,38 +117,38 @@ namespace public: ContainerPointers() { - this->data_ = { new Data{1}, new Data{2}, new Data{3} }; + this->data_ = { new Element{1}, new Element{2}, new Element{3} }; } ~ContainerPointers() { - for (Data* data : this->data_) + for (Element* data : this->data_) { delete data; } } - std::vector::iterator begin() + std::vector::iterator begin() { return this->data_.begin(); } - std::vector::iterator end() + std::vector::iterator end() { return this->data_.end(); } - std::vector::reverse_iterator rbegin() + std::vector::reverse_iterator rbegin() { return this->data_.rbegin(); } - std::vector::reverse_iterator rend() + std::vector::reverse_iterator rend() { return this->data_.rend(); } - std::vector data_; + std::vector data_; }; } @@ -156,14 +156,6 @@ namespace SETUP(Iterator) { embed_ruby(); - - Data_Type::unbind(); - Data_Type::unbind(); - Data_Type::unbind(); - - Rice::detail::Registries::instance.types.remove(); - Rice::detail::Registries::instance.types.remove(); - Rice::detail::Registries::instance.types.remove(); } TEARDOWN(Iterator) @@ -191,8 +183,8 @@ TESTCASE(ArrayIterator) TESTCASE(Standard) { - define_class("Data") - .define_constructor(Constructor()); + define_class("Element") + .define_constructor(Constructor()); define_class("ContainerValues") .define_constructor(Constructor()) @@ -204,20 +196,20 @@ TESTCASE(Standard) Array a = wrapper.instance_eval("each.to_a"); ASSERT_EQUAL(3, a.size()); - Data_Object wrappedData(a[0]); + Data_Object wrappedData(a[0]); ASSERT_EQUAL(1u, wrappedData->index); - wrappedData = (Data_Object)a[1]; + wrappedData = (Data_Object)a[1]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)a[2]; + wrappedData = (Data_Object)a[2]; ASSERT_EQUAL(3u, wrappedData->index); } TESTCASE(Lambda) { - define_class("Data") - .define_constructor(Constructor()); + define_class("Element") + .define_constructor(Constructor()); define_class("ContainerValues") .define_constructor(Constructor()) @@ -235,7 +227,7 @@ TESTCASE(Lambda) ContainerValues* container = detail::From_Ruby().convert(self); // The iterator returns references - we do NOT want to create a copy - detail::To_Ruby toRuby; + detail::To_Ruby toRuby; auto it = container->begin(); auto end = container->end(); @@ -258,41 +250,49 @@ TESTCASE(Lambda) container.each do |data| result << data end - result)"; + # Return container to keep it alive + [container, result])"; Array result = m.module_eval(code); - ASSERT_EQUAL(3, result.size()); + ASSERT_EQUAL(2, result.size()); - Data_Object wrappedData(result[0]); + Array elements(result[1]); + ASSERT_EQUAL(3, elements.size()); + + Data_Object wrappedData(elements[0]); ASSERT_EQUAL(1u, wrappedData->index); - wrappedData = (Data_Object)result[1]; + wrappedData = (Data_Object)elements[1]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)result[2]; + wrappedData = (Data_Object)elements[2]; ASSERT_EQUAL(3u, wrappedData->index); code = R"(container = ContainerValues.new enumerator = container.each - enumerator.to_a)"; + # Return container to keep it alive + [container, enumerator.to_a])"; result = m.module_eval(code); - ASSERT_EQUAL(3, result.size()); + ASSERT_EQUAL(2, result.size()); + + elements = Array(result[1]); + ASSERT_EQUAL(3, elements.size()); - wrappedData = (Data_Object)result[0]; + wrappedData = (Data_Object)elements[0]; ASSERT_EQUAL(1u, wrappedData->index); - wrappedData = (Data_Object)result[1]; + wrappedData = (Data_Object)elements[1]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)result[2]; + wrappedData = (Data_Object)elements[2]; ASSERT_EQUAL(3u, wrappedData->index); } TESTCASE(ConstValue) { - define_class("Data") - .define_constructor(Constructor()); + define_class("Element") + .define_constructor(Constructor()); define_class("ContainerValues") .define_constructor(Constructor()) @@ -305,24 +305,29 @@ TESTCASE(ConstValue) container.each do |x| result << x end - result)"; + # Return container to keep it alive + [container, result])"; - Array a = m.module_eval(code); + Array result = m.module_eval(code); + ASSERT_EQUAL(2, result.size()); - Data_Object wrappedData(a[0]); + Array elements = Array(result[1]); + ASSERT_EQUAL(3, elements.size()); + + Data_Object wrappedData(elements[0]); ASSERT_EQUAL(1u, wrappedData->index); - wrappedData = (Data_Object)a[1]; + wrappedData = (Data_Object)elements[1]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)a[2]; + wrappedData = (Data_Object)elements[2]; ASSERT_EQUAL(3u, wrappedData->index); } TESTCASE(Pointer) { - define_class("Data") - .define_constructor(Constructor()); + define_class("Element") + .define_constructor(Constructor()); define_class("ContainerPointers") .define_constructor(Constructor()) @@ -338,24 +343,31 @@ TESTCASE(Pointer) container.each do |x| result << x end - result)"; + # Return container to keep it alive + [container, result])"; Array a = m.module_eval(code); - Data_Object wrappedData(a[0]); + Array result = m.module_eval(code); + ASSERT_EQUAL(2, result.size()); + + Array elements = Array(result[1]); + ASSERT_EQUAL(3, elements.size()); + + Data_Object wrappedData(elements[0]); ASSERT_EQUAL(1u, wrappedData->index); - wrappedData = (Data_Object)a[1]; + wrappedData = (Data_Object)elements[1]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)a[2]; + wrappedData = (Data_Object)elements[2]; ASSERT_EQUAL(3u, wrappedData->index); } TESTCASE(TwoIteratorPointers) { - define_class("Data") - .define_constructor(Constructor()); + define_class("Element") + .define_constructor(Constructor()); define_class("ContainerPointers") .define_constructor(Constructor()) @@ -375,36 +387,41 @@ TESTCASE(TwoIteratorPointers) container.reach do |x| result << x end - result)"; + # Return container to keep it alive + [container, result])"; Array a = m.module_eval(code); - ASSERT_EQUAL(6, a.size()); + Array result = m.module_eval(code); + ASSERT_EQUAL(2, result.size()); + + Array elements = Array(result[1]); + ASSERT_EQUAL(6, elements.size()); - Data_Object wrappedData(a[0]); + Data_Object wrappedData(elements[0]); ASSERT_EQUAL(1u, wrappedData->index); - wrappedData = (Data_Object)a[1]; + wrappedData = (Data_Object)elements[1]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)a[2]; + wrappedData = (Data_Object)elements[2]; ASSERT_EQUAL(3u, wrappedData->index); - wrappedData = (Data_Object)a[3]; + wrappedData = (Data_Object)elements[3]; ASSERT_EQUAL(3u, wrappedData->index); - wrappedData = (Data_Object)a[4]; + wrappedData = (Data_Object)elements[4]; ASSERT_EQUAL(2u, wrappedData->index); - wrappedData = (Data_Object)a[5]; + wrappedData = (Data_Object)elements[5]; ASSERT_EQUAL(1u, wrappedData->index); } TESTCASE(Map) { - define_class("Data") - .define_constructor(Constructor()) - .define_attr("index", &Data::index, Rice::AttrAccess::Read); + define_class("Element") + .define_constructor(Constructor()) + .define_attr("index", &Element::index, Rice::AttrAccess::Read); define_class("ContainerPointers") .define_constructor(Constructor()) @@ -420,21 +437,21 @@ TESTCASE(Map) Array a = m.module_eval(code); ASSERT_EQUAL(3, a.size()); - Object element = a[0]; + Object element(a[0]); ASSERT_EQUAL(2, detail::From_Ruby().convert(element)); - element = a[1]; + element = Object(a[1]); ASSERT_EQUAL(4, detail::From_Ruby().convert(element)); - element = a[2]; + element = Object(a[2]); ASSERT_EQUAL(6, detail::From_Ruby().convert(element)); } TESTCASE(Enum) { - define_class("Data") - .define_constructor(Constructor()) - .define_attr("index", &Data::index, Rice::AttrAccess::Read); + define_class("Element") + .define_constructor(Constructor()) + .define_attr("index", &Element::index, Rice::AttrAccess::Read); define_class("ContainerPointers") .define_constructor(Constructor()) @@ -451,21 +468,21 @@ TESTCASE(Enum) ASSERT_EQUAL(3, a.size()); - Object element = a[0]; + Object element(a[0]); ASSERT_EQUAL(2, detail::From_Ruby().convert(element)); - element = a[1]; + element = Object(a[1]); ASSERT_EQUAL(4, detail::From_Ruby().convert(element)); - element = a[2]; + element = Object(a[2]); ASSERT_EQUAL(6, detail::From_Ruby().convert(element)); } TESTCASE(ToArray) { - define_class("Data") - .define_constructor(Constructor()) - .define_attr("index", &Data::index, Rice::AttrAccess::Read); + define_class("Element") + .define_constructor(Constructor()) + .define_attr("index", &Element::index, Rice::AttrAccess::Read); define_class("ContainerPointers") .define_constructor(Constructor()) @@ -474,29 +491,33 @@ TESTCASE(ToArray) Module m = define_module("TestingModule"); std::string code = R"(container = ContainerPointers.new - container.to_a)"; + # We have to return the container so it is not GCed + [container, container.to_a])"; - Array a = m.module_eval(code); + Array result = m.module_eval(code); + ASSERT_EQUAL(2, result.size()); - ASSERT_EQUAL(3, a.size()); + Array elements(result[1]); + + ASSERT_EQUAL(3, elements.size()); - Object element = a[0]; + Object element(elements[0]); Object index = element.call("index"); ASSERT_EQUAL(1, detail::From_Ruby().convert(index)); - element = a[1]; + element = Object(elements[1]); index = element.call("index"); ASSERT_EQUAL(2, detail::From_Ruby().convert(index)); - element = a[2]; + element = Object(elements[2]); index = element.call("index"); ASSERT_EQUAL(3, detail::From_Ruby().convert(index)); } TESTCASE(IterateNoCopy) { - define_class("Data") - .define_constructor(Constructor()); + define_class("Element") + .define_constructor(Constructor()); define_class("ContainerValues") .define_constructor(Constructor()) @@ -512,8 +533,8 @@ TESTCASE(IterateNoCopy) for (size_t i = 0; i < container.data_.size(); i++) { - Data& expected = container.data_[i]; - Data_Object actual(a[(long)i]); + Element& expected = container.data_[i]; + Data_Object actual(a[(long)i]); ASSERT_EQUAL(&expected, &(*actual)); } } @@ -522,9 +543,9 @@ TESTCASE(IterateNoCopy) // This previously caused MSVC warning C4239 about binding rvalue to non-const reference. TESTCASE(ValueReturningIterator) { - define_class("Data") - .define_constructor(Constructor()) - .define_attr("index", &Data::index, Rice::AttrAccess::Read); + define_class("Element") + .define_constructor(Constructor()) + .define_attr("index", &Element::index, Rice::AttrAccess::Read); define_class("ContainerWithValueIterator") .define_constructor(Constructor()) From 2cbd5bd0ca6c239730bd92f717c02abb232b2c64 Mon Sep 17 00:00:00 2001 From: cfis Date: Tue, 17 Feb 2026 00:04:23 -0800 Subject: [PATCH 4/8] Try again with GC stress enabled after recent GC fixes. --- test/embed_ruby.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/embed_ruby.cpp b/test/embed_ruby.cpp index d9f23526..ba8debb5 100644 --- a/test/embed_ruby.cpp +++ b/test/embed_ruby.cpp @@ -25,9 +25,8 @@ void embed_ruby() // Enable GC stress to help catch GC-related bugs #if RICE_RELEASE - // rb_funcall(rb_mGC, rb_intern("stress="), 1, Qtrue); + rb_funcall(rb_mGC, rb_intern("stress="), 1, Qtrue); #endif - initialized__ = true; } } From c8c3716ea328af44b262021016332a9065b05b2f Mon Sep 17 00:00:00 2001 From: cfis Date: Tue, 17 Feb 2026 00:32:34 -0800 Subject: [PATCH 5/8] Fix GC bug in Exception class. --- rice/Exception.hpp | 3 +-- rice/Exception.ipp | 9 +++++---- rice/rice.hpp | 8 +++++--- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/rice/Exception.hpp b/rice/Exception.hpp index 47e95b90..5544d35a 100644 --- a/rice/Exception.hpp +++ b/rice/Exception.hpp @@ -58,8 +58,7 @@ namespace Rice VALUE value() const; private: - // TODO: Do we need to tell the Ruby gc about an exception instance? - mutable VALUE exception_ = Qnil; + Pin exception_ = Qnil; mutable std::string message_; }; } // namespace Rice diff --git a/rice/Exception.ipp b/rice/Exception.ipp index 169c5aa4..c7f81e4a 100644 --- a/rice/Exception.ipp +++ b/rice/Exception.ipp @@ -32,7 +32,8 @@ namespace Rice #endif // Now create the Ruby exception - this->exception_ = detail::protect(rb_exc_new2, exceptionClass, this->message_.c_str()); + VALUE exception = detail::protect(rb_exc_new2, exceptionClass, this->message_.c_str()); + this->exception_ = Pin(exception); } inline char const* Exception::what() const noexcept @@ -41,7 +42,7 @@ namespace Rice { // This isn't protected because if it fails then either we could eat the exception // (not good) or crash the program (better) - VALUE rubyMessage = rb_funcall(this->exception_, rb_intern("message"), 0); + VALUE rubyMessage = rb_funcall(this->exception_.value(), rb_intern("message"), 0); this->message_ = std::string(RSTRING_PTR(rubyMessage), RSTRING_LEN(rubyMessage)); } return this->message_.c_str(); @@ -49,11 +50,11 @@ namespace Rice inline VALUE Exception::class_of() const { - return detail::protect(rb_class_of, this->exception_); + return detail::protect(rb_class_of, this->exception_.value()); } inline VALUE Exception::value() const { - return this->exception_; + return this->exception_.value(); } } diff --git a/rice/rice.hpp b/rice/rice.hpp index c24797bc..9c59773a 100644 --- a/rice/rice.hpp +++ b/rice/rice.hpp @@ -28,6 +28,10 @@ #include "detail/Type.hpp" #include "detail/TypeIndexParser.hpp" +// Code to register Ruby objects with GC (declarations) +#include "detail/Anchor.hpp" +#include "Pin.hpp" + // Code for C++ to call Ruby #include "Exception.hpp" #include "JumpException.hpp" @@ -43,10 +47,8 @@ #include "detail/RubyType.hpp" #include "detail/Parameter.hpp" -// Code to register Ruby objects with GC -#include "detail/Anchor.hpp" +// Code to register Ruby objects with GC (implementations) #include "detail/Anchor.ipp" -#include "Pin.hpp" #include "Pin.ipp" // C++ API declarations From 5e2cdf95810aec59d536f99697e8065ffe63982b Mon Sep 17 00:00:00 2001 From: cfis Date: Tue, 17 Feb 2026 01:07:14 -0800 Subject: [PATCH 6/8] Fix sometimes failing ownership test. --- test/test_Ownership.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/test/test_Ownership.cpp b/test/test_Ownership.cpp index 9b4a6c3a..6846017e 100644 --- a/test/test_Ownership.cpp +++ b/test/test_Ownership.cpp @@ -524,23 +524,20 @@ TESTCASE(NotCopyable) TESTCASE(MultipleOwnerReferences) { - detail::Registries::instance.instances.mode = detail::InstanceRegistry::Mode::All; - Module m = define_module("TestingModule"); - std::string code = R"( - box1 = OwnerBox.new - alias_obj = box1.alias - box2 = alias_obj.owner - box2.set_flag(99) - box1 = nil - GC.start - box2.flag - )"; - + + std::string code = R"(box1 = OwnerBox.new + alias_obj = box1.alias + box2 = alias_obj.owner + box1.object_id == box2.object_id)"; + + // With InstanceRegistry on, box2 should be the same Ruby object as box1. + detail::Registries::instance.instances.mode = detail::InstanceRegistry::Mode::All; Object result = m.module_eval(code); - ASSERT_EQUAL(99, detail::From_Ruby().convert(result.value())); + ASSERT_EQUAL(Qtrue, result.value()); + // With InstanceRegistry off, box2 should be a different Ruby object than box1. detail::Registries::instance.instances.mode = detail::InstanceRegistry::Mode::Off; result = m.module_eval(code); - ASSERT_NOT_EQUAL(99, detail::From_Ruby().convert(result.value())); + ASSERT_EQUAL(Qfalse, result.value()); } From bab1e51d94d6eb06ec7d3ec6c818e50188b0f285 Mon Sep 17 00:00:00 2001 From: cfis Date: Tue, 17 Feb 2026 01:08:02 -0800 Subject: [PATCH 7/8] Fix incorrect test that tries to show C++ dangling pointer. --- test/test_Instance_Registry.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/test/test_Instance_Registry.cpp b/test/test_Instance_Registry.cpp index 53c8231b..dcbd65c8 100644 --- a/test/test_Instance_Registry.cpp +++ b/test/test_Instance_Registry.cpp @@ -26,8 +26,15 @@ namespace public: static void reset() { - delete Factory::instance_; - Factory::instance_ = nullptr; + if (Factory::instance_) + { + // Zero out freed memory so AllMode_CppFreeUnderRubyWrapper_CrashRepro + // can reliably detect the dangling pointer across all platforms. + void* mem = Factory::instance_; + delete Factory::instance_; + std::memset(mem, 0, sizeof(MyClass)); + Factory::instance_ = nullptr; + } } public: @@ -289,16 +296,14 @@ TESTCASE(AllMode_CppFreeUnderRubyWrapper_CrashRepro) Factory::reset(); Module m = define_module("TestingModule"); - std::string code = R"( - factory = Factory.new - obj = factory.keep_pointer - # Simulate C++ freeing the object out from under Ruby by resetting the Factory, which deletes the MyClass instance. - Factory.reset - obj.flag - )"; + std::string code = R"(factory = Factory.new + obj = factory.keep_pointer + # Simulate C++ freeing the object out from under Ruby by resetting the Factory, which deletes the MyClass instance. + Factory.reset + obj.flag)"; // C++ frees the object out from under Ruby while Ruby still holds a wrapper. detail::Registries::instance.instances.mode = detail::InstanceRegistry::Mode::All; Object result = m.module_eval(code); - ASSERT_NOT_EQUAL(99, detail::From_Ruby().convert(result.value())); + ASSERT_NOT_EQUAL(123, detail::From_Ruby().convert(result.value())); } From 1b7c4598a448d750c5df85a7eb55e2bf2aba65f2 Mon Sep 17 00:00:00 2001 From: cfis Date: Tue, 17 Feb 2026 01:22:44 -0800 Subject: [PATCH 8/8] Last commit didn't work, so comment out the test for now. --- test/test_Instance_Registry.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/test/test_Instance_Registry.cpp b/test/test_Instance_Registry.cpp index dcbd65c8..1f7f643b 100644 --- a/test/test_Instance_Registry.cpp +++ b/test/test_Instance_Registry.cpp @@ -26,15 +26,8 @@ namespace public: static void reset() { - if (Factory::instance_) - { - // Zero out freed memory so AllMode_CppFreeUnderRubyWrapper_CrashRepro - // can reliably detect the dangling pointer across all platforms. - void* mem = Factory::instance_; - delete Factory::instance_; - std::memset(mem, 0, sizeof(MyClass)); - Factory::instance_ = nullptr; - } + delete Factory::instance_; + Factory::instance_ = nullptr; } public: @@ -291,6 +284,11 @@ TESTCASE(RubyObjectGced) ASSERT_EQUAL(std::string("MyClass"), className.str()); } +// Disabled: This test demonstrates that Ruby has no control over C++ managed +// object lifetimes. After Factory.reset deletes the C++ object, obj.flag reads +// freed memory. The result is undefined behavior — there is no portable way to +// assert on the value, so the test is unreliable across platforms. +/* TESTCASE(AllMode_CppFreeUnderRubyWrapper_CrashRepro) { Factory::reset(); @@ -307,3 +305,4 @@ TESTCASE(AllMode_CppFreeUnderRubyWrapper_CrashRepro) Object result = m.module_eval(code); ASSERT_NOT_EQUAL(123, detail::From_Ruby().convert(result.value())); } +*/