From 83ea698c76b34cf41c06729ac790ecb314903c2b Mon Sep 17 00:00:00 2001 From: Nils Wentzell Date: Tue, 21 Apr 2020 11:42:41 -0400 Subject: [PATCH] [cpp2py] Add option wrapped_members_as_shared_refs Previously member of a wrapped type were copied on access This commit introduces an option where access of wrapped members will avoid the copy at the cost of keeping the parent object alive for the full lifetime of the newly generated reference --- bin/c++2py.in | 4 +++- c++/cpp2py/py_converter.hpp | 46 +++++++++++++++++++++++++++---------- cpp2py/cpp2desc.py | 7 ++++-- cpp2py/mako/desc.py | 2 +- cpp2py/mako/wrap.cxx | 9 ++++++-- cpp2py/wrap_generator.py | 3 ++- 6 files changed, 52 insertions(+), 19 deletions(-) diff --git a/bin/c++2py.in b/bin/c++2py.in index 3e02f5a..c5ed8c2 100755 --- a/bin/c++2py.in +++ b/bin/c++2py.in @@ -37,6 +37,7 @@ parser.add_argument('--includes', '-I', action='append', help='Includes to pass parser.add_argument('--system_includes', '-isystem', action='append', help='System includes to pass to clang') parser.add_argument('--cxxflags', default = '', help='Options to pass to clang') parser.add_argument('--target_file_only', action='store_true', help='Disable recursion into included header files') +parser.add_argument('--wrapped_members_as_shared_refs', action='store_true', help='Disable recursion into included header files') args = parser.parse_args() @@ -77,7 +78,8 @@ W= Cpp2Desc(filename = args.filename, shell_command = shell_command, parse_all_comments = args.parse_all_comments, namespace_to_factor= (), # unused now - target_file_only = args.target_file_only + target_file_only = args.target_file_only, + wrapped_members_as_shared_refs = args.wrapped_members_as_shared_refs ) # Make the desc file diff --git a/c++/cpp2py/py_converter.hpp b/c++/cpp2py/py_converter.hpp index a16bcbc..3f8e4a0 100644 --- a/c++/cpp2py/py_converter.hpp +++ b/c++/cpp2py/py_converter.hpp @@ -116,20 +116,36 @@ namespace cpp2py { // default version is that the type is wrapped. // Will be specialized for type which are just converted. - template struct py_converter { + template struct py_converter { + + using T = std::decay_t; + static constexpr bool is_ref = std::is_reference_v; typedef struct { PyObject_HEAD; T *_c; + PyObject *parent = nullptr; } py_type; using is_wrapped_type = void; // to recognize - template static PyObject *c2py(U &&x) { + template static PyObject *c2py(U &&x, PyObject *parent = nullptr) { PyTypeObject *p = get_type_ptr(typeid(T)); if (p == nullptr) return NULL; py_type *self = (py_type *)p->tp_alloc(p, 0); - if (self != NULL) { self->_c = new T{std::forward(x)}; } + if (self != NULL) { + if constexpr (is_ref && wrapped_members_as_shared_refs) { + // Keep parent alive for lifetime of self + if (parent != nullptr) { + self->parent = parent; + Py_INCREF(parent); + self->_c = &x; + return (PyObject *)self; + } + } + // Create heap copy of x to guarantee lifetime + self->_c = new T{std::forward(x)}; + } return (PyObject *)self; } @@ -147,7 +163,7 @@ namespace cpp2py { if (p == nullptr) return false; if (PyObject_TypeCheck(ob, p)) { if (((py_type *)ob)->_c != NULL) return true; - auto err = std::string{"Severe internal error : Python object of "} + p->tp_name + " has a _c NULL pointer !!"; + auto err = std::string{"Severe internal error : Python object of "} + p->tp_name + " has a _c NULL pointer !!"; if (raise_exception) PyErr_SetString(PyExc_TypeError, err.c_str()); return false; } @@ -157,6 +173,12 @@ namespace cpp2py { } }; + // is_wrapped if py_converter has been reimplemented. + template struct is_wrapped : std::false_type {}; + template struct is_wrapped::is_wrapped_type> : std::true_type {}; + + template constexpr bool is_wrapped_v = is_wrapped::value; + // helpers for better error message // some class (e.g. range !) only have ONE conversion, i.e. C -> Py, but not both // we need to distinguish @@ -168,9 +190,15 @@ namespace cpp2py { struct does_have_a_converterC2Py>::c2py(std::declval()))>> : std::true_type {}; // We only use these functions in the code, not directly the converter - template static PyObject *convert_to_python(T &&x) { + template static PyObject *convert_to_python(T &&x, PyObject *parent = nullptr) { static_assert(does_have_a_converterC2Py::value, "The type does not have a converter from C++ to Python"); - return py_converter>::c2py(std::forward(x)); + PyObject *r; + if constexpr (is_wrapped_v>) { + r = py_converter::c2py(std::forward(x), parent); + } else { // Converted type + r = py_converter>::c2py(std::forward(x)); + } + return r; } template static bool convertible_from_python(PyObject *ob, bool raise_exception) { return py_converter::is_convertible(ob, raise_exception); @@ -188,12 +216,6 @@ namespace cpp2py { * */ - // is_wrapped if py_converter has been reimplemented. - template struct is_wrapped : std::false_type {}; - template struct is_wrapped::is_wrapped_type> : std::true_type {}; - - template constexpr bool is_wrapped_v = is_wrapped::value; - template static auto convert_from_python(PyObject *ob) -> decltype(py_converter::py2c(ob)) { static_assert(does_have_a_converterPy2C::value, "The type does not have a converter from Python to C++"); return py_converter::py2c(ob); diff --git a/cpp2py/cpp2desc.py b/cpp2py/cpp2desc.py index 555f651..f5ba14f 100644 --- a/cpp2py/cpp2desc.py +++ b/cpp2py/cpp2desc.py @@ -8,7 +8,7 @@ class Cpp2Desc: """ """ def __init__(self, filename, namespaces=(), classes= (), namespace_to_factor= (), appname= '', modulename = '', moduledoc ='', use_properties = False, members_read_only = True, converters = (), - compiler_options=None, includes= None, system_includes= None, libclang_location = None, shell_command = '', parse_all_comments = True, target_file_only = False): + compiler_options=None, includes= None, system_includes= None, libclang_location = None, shell_command = '', parse_all_comments = True, target_file_only = False, wrapped_members_as_shared_refs = False): """ Parse the file at construction @@ -59,9 +59,12 @@ def __init__(self, filename, namespaces=(), classes= (), namespace_to_factor= () target_file_only : bool Neglect any included files during desc generation [default = False] + + wrapped_members_as_shared_refs : bool + For classes with members which are a wrapped type, do not copy them on access but return them as shared references instead. Note that members with types that are only converted (e.g. std::vector) will continue to be copied on access [default = False] """ for x in ['filename', 'namespaces', 'classes', 'namespace_to_factor', 'appname', 'modulename', 'moduledoc', - 'use_properties', 'members_read_only', 'shell_command', 'target_file_only']: + 'use_properties', 'members_read_only', 'shell_command', 'target_file_only', 'wrapped_members_as_shared_refs']: setattr(self, x, locals()[x]) self.DE = dependency_analyzer.DependencyAnalyzer(converters) # parse the file diff --git a/cpp2py/mako/desc.py b/cpp2py/mako/desc.py index 742febf..1c9b5cc 100644 --- a/cpp2py/mako/desc.py +++ b/cpp2py/mako/desc.py @@ -3,7 +3,7 @@ from cpp2py.wrap_generator import * # The module -module = module_(full_name = "${W.modulename}", doc = r"${doc.replace_latex(W.moduledoc)}", app_name = "${W.appname}") +module = module_(full_name = "${W.modulename}", doc = r"${doc.replace_latex(W.moduledoc)}", app_name = "${W.appname}", wrapped_members_as_shared_refs = ${W.wrapped_members_as_shared_refs}) # Imports %if import_list: diff --git a/cpp2py/mako/wrap.cxx b/cpp2py/mako/wrap.cxx index c0df7b0..dd73dcb 100644 --- a/cpp2py/mako/wrap.cxx +++ b/cpp2py/mako/wrap.cxx @@ -4,6 +4,9 @@ #include //for std::cout... using dcomplex = std::complex; +// global options +constexpr bool wrapped_members_as_shared_refs = ${int(module.wrapped_members_as_shared_refs)}; + // first the basic stuff #include #include @@ -242,6 +245,7 @@ static PyObject* ${c.py_type}_richcompare (PyObject *a, PyObject *b, int op); typedef struct { PyObject_HEAD ${c.c_type} * _c; + PyObject * parent = nullptr; } ${c.py_type}; ## The new function, only if there is constructor @@ -257,7 +261,8 @@ static PyObject* ${c.py_type}_new(PyTypeObject *type, PyObject *args, PyObject * // dealloc static void ${c.py_type}_dealloc(${c.py_type}* self) { - if (self->_c != NULL) delete self->_c; // should never be null, but I protect it anyway + if ((self->_c != NULL) and (self->parent == nullptr)) delete self->_c; // should never be null, but I protect it anyway + Py_XDECREF(self->parent); Py_TYPE(self)->tp_free((PyObject*)self); } @@ -612,7 +617,7 @@ template <> struct py_converter<${en.c_name}> { static PyObject * ${c.py_type}__get_member_${m.py_name} (PyObject *self, void *closure) { auto & self_c = convert_from_python<${c.c_type}>(self); - return convert_to_python(self_c.${m.c_name}); + return convert_to_python(self_c.${m.c_name}, self); } %if not m.read_only: diff --git a/cpp2py/wrap_generator.py b/cpp2py/wrap_generator.py index 0dca66c..ebe05bd 100644 --- a/cpp2py/wrap_generator.py +++ b/cpp2py/wrap_generator.py @@ -688,7 +688,7 @@ class module_: """ Representation of a module """ - def __init__(self, full_name, doc = '', app_name = None) : + def __init__(self, full_name, doc = '', app_name = None, wrapped_members_as_shared_refs = False) : """ Parameters ---------- @@ -701,6 +701,7 @@ def __init__(self, full_name, doc = '', app_name = None) : """ self.full_name = full_name if app_name is None or app_name=="triqs" else app_name+"."+full_name + self.wrapped_members_as_shared_refs = wrapped_members_as_shared_refs self.name = full_name.rsplit('.',1)[-1] self.doc = doc self.classes = {} # dict : string -> class_. Key is the Python type