[PyROOT] Infinite recursion in cross inheritance when invoking method of base class

[etejedor]

Describe the bug

Reported in: https://root-forum.cern.ch/t/updated-pyroot-in-current-master-6-23-01

The following is a standalone reproducer. When running the code below:

import cppyy

cppyy.cppdef("""
class MyCppClass {
  public:
    virtual void MyMethod() { cout << "In C++" << endl; }
    virtual ~MyCppClass() {}
};
""")

class MyPyClass(cppyy.gbl.MyCppClass):
    def MyMethod(self):
        super(MyPyClass, self).MyMethod()
        print("In Python")

a = MyPyClass()
a.MyMethod()

it goes in infinite recursion when invoking a.MyMethod(), in particular in super(MyPyClass, self).MyMethod(), which calls again the MyMethod defined in Python.

Expected behavior

That super(MyPyClass, self).MyMethod() invokes MyCppClass::MyMethod (behaviour of old PyROOT).

To Reproduce

Described above.

Setup

ROOT 6.23/01, Ubuntu18, compiled from sources.

[etejedor]

This is a C++ reproducer of what's going on:

class MyCppClass {
  public:
    virtual void MyMethod() { cout << "In MyCppClass" << endl; }
    virtual ~MyCppClass() {}
};

namespace __cppyy_internal {
class Dispatcher1 : public ::MyCppClass {
public:
  virtual ~Dispatcher1() {}
  void MyMethod() {
    cout << "In Dispatcher1" << endl;
  }
  using MyCppClass::MyCppClass;
  Dispatcher1() {}
  Dispatcher1(const Dispatcher1& other) : MyCppClass(other) {}
};
}

TInterpreter::CallFuncIFacePtr_t GetCallFunc(TFunction *f)
{
    CallFunc_t* callf = gInterpreter->CallFunc_Factory();
    MethodInfo_t* meth = gInterpreter->MethodInfo_Factory(f->GetDeclId());
    gInterpreter->CallFunc_SetFunc(callf, meth);
    gInterpreter->MethodInfo_Delete(meth);

    auto oldErrLvl = gErrorIgnoreLevel;
    gErrorIgnoreLevel = kFatal;
    auto faceptr = gInterpreter->CallFunc_IFacePtr(callf);
    gErrorIgnoreLevel = oldErrLvl;

    gInterpreter->CallFunc_Delete(callf);

    return faceptr;
}

void root_6637()
{
    TClassRef c0("MyCppClass");
    TFunction *f0 = (TFunction*)c0->GetListOfMethods()->At(0);
    cout << "METHOD: " << f0->GetPrototype() << endl;

    const TInterpreter::CallFuncIFacePtr_t& faceptr0 = GetCallFunc(f0);

    __cppyy_internal::Dispatcher1 *d = new __cppyy_internal::Dispatcher1();

    void* smallbuf0[8];
    void* objresult0 = nullptr;
    faceptr0.fGeneric((void*)d, 0, smallbuf0, &objresult0);
}

The macro above prints:

METHOD: void MyCppClass::MyMethod()
In Dispatcher1

So I'm trying to invoke MyCppClass::MyMethod() on a Dispatcher1 object, but what is invoked is Dispatcher1::MyMethod(). To be checked with @Axel-Naumann if the TCling code above should (and is intended to) do the equivalent of d->MyCppClass::MyMethod() or it should behave as d->MyMethod() (as it does right now).

[Axel-Naumann]

I'd say that's indeed the expected behavior; I'm fairly certain that hell will break loose e.g. in the I/O if we were to change this. @pcanal do you have anything to add here?

[etejedor]

Thanks for having a look. What would need to change then, in the code above, so that the equivalent of d->MyCppClass::MyMethod() happens?

[Axel-Naumann]

I don't think CallFunc ever offered that. Unless I am mistaken, of course :-) Was this working in the past?

[etejedor]

Yes, this worked in the old PyROOT. As far as I understand, this worked because there was no Dispatcher wrapper class in between the Python class and the C++ base class. Therefore, the proxied C++ object was a base class object, not a Dispatcher object, and the invocation of the base class MyMethod on that object worked with a code equivalent to the one in my C++ reproducer.

[Axel-Naumann]

Sorry, my question was specifically about the old callfunc / member iteration implementation. I.e. is your C++ example also calling the derived class's function in older ROOT versions?

[etejedor]

Sorry, my question was specifically about the old callfunc / member iteration implementation. I.e. is your C++ example also calling the derived class's function in older ROOT versions?

Yes, I just tried my example with 6.20/06, 6.18/04 and 6.16/00, all of them call the derived class's function.

[pcanal]

This seems to be a long standing "feature". When the I/O used to rely on the interpreter for streaming, we even had to introduce Class::StreamerNVirtual to work around the missing feature.

I'm fairly certain that hell will break loose e.g. in the I/O if we were to change this.

I think it would be okay as the I/O does not rely on CallFunc (but on generated accelerator functions) to work.

I'd say that's indeed the expected behavior;

Yes, this is the most commonly expected behavior but for the same reasons that C++ has the notation d->MyCppClass::MyMethod() there are cases where you would want to be able to call an instance of the virtual function explicitly. With the current implementation of TClingCallFunc (it generates text), it would be trivial to add the option (i.e. add the ::MyCppClass in the right place(s) when requested.

[guitargeek]

I would consider this a low priority item. One can easily work around the problem with an intermediate C++ class:

import cppyy

cppyy.cppdef("""

class MyCppClass {
  public:
    virtual void MyMethod() { std::cout << "In C++" << std::endl; }
    virtual ~MyCppClass() {}
};

class MyCppClassBridge : public MyCppClass {
  public:
    void _MyMethodBase() { MyCppClass::MyMethod(); }
};

""")

class MyPyClass(cppyy.gbl.MyCppClassBridge):
    def MyMethod(self):
        self._MyMethodBase()
        print("In Python")

a = MyPyClass()
a.MyMethod()

So we don't really need to support super(MyPyClass, self).MyMethod() to make the pattern of calling the base class method in PyROOT work.

Given that the user in the forum who originally discovered this problem could also work around it, I would assign low priority to restoring this old PyROOT behavior from now more than 5 years ago.

[wlav]

In master, the trivial workaround is to use C++ base method call syntax instead of Python syntax:

class MyPyClass(cppyy.gbl.MyCppClass):
    def MyMethod(self):
        cppyy.gbl.MyCppClass.MyMethod(self)
        print("In Python")

(This requires additional support in CallFunc as the dispatch is slightly different.)

[aaronj0]

Closing as this looks like this issue has workarounds avoiding calls like super(MyPyClass, self).MyMethod()