Re: C++ to Python and returning members of a C++ object

Hi Jake,

Thanks for the information. That is one complex typemap syntax !
Is there any information on how this works and how to use it ?
I will have a try using it to implement a reference count system on all 
references to members returned from all C++ objects.

Terry
On 04/08/2020 22:35, Jake wrote:
> I don't believe anyone has said it would not be useful.  There are 
> just some questions about implementation.
>
> Since the Python container patch is in SWIG already and includes 
> machinery to set a back-reference (it effectively does the equivalent 
> of `child.__owner = parent` in Python), and this also accomplishes 
> automatic reference/dereference, why not re-use it rather than 
> re-implement in a different way?  It also does not require modifying 
> the native-code part of SWIG; much of SWIG's language support is 
> written in SWIG's language.
>
> Typemaps can be a little tricky to place sometimes but can be pretty 
> generic.  You seem to think you'd have to repeat them for all your 
> types, but this isn't really the case, e.g. my container patch sticks 
> one in:
>
>     %typemap(ret, fragment="reference_container_owner", noblock=1) 
> value_type& {
> (void)swig::container_owner<swig::traits<$*1_ltype>::category>::reference($result, 
> $self);
>     }
>
> This applies to all the container functions returning a reference to 
> the value type.  There is even a placeholder type `SWIGTYPE` you could 
> use, so something like
>
> %typemap(out) SWIGTYPE&
>
> Would apply to everything returned by reference, it does not need to 
> be repeated type-by-type.  However, it does give the user the option 
> of overriding the behavior by specifying their own typemap at the same 
> or a more specific level.
>
> The other thing the container patch has that could be re-used is that 
> template specialization on the type traits, which lets you distinguish 
> things which are wrapped as pointers vs values.  This is to prevent 
> setting a back-reference to something that got copied like an int, 
> less overhead and you don't pin the parent memory unnecessarily.
>
> William probably has some other subtleties in mind on when it should 
> apply or not.
> -Jake
>
> On Tue, Jul 28, 2020 at 1:44 AM Terry Barnaby <[email protected] 
> <mailto:[email protected]>> wrote:
>
>     Hi William,
>
>     As stated previously I have some reasonably large object
>     orientated C++ class libraries where classes/structs make use of
>     class members extensively and at depth. To have to add special
>     typemap for all of the cases where member objects are returned in
>     all situations would end up with a lot of work, a lot of SWIG code
>     to handle all the possible cases, would be difficult to test and,
>     I think,  unmaintainable.
>
>     I assume your idea with a typemap memberout would end up being
>     similar in that I would have to add a memberout typemap for every
>     class/struct type in the library ?
>
>     If so I would have thought this memberout code would end up having
>     to increment the reference on the container object and having some
>     code that decrements the reference on the container object when
>     the returned member is deleted similar to my simpler proposed method.
>
>     From my perspective, the ideal for SWIG, would be to wrap these
>     libraries with the absolute minimum of SWIG code with it just
>     using the C++ headers. I nearly have this apart from the returning
>     of dangling pointers, handling C fixed arrays (which I think SWIG
>     could also automatically do) and adding Python list methods to
>     simplify the use of the C++ lists in Python.
>
>     It seems to me that the current implementation (C++ -> Python),
>     where a member of a C++ object is returned by reference without
>     incrementing the reference on the container object is
>     fundamentally flawed. It quickly leads to dangling memory pointers
>     and segment faults that are horrible for a user to track down and
>     the fix of telling the user to create a copy of each C++ member
>     object they want to return from a Python function is also not good.
>
>     I thought my simple idea of just incrementing the reference of the
>     containing object whenever a pointer to its internal data is
>     returned for use and then decrementing this reference when the
>     returned member goes out of scope was the simplest, easiest and
>     most tidy way of achieving this with no SWIG code (apart from
>     turning the feature on at the top of the SWIG code file). It is
>     only a few lines of code added to SWIG and it should work in all
>     reference memberout cases and all depths automatically with very
>     little overhead. This would likely work for the std:vector and all
>     other such cases automatically. As far as I can see the only
>     detractor is the delayed container object destruction that could
>     affect some code, but I would have thought that would not affect
>     many cases and in these the feature could be turned off for the
>     particular classes or the user asked to make a copy of the
>     returned member.
>
>     Now I may be missing something fundamental in this, but if not
>     (and no one so far has stated why this won't work), why don't you
>     think this would be a useful feature in SWIG ?
>
>     Terry
>     On 27/07/2020 22:30, William S Fulton wrote:
>>     Hi Terry
>>
>>     I don't know why you have not based your solutions on the
>>     approaches mentioned in my original response. One of those is the
>>     std::vector solution that Jake also brought up. What I think
>>     might work is, instead of a new feature, is a new typemap that is
>>     only used when wrapping 'getter' member variables. Let's call it
>>     say memberout. It will contain the typemap code that I posted
>>     originally and only be generated when wrapping member variable
>>     getters. I'd need to think how to make it work only when needed,
>>     which I think is for all member types stored by value that are a
>>     SWIG wrapped proxy class type.
>>
>>     William
>>
>>     On Wed, 22 Jul 2020 at 13:45, Terry Barnaby <[email protected]
>>     <mailto:[email protected]>> wrote:
>>
>>         In effect, this is what my experimental patch does at the
>>         SWIG C++ wrapping level. It adds a PyObject* to the members
>>         SwigPyObject that is set to point to the container object
>>         that the members SwigPyObject is referencing. This PyObject*
>>         pointer is set on a members SwigPyObject object when a
>>         pointer/reference to a member is created as well as the
>>         container object having its reference counter incremented.
>>         When the members SwigPyObject is deleted it will use the
>>         PyObject* to decrement the container objects reference
>>         counter if the PyObject* is actually set. There is the
>>         addition of the pointer to every SwigPyObject and the call to
>>         the reference counter increment/decrement but that is all so
>>         it seems pretty efficient if this method is ok.
>>
>>         It is simple, but I am unsure of its implications in the
>>         wider SWIG codebase as I don't have much knowledge of this.
>>         So any comments on this approach welcome.
>>         If ok, I will have to work out how to use the "feature"
>>         functionality of SWIG to implement it properly.
>>
>>         Terry
>>
>>         On 22/07/2020 13:00, Jake wrote:
>>>         I support this idea too.  I had a PR that got into 4.0 for
>>>         essentially the same issue on containers like std::vector
>>>         that might be of interest:
>>>
>>>         https://github.com/swig/swig/pull/1234
>>>
>>>         Strategy there was to point to the parent wrapper from the
>>>         child wrapper to tie their GC lifetimes together.
>>>
>>>         -Jake
>>>
>>>         On Wed, Jul 22, 2020, 7:42 AM Terry Barnaby
>>>         <[email protected] <mailto:[email protected]>> wrote:
>>>
>>>             Hi,
>>>
>>>             Yes, it is a very simple patch that just increments the
>>>             reference counter on the SWIG wrapped C++ object when a
>>>             member of it is returned via a pointer/reference and
>>>             decrements that reference when the returned member is
>>>             finally deleted. The idea would be to implement this as
>>>             a SWIG feature than could be turned on for an individual
>>>             class/struct or the whole library. I believe it would be
>>>             of use if quite a few cases, but as said, I may be
>>>             missing something.
>>>
>>>             Terry
>>>             On 22/07/2020 10:57, D Haley wrote:
>>>>
>>>>             Dear List,
>>>>
>>>>             I would like to support the demand side of things for
>>>>             this, I maintain a small library that provide
>>>>             C++->python using swig, and we've had troubles with
>>>>             library users having memory corruption when using
>>>>             python. Our workaround has been to discourage the
>>>>             inadvertent creation of temporary variables, but it
>>>>             seems a poor solution.
>>>>
>>>>             As a library maintainer, it sounds like (and I've not
>>>>             gone through the patch in detail), that this reference
>>>>             counting approach would help minimise the chance of
>>>>             corruption by early collection?
>>>>
>>>>             Thanks.
>>>>

_______________________________________________
Swig-user mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/swig-user