From 8e3d93fba60e257227fcd96974694619e16a2e14 Mon Sep 17 00:00:00 2001
From: Charlie Savage <cfis@savagexi.com>
Date: Sat, 17 Feb 2024 01:09:08 -0800
Subject: [PATCH] Support pointers in STL collections like vectors. Fixes #192.

---
 rice/Data_Object.ipp     | 27 ++++++++++++++++++
 rice/stl/optional.ipp    |  2 +-
 rice/stl/string.ipp      | 58 +++++++++++++++++++++++++-------------
 rice/stl/vector.ipp      |  2 +-
 test/test_Stl_Vector.cpp | 60 ++++++++++++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 21 deletions(-)

diff --git a/rice/Data_Object.ipp b/rice/Data_Object.ipp
index 9d1e14b5..9f5452d4 100644
--- a/rice/Data_Object.ipp
+++ b/rice/Data_Object.ipp
@@ -299,6 +299,33 @@ namespace Rice::detail
     }
   };
 
+  template<typename T>
+  class From_Ruby<T*&>
+  {
+    static_assert(!std::is_fundamental_v<intrinsic_type<T>>,
+                  "Data_Object cannot be used with fundamental types");
+  public:
+    bool is_convertible(VALUE value)
+    {
+      return rb_type(value) == RUBY_T_DATA &&
+             Data_Type<T>::is_descendant(value);
+    }
+
+    T* convert(VALUE value)
+    {
+      using Intrinsic_T = intrinsic_type<T>;
+
+      if (value == Qnil)
+      {
+        return nullptr;
+      }
+      else
+      {
+        return Data_Object<Intrinsic_T>::from_ruby(value);
+      }
+    }
+  };
+
   template<typename T>
   class From_Ruby<Data_Object<T>>
   {
diff --git a/rice/stl/optional.ipp b/rice/stl/optional.ipp
index eacf53e7..8a2653d6 100644
--- a/rice/stl/optional.ipp
+++ b/rice/stl/optional.ipp
@@ -7,7 +7,7 @@ namespace Rice::detail
   {
     constexpr static bool verify()
     {
-      return Type<T>::verify();
+      return Type<intrinsic_type<T>>::verify();
     }
   };
 
diff --git a/rice/stl/string.ipp b/rice/stl/string.ipp
index 7233f53a..8e91f076 100644
--- a/rice/stl/string.ipp
+++ b/rice/stl/string.ipp
@@ -67,6 +67,40 @@ namespace Rice::detail
     Arg* arg_ = nullptr;
   };
 
+  template<>
+  class From_Ruby<std::string&>
+  {
+  public:
+    From_Ruby() = default;
+
+    explicit From_Ruby(Arg* arg) : arg_(arg)
+    {
+    }
+
+    bool is_convertible(VALUE value)
+    {
+      return rb_type(value) == RUBY_T_STRING;
+    }
+
+    std::string& convert(VALUE value)
+    {
+      if (value == Qnil && this->arg_ && this->arg_->hasDefaultValue())
+      {
+        return this->arg_->defaultValue<std::string>();
+      }
+      else
+      {
+        detail::protect(rb_check_type, value, (int)T_STRING);
+        this->converted_ = std::string(RSTRING_PTR(value), RSTRING_LEN(value));
+        return this->converted_;
+      }
+    }
+
+  private:
+    Arg* arg_ = nullptr;
+    std::string converted_;
+  };
+
   template<>
   class From_Ruby<std::string*>
   {
@@ -88,36 +122,22 @@ namespace Rice::detail
   };
 
   template<>
-  class From_Ruby<std::string&>
+  class From_Ruby<std::string*&>
   {
   public:
-    From_Ruby() = default;
-
-    explicit From_Ruby(Arg* arg) : arg_(arg)
-    {
-    }
-
     bool is_convertible(VALUE value)
     {
       return rb_type(value) == RUBY_T_STRING;
     }
 
-    std::string& convert(VALUE value)
+    std::string* convert(VALUE value)
     {
-      if (value == Qnil && this->arg_ && this->arg_->hasDefaultValue())
-      {
-        return this->arg_->defaultValue<std::string>();
-      }
-      else
-      {
-        detail::protect(rb_check_type, value, (int)T_STRING);
-        this->converted_ = std::string(RSTRING_PTR(value), RSTRING_LEN(value));
-        return this->converted_;
-      }
+      detail::protect(rb_check_type, value, (int)T_STRING);
+      this->converted_ = std::string(RSTRING_PTR(value), RSTRING_LEN(value));
+      return &this->converted_;
     }
 
   private:
-    Arg* arg_ = nullptr;
     std::string converted_;
   };
 }
\ No newline at end of file
diff --git a/rice/stl/vector.ipp b/rice/stl/vector.ipp
index 7ecd91bd..515049a6 100644
--- a/rice/stl/vector.ipp
+++ b/rice/stl/vector.ipp
@@ -365,7 +365,7 @@ namespace Rice
     {
       static bool verify()
       {
-        Type<T>::verify();
+        Type<intrinsic_type<T>>::verify();
 
         if (!detail::Registries::instance.types.isDefined<std::vector<T>>())
         {
diff --git a/test/test_Stl_Vector.cpp b/test/test_Stl_Vector.cpp
index d550b3b3..f3dcccc2 100644
--- a/test/test_Stl_Vector.cpp
+++ b/test/test_Stl_Vector.cpp
@@ -809,3 +809,63 @@ TESTCASE(ToEnumSize)
 
   ASSERT_EQUAL(3, detail::From_Ruby<int>().convert(result));
 }
+
+namespace
+{
+  std::vector<std::string*> vectorOfStringPointers()
+  {
+    std::vector<std::string*> result;
+    std::string* pString = new std::string("Hello");
+    result.push_back(pString);
+    return result;
+  }
+}
+
+TESTCASE(StringPointerVector)
+{
+  define_global_function("vector_of_string_pointers", &vectorOfStringPointers);
+
+  Module m(rb_mKernel);
+  Data_Object<std::vector<std::string*>> vec = m.call("vector_of_string_pointers");
+  ASSERT_EQUAL(1, vec->size());
+
+  std::string expected("Hello");
+  std::string* actual = (*vec)[0];
+  ASSERT_EQUAL(expected, *actual);
+}
+
+namespace
+{
+  class MyClass2
+  {
+  public:
+    MyClass2(std::string name): name(name)
+    {
+    }
+    std::string name;
+  };
+
+  std::vector<MyClass2*> vectorOfMyClass2Pointers()
+  {
+    std::vector<MyClass2*> result;
+    MyClass2* pMyClass = new MyClass2("Hello MyClass2");
+    result.push_back(pMyClass);
+    return result;
+  }
+}
+
+TESTCASE(MyClass2PointerVector)
+{
+  Class c = define_class<MyClass2>("MyClass2").
+    define_constructor(Constructor<MyClass2, std::string>()).
+    define_attr("name", &MyClass2::name, AttrAccess::Read);
+
+  define_global_function("vector_of_myclass2_pointers", &vectorOfMyClass2Pointers);
+
+  Module m(rb_mKernel);
+  Data_Object<std::vector<MyClass2*>> result = m.call("vector_of_myclass2_pointers");
+  ASSERT_EQUAL(1, result->size());
+
+ MyClass2* pMyClass = (*result)[0];
+ ASSERT_EQUAL("Hello MyClass2", pMyClass->name);
+}