wlav / cppyy

Other
391 stars 40 forks source link

Pythonization of member function to yield Python ownership of passed parameter is undone by overriding #107

Open torokati44 opened 1 year ago

torokati44 commented 1 year ago

Currently I think it's impossible to mark pointer arguments of member functions as ownership-transferring in such a way that they "survive" being overridden - either from C++ or from Python.

The closest I could get is as demonstrated by the snippet below.

Sure, the pythonization could be extended to also include the derived classes, but for a base class in a framework that is meant to be subclassed by its users, this is (at least) inconventient and error-prone. Additionally, I think the ownership transfer (in either direction) is more a part of the contract that the base level method defines, and overrides should automatically abide by it.

And a similar problem is present with return values as well.


import cppyy

def replace_takes(klazz, name):
    if name == "Base":
        orig_take = klazz.take
        def take(self, obj):
            obj.__python_owns__ = False
            return orig_take(self, obj)
        klazz.take = take

        orig_take2 = klazz.take2
        def take2(self, obj):
            obj.__python_owns__ = False
            return orig_take2(self, obj)
        klazz.take2 = take2

cppyy.py.add_pythonization(replace_takes)

cppyy.cppdef("""

class Obj {};

class Base {
public:
    virtual void take(Obj *o) {
        std::cout << "in Base::take" << std::endl;
    }

    virtual void take2(Obj *o) {
        std::cout << "in Base::take2" << std::endl;
    }
};

class Derived : public Base {
public:
    virtual void take2(Obj *o) override {
        std::cout << "in Derived::take2" << std::endl;
    }
};

""")

class PyDerived(cppyy.gbl.Derived):
    def take2(self, obj):
        print("in PyDerived::take2")

b = cppyy.gbl.Base()
d = cppyy.gbl.Derived()
p = PyDerived()

print("==== testing Base::take")
o = cppyy.gbl.Obj()
print(o.__python_owns__) # True
b.take(o)
print(o.__python_owns__) # False

print("==== testing Derived::take")
o = cppyy.gbl.Obj()
print(o.__python_owns__) # True
d.take(o)
print(o.__python_owns__) # False

print("==== testing PyDerived::take")
o = cppyy.gbl.Obj()
print(o.__python_owns__) # True
p.take(o)
print(o.__python_owns__) # False

print("==== testing Base::take2")
o = cppyy.gbl.Obj()
print(o.__python_owns__) # True
b.take2(o)
print(o.__python_owns__) # False

print("==== testing Derived::take2")
o = cppyy.gbl.Obj()
print(o.__python_owns__) # True
d.take2(o)
print(o.__python_owns__) # True, but should be False

print("==== testing PyDerived::take2")
o = cppyy.gbl.Obj()
print(o.__python_owns__) # True
p.take2(o)
print(o.__python_owns__) # True, but should be False
wlav commented 1 year ago

I see. Yes, it does make sense, but there's no easy place to put it if coded like the above example: the semantics of owernship passing are completely encoded in Python and the methods of the derived class, being written in Python too, do not pass through cppyy to inspect or modify. I.e. there are no hooks beyond the pythonization one (and none for the Python subclass; or at least no programmatically accessible ones: there's always the metaclass).

Seems to me that at some level the true problem is that the pythonization has no good way of checking for klazz to be a subclass of some base.

There's some old history trying to flesh out the arguments' ownership rules, but it has always been put on a back burner since most cases are simple to solve with Pythonizations and, well, we have smart pointers now in the standard. :) Here it may be helpful as a start towards a solution though.

If the arguments are consistent in that a T* (as opposed a const T*, T& and const T&) is always transferring ownership, there is a memory policy designed for that:

def replace_takes(klazz, name):
    if name == "Base":
        klazz.take.__mempolicy__ = 0x000100
        klazz.take2.__mempolicy__ = 0x000100

At that point the intend is declared to cppyy, and we can start thinking of making the example work by having methods "inherit" the flags (still not trivial, but at least now the information is accessible to be passed on).

These flags also have a global variant, which should really be a namespace one, such that if a C++ project as a whole is consistent, the memory policy could be switched across the board with a single flag.

torokati44 commented 1 year ago

Fortunately, in our case, we control the construction of the objects of interest (through the usual factory pattern), so we can wrap the instance methods after the fact to set __python_owns__ on any parameters and the return value as needed.