From a5222b71bcdd688a40b77d06f217b4d2748b35a9 Mon Sep 17 00:00:00 2001
From: Jonas Rembser <jonas.rembser@cern.ch>
Date: Mon, 19 Feb 2024 17:46:09 +0100
Subject: [PATCH] Carry a copy of the string also in the `std::string_view`
 converter

There are two possible fixes to the problem with string lifetimes and
`std::string_view` arguments:

  * setting a lifeline

  * copying the string

Copying the string is supposedly faster on average, at least in the ROOT
usecase the strings that are passed around are not very long (RDataFrame
filter and variable definitions).

This commit fixes the following reproducer:

```Python
import cppyy

cppyy.cppdef("""
void foo(std::string_view s1, std::string_view s2)
{
   std::cout << s1 << std::endl;
   std::cout << s2 << std::endl;
   std::cout << std::endl;
}

void bar(std::initializer_list<std::string_view> sl)
{
   for (auto const& s : sl) {
      std::cout << s << std::endl;
   }
   std::cout << std::endl;
}
""")

cppyy.gbl.foo("hello", "world")
```

Closes #13.
---
 src/Converters.cxx      | 26 ++++++++++++++++++--------
 src/DeclareConverters.h | 10 +++++++---
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/src/Converters.cxx b/src/Converters.cxx
index 661dbf5..9b68750 100644
--- a/src/Converters.cxx
+++ b/src/Converters.cxx
@@ -1833,8 +1833,8 @@ CPyCppyy::name##Converter::name##Converter(bool keepControl) :               \
 bool CPyCppyy::name##Converter::SetArg(                                      \
     PyObject* pyobject, Parameter& para, CallContext* ctxt)                  \
 {                                                                            \
-    if (CPyCppyy_PyUnicodeAsBytes2Buffer(pyobject, fBuffer)) {               \
-        para.fValue.fVoidp = &fBuffer;                                       \
+    if (CPyCppyy_PyUnicodeAsBytes2Buffer(pyobject, fStringBuffer)) {         \
+        para.fValue.fVoidp = &fStringBuffer;                                 \
         para.fTypeCode = 'V';                                                \
         return true;                                                         \
     }                                                                        \
@@ -1871,8 +1871,14 @@ CPPYY_IMPL_STRING_AS_PRIMITIVE_CONVERTER(STLStringViewBase, std::string_view, da
 bool CPyCppyy::STLStringViewConverter::SetArg(
     PyObject* pyobject, Parameter& para, CallContext* ctxt)
 {
-    if (this->STLStringViewBaseConverter::SetArg(pyobject, para, ctxt))
+    if (this->STLStringViewBaseConverter::SetArg(pyobject, para, ctxt)) {
+        // One extra step compared to the regular std::string converter:
+        // Create a corresponding std::string_view and set the parameter value
+        // accordingly.
+        fStringView = *reinterpret_cast<std::string*>(para.fValue.fVoidp);
+        para.fValue.fVoidp = &fStringView;
         return true;
+    }
 
     if (!CPPInstance_Check(pyobject))
         return false;
@@ -1884,8 +1890,12 @@ bool CPyCppyy::STLStringViewConverter::SetArg(
         if (!ptr)
             return false;
 
-        fBuffer = *((std::string*)ptr);
-        para.fValue.fVoidp = &fBuffer;
+        // Copy the string to ensure the lifetime of the string_view and the
+        // underlying string is identical.
+        fStringBuffer = *((std::string*)ptr);
+        // Create the string_view on the copy
+        fStringView = fStringBuffer;
+        para.fValue.fVoidp = &fStringView;
         para.fTypeCode = 'V';
         return true;
     }
@@ -1902,9 +1912,9 @@ bool CPyCppyy::STLWStringConverter::SetArg(
 {
     if (PyUnicode_Check(pyobject)) {
         Py_ssize_t len = CPyCppyy_PyUnicode_GET_SIZE(pyobject);
-        fBuffer.resize(len);
-        CPyCppyy_PyUnicode_AsWideChar(pyobject, &fBuffer[0], len);
-        para.fValue.fVoidp = &fBuffer;
+        fStringBuffer.resize(len);
+        CPyCppyy_PyUnicode_AsWideChar(pyobject, &fStringBuffer[0], len);
+        para.fValue.fVoidp = &fStringBuffer;
         para.fTypeCode = 'V';
         return true;
     }
diff --git a/src/DeclareConverters.h b/src/DeclareConverters.h
index 9e4c2b3..f712036 100644
--- a/src/DeclareConverters.h
+++ b/src/DeclareConverters.h
@@ -346,18 +346,20 @@ class VoidPtrPtrConverter : public Converter {
 
 CPPYY_DECLARE_BASIC_CONVERTER(PyObject);
 
-
+// A stateful string converter that always buffers a copy of the string.
+// This buffering is important also for the string_view case, otherwise the
+// pointed-to string might not live long enough. See also:
+// https://github.com/wlav/CPyCppyy/issues/13
 #define CPPYY_DECLARE_STRING_CONVERTER(name, strtype)                        \
 class name##Converter : public InstanceConverter {                           \
 public:                                                                      \
     name##Converter(bool keepControl = true);                                \
-public:                                                                      \
     virtual bool SetArg(PyObject*, Parameter&, CallContext* = nullptr);      \
     virtual PyObject* FromMemory(void* address);                             \
     virtual bool ToMemory(PyObject*, void*, PyObject* = nullptr);            \
     virtual bool HasState() { return true; }                                 \
 protected:                                                                   \
-    strtype fBuffer;                                                         \
+    std::basic_string<strtype::value_type> fStringBuffer;                    \
 }
 
 CPPYY_DECLARE_STRING_CONVERTER(STLString, std::string);
@@ -366,6 +368,8 @@ CPPYY_DECLARE_STRING_CONVERTER(STLStringViewBase, std::string_view);
 class STLStringViewConverter : public STLStringViewBaseConverter {
 public:
     virtual bool SetArg(PyObject*, Parameter&, CallContext* = nullptr);
+private:
+    std::string_view fStringView;
 };
 #endif
 CPPYY_DECLARE_STRING_CONVERTER(STLWString, std::wstring);