From 38e348e71cfac073fd73722696fa64907b8437eb Mon Sep 17 00:00:00 2001 From: cfis Date: Thu, 14 Nov 2024 23:37:33 -0800 Subject: [PATCH] Fix enums to return the correct value via the #to_int method. The problem was the self parameter was incorrect. This fix allows enums to be passed to C++ methods that take parameters of type int instead of an enum type. --- rice/Enum.ipp | 30 ++++++---- rice/detail/from_ruby.ipp | 66 +++++++++++++++++++++- test/test_Enum.cpp | 112 ++++++++++++++++++++++++++------------ 3 files changed, 162 insertions(+), 46 deletions(-) diff --git a/rice/Enum.ipp b/rice/Enum.ipp index 21d560f7..220bd295 100644 --- a/rice/Enum.ipp +++ b/rice/Enum.ipp @@ -32,34 +32,44 @@ namespace Rice // First we need a constructor klass.define_constructor(Constructor()); - // Instance methods - klass.define_method("to_s", [](Enum_T& self) + // Instance methods. The self parameter is confusing because it is really a Data_Object. + // However, if we make that the type then the From_Ruby code will consider it a + // Data_Type>>. But in define class above it was actually bound as + // Data_Type. Thus the static_casts in the methods below. + klass.define_method("to_s", [](Enum_T& notSelf) { // We have to return string because we don't know if std::string support has // been included by the user - return String(valuesToNames_[self]); + Data_Object self = static_cast>(notSelf); + return String(valuesToNames_[*self]); }) - .define_method("to_i", [](Enum_T& self) -> Underlying_T + .define_method("to_int", [](Enum_T& notSelf) -> Underlying_T { - return (Underlying_T)self; + Data_Object self = static_cast>(notSelf); + return static_cast(*self); }) - .define_method("inspect", [](Enum_T& self) + .define_method("inspect", [](Enum_T& notSelf) { + Data_Object self = static_cast>(notSelf); + std::stringstream result; VALUE rubyKlass = Enum::klass().value(); result << "#<" << detail::protect(rb_class2name, rubyKlass) - << "::" << Enum::valuesToNames_[self] << ">"; + << "::" << Enum::valuesToNames_[*self] << ">"; // We have to return string because we don't know if std::string support has // been included by the user return String(result.str()); }) - .define_method("hash", [](Enum_T& self) -> Underlying_T + .define_method("hash", [](Enum_T& notSelf) -> Underlying_T { - return (Underlying_T)self; + Data_Object self = static_cast>(notSelf); + return (Underlying_T)*self; }) - .define_method("eql?", [](Enum_T& self, Enum_T& other) + .define_method("eql?", [](Enum_T& notSelf, Enum_T& notOther) { + Data_Object self = static_cast>(notSelf); + Data_Object other = static_cast>(notOther); return self == other; }); diff --git a/rice/detail/from_ruby.ipp b/rice/detail/from_ruby.ipp index 055e4888..dd076dc0 100644 --- a/rice/detail/from_ruby.ipp +++ b/rice/detail/from_ruby.ipp @@ -5,6 +5,7 @@ #include #include "../Exception_defn.hpp" #include "../Arg.hpp" +#include "../Identifier.hpp" #include "RubyFunction.hpp" /* This file implements conversions from Ruby to native values fo fundamental types @@ -163,6 +164,23 @@ namespace Rice::detail case RUBY_T_FIXNUM: return Convertible::Exact; break; + + // This case is for Enums which are defined as Ruby classes. Some C++ apis + // will take a int parameter but really what we have is an Enum + case RUBY_T_DATA: + { + static ID id = protect(rb_intern, "to_int"); + if (protect(rb_respond_to, value, id)) + { + return Convertible::TypeCast; + } + else + { + return Convertible::None; + } + + break; + } default: return Convertible::None; } @@ -176,6 +194,7 @@ namespace Rice::detail } else { + // rb_num2long_inline will call to_int for RUBY_T_DATA objects return (int)protect(rb_num2long_inline, value); } } @@ -1106,7 +1125,7 @@ namespace Rice::detail } } } - + template<> class From_Ruby { @@ -1262,7 +1281,7 @@ namespace Rice::detail } }; - // =========== unsinged char ============ + // =========== unsigned char ============ template<> class From_Ruby { @@ -1305,6 +1324,49 @@ namespace Rice::detail Arg* arg_ = nullptr; }; + template<> + class From_Ruby + { + public: + From_Ruby() = default; + + explicit From_Ruby(Arg* arg) : arg_(arg) + { + } + + Convertible is_convertible(VALUE value) + { + switch (rb_type(value)) + { + case RUBY_T_STRING: + return Convertible::Exact; + break; + // This is for C++ chars which are converted to Ruby integers + case RUBY_T_FIXNUM: + return Convertible::TypeCast; + break; + default: + return Convertible::None; + } + } + + unsigned char* convert(VALUE value) + { + if (value == Qnil) + { + return nullptr; + } + else + { + detail::protect(rb_check_type, value, (int)T_STRING); + return reinterpret_cast(RSTRING_PTR(value)); + } + } + + private: + Arg* arg_ = nullptr; + }; + // =========== signed char ============ template<> class From_Ruby diff --git a/test/test_Enum.cpp b/test/test_Enum.cpp index 45c994d9..e5ae21d2 100644 --- a/test/test_Enum.cpp +++ b/test/test_Enum.cpp @@ -9,19 +9,15 @@ using namespace Rice; TESTSUITE(Enum); +SETUP(Enum) +{ + embed_ruby(); +} + namespace { enum Color { RED, BLACK, GREEN }; - Enum define_color_enum() - { - static Enum colors = define_enum("Color") - .define_value("RED", RED) - .define_value("BLACK", BLACK) - .define_value("GREEN", GREEN); - return colors; - } - enum class Season { Spring, Summer, Fall, Winter }; // This is needed to make unittest compile (it uses ostream to report errors) @@ -30,22 +26,26 @@ namespace os << static_cast>(season); return os; } +} - Enum define_season_enum() - { - static Enum seasons = define_enum("Season") - .define_value("Spring", Season::Spring) - .define_value("Summer", Season::Summer) - .define_value("Fall", Season::Fall) - .define_value("Winter", Season::Winter); - - return seasons; - } +Enum define_color_enum() +{ + static Enum colors = define_enum("Color") + .define_value("RED", RED) + .define_value("BLACK", BLACK) + .define_value("GREEN", GREEN); + return colors; } -SETUP(Enum) +Enum define_season_enum() { - embed_ruby(); + static Enum seasons = define_enum("Season") + .define_value("Spring", Season::Spring) + .define_value("Summer", Season::Summer) + .define_value("Fall", Season::Fall) + .define_value("Winter", Season::Winter); + + return seasons; } TESTCASE(copy_construct) @@ -157,14 +157,14 @@ TESTCASE(to_s) ASSERT_EQUAL(String("GREEN"), String(m.module_eval("Color::GREEN.to_s"))); } -TESTCASE(to_i) +TESTCASE(to_int) { Module m = define_module("Testing"); Enum colorEnum = define_color_enum(); - ASSERT_EQUAL(detail::to_ruby(int(RED)), m.module_eval("Color::RED.to_i").value()); - ASSERT_EQUAL(detail::to_ruby(int(BLACK)), m.module_eval("Color::BLACK.to_i").value()); - ASSERT_EQUAL(detail::to_ruby(int(GREEN)), m.module_eval("Color::GREEN.to_i").value()); + ASSERT_EQUAL(detail::to_ruby(int(RED)), m.module_eval("Color::RED.to_int").value()); + ASSERT_EQUAL(detail::to_ruby(int(BLACK)), m.module_eval("Color::BLACK.to_int").value()); + ASSERT_EQUAL(detail::to_ruby(int(GREEN)), m.module_eval("Color::GREEN.to_int").value()); } TESTCASE(inspect) @@ -283,9 +283,9 @@ TESTCASE(nested_enums) Module m = define_module("Testing"); - ASSERT_EQUAL(detail::to_ruby(int(0)), m.module_eval("Inner::Props::VALUE1.to_i").value()); - ASSERT_EQUAL(detail::to_ruby(int(1)), m.module_eval("Inner::Props::VALUE2.to_i").value()); - ASSERT_EQUAL(detail::to_ruby(int(2)), m.module_eval("Inner::Props::VALUE3.to_i").value()); + ASSERT_EQUAL(detail::to_ruby(int(0)), m.module_eval("Inner::Props::VALUE1.to_int").value()); + ASSERT_EQUAL(detail::to_ruby(int(1)), m.module_eval("Inner::Props::VALUE2.to_int").value()); + ASSERT_EQUAL(detail::to_ruby(int(2)), m.module_eval("Inner::Props::VALUE3.to_int").value()); } namespace @@ -295,9 +295,14 @@ namespace return RED; } - bool isMyFavoriteColor(Color aColor) + bool isMyFavoriteColor(Color color) { - return aColor == RED; + return color == RED; + } + + bool myFavoriteColorAsInt(int color) + { + return color == RED; } } @@ -305,9 +310,8 @@ TESTCASE(using_enums) { Enum colorEnum = define_color_enum(); colorEnum.define_singleton_function("my_favorite_color", &myFavoriteColor) - .define_singleton_function("is_my_favorite_color", &isMyFavoriteColor) - .define_singleton_function("is_my_favorite_color", &isMyFavoriteColor) - .define_method("is_my_favorite_color", &isMyFavoriteColor); + .define_singleton_function("is_my_favorite_color", &isMyFavoriteColor) + .define_method("is_my_favorite_color", &isMyFavoriteColor); Module m = define_module("Testing"); @@ -327,6 +331,46 @@ TESTCASE(using_enums) ASSERT_EQUAL(Qfalse, result.value()); } +TESTCASE(enum_to_int) +{ + Enum colorEnum = define_color_enum(); + + Module m = define_module("Testing"); + m.define_module_function("my_favorite_color_as_int", &myFavoriteColorAsInt); + + std::string code = R"(my_favorite_color_as_int(Color::RED))"; + Object result = m.module_eval(code); + ASSERT_EQUAL(Qtrue, result.value()); + + code = R"(my_favorite_color_as_int(Color::GREEN))"; + result = m.module_eval(code); + ASSERT_EQUAL(Qfalse, result.value()); +} + +namespace +{ + bool isMyFavoriteSeasonAsInt(int season) + { + return ((Season)season == Season::Summer); + } +} + +TESTCASE(enum_class_to_int) +{ + define_season_enum(); + + Module m = define_module("Testing"); + m.define_module_function("is_my_favorite_season_as_int", &isMyFavoriteSeasonAsInt); + + std::string code = R"(is_my_favorite_season_as_int(Season::Spring))"; + Object result = m.module_eval(code); + ASSERT_EQUAL(Qfalse, result.value()); + + code = R"(is_my_favorite_season_as_int(Season::Summer))"; + result = m.module_eval(code); + ASSERT_EQUAL(Qtrue, result.value()); +} + namespace { Color defaultColor(Color aColor = BLACK) @@ -377,4 +421,4 @@ TESTCASE(not_defined) define_global_function("undefined_return", &undefinedReturn), ASSERT_EQUAL(message, ex.what()) ); -} +} \ No newline at end of file