diff --git a/CHANGELOG.md b/CHANGELOG.md index 455153cf..a7ecf767 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,26 @@ # Changelog +## 4.11.3 (?) + +### Enhancements + +* Add support for `std::shared_ptr` +* Add support for `std::unique_ptr` + + +## 4.11.2 (2026-02-21) + +### Enhancements + +* Add support for `long double` +* Improve support for references to incomplete types + ## 4.11.1 (2026-02-18) ### Enhancements * Be more lenient on wrapp Qnil values in the C++ API - ## 4.11.0 (2026-02-17) This release focuses on improving memory management. diff --git a/include/rice/rice.hpp b/include/rice/rice.hpp index b09bf66c..85809b3c 100644 --- a/include/rice/rice.hpp +++ b/include/rice/rice.hpp @@ -3797,6 +3797,17 @@ namespace Rice::detail static inline std::string name = "Float"; }; + template<> + class RubyType + { + public: + using FromRuby_T = double(*)(VALUE); + + static inline FromRuby_T fromRuby = rb_num2dbl; + static inline std::string packTemplate = "d*"; + static inline std::string name = "Float"; + }; + template<> class RubyType { @@ -6073,6 +6084,35 @@ namespace Rice::detail } }; + template<> + struct Type + { + static bool verify() + { + return true; + } + + static VALUE rubyKlass() + { + return rb_cFloat; + } + }; + + template + struct Type + { + static bool verify() + { + define_buffer(); + return true; + } + + static VALUE rubyKlass() + { + return rb_cString; + } + }; + template<> struct Type { @@ -6601,6 +6641,62 @@ namespace Rice Arg* arg_ = nullptr; }; + // =========== long double ============ + template<> + class To_Ruby + { + public: + To_Ruby() = default; + + explicit To_Ruby(Arg* arg) : arg_(arg) + {} + + VALUE convert(const long double& native) + { + return protect(rb_float_new, native); + } + + private: + Arg* arg_ = nullptr; + }; + + template<> + class To_Ruby + { + public: + To_Ruby() = default; + + explicit To_Ruby(Arg* arg) : arg_(arg) + {} + + VALUE convert(const long double& native) + { + return protect(rb_float_new, native); + } + + private: + Arg* arg_ = nullptr; + }; + + template + class To_Ruby + { + public: + To_Ruby() = default; + + explicit To_Ruby(Arg* arg) : arg_(arg) + {} + + VALUE convert(long double data[N]) + { + Buffer buffer(data, N); + Data_Object> dataObject(std::move(buffer)); + return dataObject.value(); + } + private: + Arg* arg_ = nullptr; + }; + // =========== float ============ template<> class To_Ruby @@ -7879,6 +7975,86 @@ namespace Rice::detail Reference reference_; }; + // =========== long double ============ + template<> + class From_Ruby + { + public: + From_Ruby() = default; + + explicit From_Ruby(Arg* arg) : arg_(arg) + {} + + long double is_convertible(VALUE value) + { + return FromRubyFundamental::is_convertible(value); + } + + long double convert(VALUE value) + { + return FromRubyFundamental::convert(value); + } + + private: + Arg* arg_ = nullptr; + }; + + template<> + class From_Ruby + { + public: + using Reference_T = Reference; + + From_Ruby() = default; + + explicit From_Ruby(Arg* arg) : arg_(arg) + {} + + long double is_convertible(VALUE value) + { + switch (rb_type(value)) + { + case RUBY_T_DATA: + { + if (Data_Type::is_descendant(value)) + { + return Convertible::Exact; + } + [[fallthrough]]; + } + default: + { + return FromRubyFundamental::is_convertible(value); + } + } + } + + long double& convert(VALUE value) + { + switch (rb_type(value)) + { + case RUBY_T_DATA: + { + if (Data_Type::is_descendant(value)) + { + Reference_T* reference = unwrap(value, Data_Type::ruby_data_type(), false); + return reference->get(); + } + [[fallthrough]]; + } + default: + { + this->reference_ = Reference(value); + return this->reference_.get(); + } + } + } + + private: + Arg* arg_ = nullptr; + Reference reference_; + }; + // =========== float ============ template<> class From_Ruby @@ -8973,6 +9149,12 @@ namespace Rice::detail { return std::type_index(typeid(T)); } + else if constexpr (std::is_reference_v) + { + // For incomplete reference types, strip the reference and use pointer. + // Can't form T* when T is a reference type (pointer-to-reference is illegal). + return std::type_index(typeid(std::remove_reference_t*)); + } else { return std::type_index(typeid(T*)); @@ -14845,8 +15027,6 @@ namespace Rice template inline Data_Type& Data_Type::define_attr_internal(VALUE klass, std::string name, Attribute_T attribute, Access_T, const Arg_Ts&...args) { - using Attr_T = typename detail::attribute_traits::attr_type; - // Define attribute getter if constexpr (std::is_same_v || std::is_same_v) { diff --git a/lib/rice/version.rb b/lib/rice/version.rb index 72816c80..a42b3e4d 100644 --- a/lib/rice/version.rb +++ b/lib/rice/version.rb @@ -1,3 +1,3 @@ module Rice - VERSION = "4.11.1" + VERSION = "4.11.2" end diff --git a/rice/detail/Wrapper.ipp b/rice/detail/Wrapper.ipp index be7fb4df..64489cc4 100644 --- a/rice/detail/Wrapper.ipp +++ b/rice/detail/Wrapper.ipp @@ -121,7 +121,9 @@ namespace Rice::detail if constexpr (is_complete_v) { - if constexpr (std::is_destructible_v) + // is_abstract_v requires a complete type, so nest inside is_complete_v. + // Deleting an abstract class through a non-virtual destructor is UB. + if constexpr (std::is_destructible_v && !std::is_abstract_v) { if (this->isOwner_) { diff --git a/rice/detail/from_ruby.ipp b/rice/detail/from_ruby.ipp index 87873221..3495c531 100644 --- a/rice/detail/from_ruby.ipp +++ b/rice/detail/from_ruby.ipp @@ -1608,31 +1608,15 @@ namespace Rice::detail } } - void* convert(VALUE value) + std::nullptr_t convert(VALUE value) { if (value == Qnil) { return nullptr; } - if (this->arg_ && this->arg_->isOpaque()) - { - return (void*)value; - } - - switch (rb_type(value)) - { - case RUBY_T_NIL: - { - return nullptr; - break; - } - default: - { - throw Exception(rb_eTypeError, "wrong argument type %s (expected %s)", - detail::protect(rb_obj_classname, value), "nil"); - } - } + throw Exception(rb_eTypeError, "wrong argument type %s (expected %s)", + detail::protect(rb_obj_classname, value), "nil"); } private: Arg* arg_ = nullptr; diff --git a/rice/stl/map.ipp b/rice/stl/map.ipp index 8814c712..eacc9c05 100644 --- a/rice/stl/map.ipp +++ b/rice/stl/map.ipp @@ -156,7 +156,7 @@ namespace Rice }, Arg("key")) .define_method("[]=", [](T& map, Key_T key, Mapped_Parameter_T value) -> Mapped_T { - map[key] = value; + map.insert_or_assign(key, value); return value; }, Arg("key").keepAlive(), Arg("value").keepAlive()); @@ -278,7 +278,7 @@ namespace Rice // exceptions propogate back to Ruby return cpp_protect([&] { - result->operator[](From_Ruby().convert(key)) = From_Ruby().convert(value); + result->insert_or_assign(From_Ruby().convert(key), From_Ruby().convert(value)); return ST_CONTINUE; }); } diff --git a/rice/stl/pair.ipp b/rice/stl/pair.ipp index 0b555adf..92ee5c25 100644 --- a/rice/stl/pair.ipp +++ b/rice/stl/pair.ipp @@ -24,8 +24,12 @@ namespace Rice private: void define_constructors() { - klass_.define_constructor(Constructor()) - .define_constructor(Constructor(), Arg("x").keepAlive(), Arg("y").keepAlive()); + if constexpr (std::is_default_constructible_v) + { + klass_.define_constructor(Constructor()); + } + + klass_.define_constructor(Constructor(), Arg("x").keepAlive(), Arg("y").keepAlive()); if constexpr (std::is_copy_constructible_v && std::is_copy_constructible_v) { diff --git a/rice/stl/shared_ptr.ipp b/rice/stl/shared_ptr.ipp index c762dd49..c8f8b8c0 100644 --- a/rice/stl/shared_ptr.ipp +++ b/rice/stl/shared_ptr.ipp @@ -32,7 +32,11 @@ namespace Rice if constexpr (detail::is_complete_v && !std::is_void_v) { - result.define_constructor(Constructor(), Arg("value").takeOwnership()); + // is_abstract_v requires a complete type, so it must be nested inside the is_complete_v check + if constexpr (!std::is_abstract_v) + { + result.define_constructor(Constructor(), Arg("value").takeOwnership()); + } } // Forward methods to wrapped T @@ -87,7 +91,7 @@ namespace Rice::detail } else if (rb_typeddata_inherited_p(this->inner_rb_data_type_, requestedType)) { - return this->data_.get(); + return (void*)this->data_.get(); } else { diff --git a/rice/stl/unique_ptr.ipp b/rice/stl/unique_ptr.ipp index 6310d5c5..917d25ca 100644 --- a/rice/stl/unique_ptr.ipp +++ b/rice/stl/unique_ptr.ipp @@ -83,7 +83,7 @@ namespace Rice::detail } else if (rb_typeddata_inherited_p(this->inner_rb_data_type_, requestedType)) { - return this->data_.get(); + return (void*)this->data_.get(); } else { diff --git a/rice/stl/unordered_map.ipp b/rice/stl/unordered_map.ipp index ae32d015..9f742d47 100644 --- a/rice/stl/unordered_map.ipp +++ b/rice/stl/unordered_map.ipp @@ -156,7 +156,7 @@ namespace Rice }, Arg("key")) .define_method("[]=", [](T& unordered_map, Key_T key, Mapped_Parameter_T value) -> Mapped_T { - unordered_map[key] = value; + unordered_map.insert_or_assign(key, value); return value; }, Arg("key").keepAlive(), Arg("value").keepAlive()); @@ -278,7 +278,7 @@ namespace Rice // exceptions propogate back to Ruby return cpp_protect([&] { - result->operator[](From_Ruby().convert(key)) = From_Ruby().convert(value); + result->insert_or_assign(From_Ruby().convert(key), From_Ruby().convert(value)); return ST_CONTINUE; }); } diff --git a/test/test_From_Ruby.cpp b/test/test_From_Ruby.cpp index 55b0cb9c..3d0aad02 100644 --- a/test/test_From_Ruby.cpp +++ b/test/test_From_Ruby.cpp @@ -615,3 +615,17 @@ TESTCASE(void_pointer_array) Object result = m.module_eval(code); ASSERT_EQUAL("[4, 3, 2, 1]", detail::From_Ruby().convert(result.value())); } + +TESTCASE(nullptr_t) +{ + detail::From_Ruby fromRuby; + + std::nullptr_t result = fromRuby.convert(Qnil); + ASSERT_EQUAL(nullptr, result); + + ASSERT_EXCEPTION_CHECK( + Exception, + fromRuby.convert(rb_str_new2("not nil")), + ASSERT_EQUAL("wrong argument type String (expected nil)", ex.what()) + ); +} diff --git a/test/test_Stl_Map.cpp b/test/test_Stl_Map.cpp index 352935ae..9234578d 100644 --- a/test/test_Stl_Map.cpp +++ b/test/test_Stl_Map.cpp @@ -365,6 +365,62 @@ TESTCASE(NotPrintable) ASSERT_EQUAL("[Not printable]", detail::From_Ruby().convert(result)); } +namespace +{ + // Type with no default constructor - verifies insert_or_assign works in []= + class NoDefaultMap + { + public: + NoDefaultMap(int value) : value_(value) {} + int value() const { return value_; } + + bool operator==(const NoDefaultMap& other) const + { + return this->value_ == other.value_; + } + + private: + int value_; + }; +} + +TESTCASE(NoDefaultConstructor) +{ + define_class("NoDefaultMap"). + define_constructor(Constructor(), Arg("value")). + define_method("value", &NoDefaultMap::value); + + Class c = define_map("NoDefaultMap_Map"); + + Object map = c.call("new"); + + // Test []= (insert_or_assign) with non-default-constructible value + map.call("[]=", "one", NoDefaultMap(1)); + map.call("[]=", "two", NoDefaultMap(2)); + + Object result = map.call("size"); + ASSERT_EQUAL(2, detail::From_Ruby().convert(result)); + + // Test [] access + result = map.call("[]", "one"); + ASSERT_EQUAL(1, detail::From_Ruby().convert(result.call("value"))); + + // Test update existing key + map.call("[]=", "one", NoDefaultMap(10)); + result = map.call("[]", "one"); + ASSERT_EQUAL(10, detail::From_Ruby().convert(result.call("value"))); + + result = map.call("size"); + ASSERT_EQUAL(2, detail::From_Ruby().convert(result)); + + // Test delete + result = map.call("delete", "two"); + ASSERT_EQUAL(2, detail::From_Ruby().convert(result.call("value"))); + + result = map.call("size"); + ASSERT_EQUAL(1, detail::From_Ruby().convert(result)); +} + namespace { class Comparable diff --git a/test/test_Stl_Pair.cpp b/test/test_Stl_Pair.cpp index 8d854001..15f08e89 100644 --- a/test/test_Stl_Pair.cpp +++ b/test/test_Stl_Pair.cpp @@ -144,6 +144,38 @@ TESTCASE(AutoRegister) ASSERT(result.is_instance_of(pair.class_of())); } +namespace +{ + // Type with no default constructor - like cv::detail::tracking::tbm::Track + class NoDefault + { + public: + NoDefault(int value) : value_(value) {} + int value() const { return value_; } + private: + int value_; + }; +} + +TESTCASE(PairNoDefaultConstructor) +{ + define_class("NoDefault"). + define_constructor(Constructor(), Arg("value")). + define_method("value", &NoDefault::value); + + // This should compile and work even though NoDefault has no default constructor. + // The pair's default constructor should be skipped, but the two-argument constructor should work. + Class c = define_pair("IntNoDefaultPair"); + + Object pair = c.call("new", 42, NoDefault(7)); + + Object result = pair.call("first"); + ASSERT_EQUAL(42, detail::From_Ruby().convert(result)); + + result = pair.call("second"); + ASSERT_EQUAL(7, detail::From_Ruby().convert(result.call("value"))); +} + namespace { struct SomeStruct diff --git a/test/test_Stl_SharedPtr.cpp b/test/test_Stl_SharedPtr.cpp index d3940cf5..9f589e2a 100644 --- a/test/test_Stl_SharedPtr.cpp +++ b/test/test_Stl_SharedPtr.cpp @@ -609,6 +609,99 @@ TESTCASE(InheritedForwarding) ASSERT_EQUAL(456, detail::From_Ruby().convert(result)); } +// --- shared_ptr tests --- +// When shared_ptr wraps a const T, the Wrapper::get() method must handle +// the const-to-void* conversion. Without a cast, data_.get() returns +// const T* which cannot implicitly convert to void*. +namespace +{ + class ConstTarget + { + public: + int value; + + ConstTarget() : value(0) {} + ConstTarget(int v) : value(v) {} + + int getValue() const { return value; } + }; + + std::shared_ptr createConstTarget(int v) + { + return std::make_shared(v); + } + + int extractConstTargetValue(std::shared_ptr ptr) + { + return ptr->getValue(); + } +} + +TESTCASE(SharedPtrConstT) +{ + define_class("ConstTarget"). + define_constructor(Constructor(), + Arg("v")). + define_method("get_value", &ConstTarget::getValue); + + Module m = define_module("SharedPtrConstTest"). + define_module_function("create_const_target", &createConstTarget). + define_module_function("extract_const_target_value", &extractConstTargetValue); + + // Test that shared_ptr can be unwrapped to access the inner T + std::string code = R"(ptr = create_const_target(42) + extract_const_target_value(ptr))"; + + Object result = m.instance_eval(code); + ASSERT_EQUAL(42, detail::From_Ruby().convert(result.value())); +} + +// Abstract class with non-virtual destructor - like cv::cudacodec::NVSurfaceToColorConverter +namespace +{ + class AbstractClass + { + public: + virtual bool compute(int value) const = 0; + ~AbstractClass() {} // non-virtual destructor + }; + + class ConcreteImpl : public AbstractClass + { + public: + bool compute(int value) const override { return value > 0; } + }; + + std::shared_ptr createAbstract() + { + return std::make_shared(); + } + + bool useAbstract(std::shared_ptr ptr, int value) + { + return ptr->compute(value); + } +} + +TESTCASE(SharedPtrAbstractT) +{ + // shared_ptr where T is abstract should NOT define a constructor taking T*, + // because deleting through a non-virtual destructor on an abstract class is UB. + define_class("AbstractClass"). + define_method("compute", &AbstractClass::compute, + Arg("value")); + + Module m = define_module("SharedPtrAbstractTest"). + define_module_function("create_abstract", &createAbstract). + define_module_function("use_abstract", &useAbstract); + + std::string code = R"(ptr = create_abstract + use_abstract(ptr, 5))"; + + Object result = m.instance_eval(code); + ASSERT_EQUAL(Qtrue, result.value()); +} + // Forward declaration only - IncompleteClass is never defined class IncompleteClass; diff --git a/test/test_Stl_UniquePtr.cpp b/test/test_Stl_UniquePtr.cpp index 236615eb..dff6a3c7 100644 --- a/test/test_Stl_UniquePtr.cpp +++ b/test/test_Stl_UniquePtr.cpp @@ -252,4 +252,51 @@ TESTCASE(Release) Exception, m.module_eval(code), ASSERT(std::string(ex.what()).find("undefined method") == 0)); +} + +// --- unique_ptr tests --- +// When unique_ptr wraps a const T, the Wrapper::get() method must handle +// the const-to-void* conversion. Without a cast, data_.get() returns +// const T* which cannot implicitly convert to void*. +namespace +{ + class ConstTarget + { + public: + int value; + + ConstTarget() : value(0) {} + ConstTarget(int v) : value(v) {} + + int getValue() const { return value; } + }; + + std::unique_ptr createConstTarget(int v) + { + return std::make_unique(v); + } + + int extractConstTargetValue(const std::unique_ptr& ptr) + { + return ptr->getValue(); + } +} + +TESTCASE(UniquePtrConstT) +{ + define_class("ConstTarget"). + define_constructor(Constructor(), + Arg("v")). + define_method("get_value", &ConstTarget::getValue); + + Module m = define_module("UniquePtrConstTest"). + define_module_function("create_const_target", &createConstTarget). + define_module_function("extract_const_target_value", &extractConstTargetValue); + + // Test that unique_ptr can be unwrapped to access the inner T + std::string code = R"(ptr = create_const_target(42) + extract_const_target_value(ptr))"; + + Object result = m.instance_eval(code); + ASSERT_EQUAL(42, detail::From_Ruby().convert(result.value())); } \ No newline at end of file diff --git a/test/test_Stl_Unordered_Map.cpp b/test/test_Stl_Unordered_Map.cpp index b1168e3e..ee0df70a 100644 --- a/test/test_Stl_Unordered_Map.cpp +++ b/test/test_Stl_Unordered_Map.cpp @@ -357,6 +357,62 @@ TESTCASE(NotPrintable) ASSERT_EQUAL("[Not printable]", detail::From_Ruby().convert(result)); } +namespace +{ + // Type with no default constructor - verifies insert_or_assign works in []= + class NoDefaultUnorderedMap + { + public: + NoDefaultUnorderedMap(int value) : value_(value) {} + int value() const { return value_; } + + bool operator==(const NoDefaultUnorderedMap& other) const + { + return this->value_ == other.value_; + } + + private: + int value_; + }; +} + +TESTCASE(NoDefaultConstructor) +{ + define_class("NoDefaultUnorderedMap"). + define_constructor(Constructor(), Arg("value")). + define_method("value", &NoDefaultUnorderedMap::value); + + Class c = define_unordered_map("NoDefaultUnorderedMap_Map"); + + Object map = c.call("new"); + + // Test []= (insert_or_assign) with non-default-constructible value + map.call("[]=", "one", NoDefaultUnorderedMap(1)); + map.call("[]=", "two", NoDefaultUnorderedMap(2)); + + Object result = map.call("size"); + ASSERT_EQUAL(2, detail::From_Ruby().convert(result)); + + // Test [] access + result = map.call("[]", "one"); + ASSERT_EQUAL(1, detail::From_Ruby().convert(result.call("value"))); + + // Test update existing key + map.call("[]=", "one", NoDefaultUnorderedMap(10)); + result = map.call("[]", "one"); + ASSERT_EQUAL(10, detail::From_Ruby().convert(result.call("value"))); + + result = map.call("size"); + ASSERT_EQUAL(2, detail::From_Ruby().convert(result)); + + // Test delete + result = map.call("delete", "two"); + ASSERT_EQUAL(2, detail::From_Ruby().convert(result.call("value"))); + + result = map.call("size"); + ASSERT_EQUAL(1, detail::From_Ruby().convert(result)); +} + namespace { class Comparable