Skip to content

Carry a copy of the string also in the std::string_view converter and consider statefulness in InitializerListConverter #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wlav merged 3 commits into from
Mar 8, 2024
Merged

Carry a copy of the string also in the std::string_view converter and consider statefulness in InitializerListConverter #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70dfa81
Rule of 5 for Converter class
guitargeek Feb 19, 2024
e3011c3
Carry a copy of the string also in the `std::string_view` converter
guitargeek Feb 19, 2024
8b7145f
Separate converters for each element in initializer list converter
guitargeek Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 47 additions & 13 deletions src/Converters.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; \
} \
Expand Down Expand Up @@ -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;
Expand All @@ -1884,14 +1890,28 @@ 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;
}

return false;
}
bool CPyCppyy::STLStringViewConverter::ToMemory(
PyObject* value, void* address, PyObject* ctxt)
{
if (CPyCppyy_PyUnicodeAsBytes2Buffer(value, fStringBuffer)) {
fStringView = fStringBuffer;
*reinterpret_cast<std::string_view*>(address) = fStringView;
return true;
}
return InstanceConverter::ToMemory(value, address, ctxt);
}
#endif

CPyCppyy::STLWStringConverter::STLWStringConverter(bool keepControl) :
Expand All @@ -1902,9 +1922,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;
}
Expand Down Expand Up @@ -2852,9 +2872,20 @@ struct faux_initlist

} // unnamed namespace

CPyCppyy::InitializerListConverter::InitializerListConverter(Cppyy::TCppType_t klass, std::string const &value_type)

: InstanceConverter{klass},
fValueTypeName{value_type},
fValueType{Cppyy::GetScope(value_type)},
fValueSize{Cppyy::SizeOf(value_type)}
{
}

CPyCppyy::InitializerListConverter::~InitializerListConverter()
{
if (fConverter && fConverter->HasState()) delete fConverter;
for (Converter *converter : fConverters) {
if (converter && converter->HasState()) delete converter;
}
if (fBuffer) Clear();
}

Expand Down Expand Up @@ -2928,7 +2959,8 @@ bool CPyCppyy::InitializerListConverter::SetArg(
PyObject* item = PySequence_GetItem(pyobject, i);
bool convert_ok = false;
if (item) {
if (!fConverter) {
Converter *converter = CreateConverter(fValueTypeName);
if (!converter) {
if (CPPInstance_Check(item)) {
// by convention, use byte copy
memcpy((char*)fake->_M_array + i*fValueSize,
Expand All @@ -2948,7 +2980,10 @@ bool CPyCppyy::InitializerListConverter::SetArg(
"default ctor needed for initializer list of objects");
}
}
if (memloc) convert_ok = fConverter->ToMemory(item, memloc);
if (memloc) {
convert_ok = converter->ToMemory(item, memloc);
}
fConverters.emplace_back(converter);
}


Expand Down Expand Up @@ -3102,8 +3137,7 @@ CPyCppyy::Converter* CPyCppyy::CreateConverter(const std::string& fullType, cdim
// get the type of the list and create a converter (TODO: get hold of value_type?)
auto pos = realType.find('<');
std::string value_type = realType.substr(pos+1, realType.size()-pos-2);
return new InitializerListConverter(Cppyy::GetScope(realType),
CreateConverter(value_type), Cppyy::GetScope(value_type), Cppyy::SizeOf(value_type));
return new InitializerListConverter(Cppyy::GetScope(realType), value_type);
}

//-- still nothing? use a generalized converter
Expand Down
7 changes: 7 additions & 0 deletions src/Converters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ class CPYCPPYY_CLASS_EXPORT Converter {
public:
virtual ~Converter();

Converter() = default;

Converter(Converter const& other) = delete;
Converter(Converter && other) = delete;
Converter& operator=(Converter const& other) = delete;
Converter& operator=(Converter && other) = delete;

public:
virtual bool SetArg(PyObject*, Parameter&, CallContext* = nullptr) = 0;
virtual PyObject* FromMemory(void* address);
Expand Down
20 changes: 12 additions & 8 deletions src/DeclareConverters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,21 +351,26 @@ CPPYY_DECLARE_BASIC_CONVERTER(PyObject);
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; \
strtype fStringBuffer; \
}

CPPYY_DECLARE_STRING_CONVERTER(STLString, std::string);
#if __cplusplus > 201402L
CPPYY_DECLARE_STRING_CONVERTER(STLStringViewBase, std::string_view);
// The buffer type needs to be std::string also in the string_view case,
// otherwise the pointed-to string might not live long enough. See also:
// https://github.com/wlav/CPyCppyy/issues/13
CPPYY_DECLARE_STRING_CONVERTER(STLStringViewBase, std::string);
class STLStringViewConverter : public STLStringViewBaseConverter {
public:
virtual bool SetArg(PyObject*, Parameter&, CallContext* = nullptr);
virtual bool ToMemory(PyObject*, void*, PyObject* = nullptr);
private:
std::string_view fStringView;
};
#endif
CPPYY_DECLARE_STRING_CONVERTER(STLWString, std::wstring);
Expand Down Expand Up @@ -444,9 +449,7 @@ class SmartPtrConverter : public Converter {
// initializer lists
class InitializerListConverter : public InstanceConverter {
public:
InitializerListConverter(Cppyy::TCppType_t klass,
Converter* cnv, Cppyy::TCppType_t valuetype, size_t sz) : InstanceConverter(klass),
fBuffer(nullptr), fConverter(cnv), fValueType(valuetype), fValueSize(sz) {}
InitializerListConverter(Cppyy::TCppType_t klass, std::string const& value_type);
InitializerListConverter(const InitializerListConverter&) = delete;
InitializerListConverter& operator=(const InitializerListConverter&) = delete;
virtual ~InitializerListConverter();
Expand All @@ -459,8 +462,9 @@ class InitializerListConverter : public InstanceConverter {
void Clear();

protected:
void* fBuffer;
Converter* fConverter;
void* fBuffer = nullptr;
std::vector<Converter*> fConverters;
std::string fValueTypeName;
Cppyy::TCppType_t fValueType;
size_t fValueSize;
};
Expand Down