ROOT 6.22.00/02: operators from C++ base class are not propagated in PyROOT

ROOT
,
1

Hello,

during the migration to a new PyROOT release in ROOT 6.22.00 I’ve found the case that looks like a regression to me.
Operators () and [] defined in C++ base classes are not propagated by PyROOT when other operators are defined in derived classes.
In new PyROOT definition of new operators prevents operators from base class to be considered at all.
In legacy PyROOT definition of new operators prevents only similar operators from base class to be considered at all, i.e. operator() in derived class would prevent operators() from base class to be considered for overload resolution.

The using Base::operator[]/() declaration has no effect on PyROOT behavior.

Here comes minimal reproducer (2.8 KB) for the problem where there is a base class with operator[] and operator() and 2 derived classes, with and without additional operators in derived class.
I’ve also put observed behaviors for PyROOTs I’ve tried as comments before each invokation of operators:

#!/usr/bin/python3

import ROOT
import cppyy


code = '''
struct Base {
    Base() {};

    const std::string& operator[](const std::string& x) {
        return x;
    };

    const std::string& operator()(const std::string& x) {
        return x;
    };

    int operator()(int x) {
        return x;
    };

};

struct Problematic: public Base {
    Problematic(): Base{} {};

    using Base::operator[];
    double operator()(double x) {
        return x+1;
    };

};

struct Good: public Base {
    Good(): Base{} {};
};
'''

def main():
    ROOT.gInterpreter.Declare(code)
    
    problem = cppyy.gbl.Problematic()
    good = cppyy.gbl.Good()

    try:
        # ROOT 6.22/00 -- modern PyROOT -- will fail
        # ROOT 6.22/00 -- legacy PyROOT -- will fail
        # ROOT 6.20/04 -- legacy PyROOT -- will fail
        print("Trying to call Problematic::operator()(const std::string&):")
        _ = problem('Call succeeded')
        print(_, '\n')
    except TypeError:
        print("Call to Problematic::operator()(const std::string&) failed\n")

    try:
        # ROOT 6.22/02 -- modern PyROOT -- will fail
        # ROOT 6.22/00 -- modern PyROOT -- will fail
        # ROOT 6.22/00 -- legacy PyROOT -- will succeed 
        # ROOT 6.20/04 -- legacy PyROOT -- will succeed 
        print("Trying to call Problematic::operator[](const std::string&):")
        _ = problem['Call succeeded']
        print(_, '\n')
    except TypeError:
        print("Call to Problematic::operator[](const std::string&) failed\n")
    
    try:
        # ROOT 6.22/02 -- modern PyROOT -- will select double overload 
        # ROOT 6.22/00 -- modern PyROOT -- will select double overload 
        # ROOT 6.22/00 -- legacy PyROOT -- will select double overload 
        # ROOT 6.20/04 -- legacy PyROOT -- will select double overload 
        print("Trying to call Problematic::operator()(int):")
        inp = int(1)
        _ = problem(inp)
        if inp != _:
            print("Called Problematic::operator()(double)\n")
        else:
            print("Call succeeded")
    except TypeError:
        print("Call to Problematic::operator()(int) failed\n")

    try:
        # ROOT 6.22/02 -- modern PyROOT -- will succeed 
        # ROOT 6.22/00 -- modern PyROOT -- will succeed 
        # ROOT 6.22/00 -- legacy PyROOT -- will succeed 
        # ROOT 6.20/04 -- legacy PyROOT -- will succeed 
        print("Trying to call Good::operator[](const std::string&):")
        _ = good['Call succeeded']
        print(_, '\n')
    except TypeError:
        print("Call to Good::operator[](const std::string&) failed\n")

    try:
        # ROOT 6.22/02 -- modern PyROOT -- will succeed 
        # ROOT 6.22/00 -- modern PyROOT -- will succeed 
        # ROOT 6.22/00 -- legacy PyROOT -- will succeed 
        # ROOT 6.20/04 -- legacy PyROOT -- will succeed 
        print("Trying to call Good::operator[](const std::string&):")
        _ = good['Call succeeded']
        print(_, '\n')
    except TypeError:
        print("Call to Good::operator[](const std::string&) failed\n")


if __name__ == "__main__":
    main()

The observed behavior that any operator definition in derived class blocks all operators from base class in new PyROOT looks very strange to me.

Please, can anyone help me to understand whether this behavior is expected from PyROOT or it is a regression?

Best wishes,
Konstantin

ROOT Version:
- ROOT 6.22.02 with modern PyROOT
- ROOT 6.22.00 with modern PyROOT
- ROOT 6.22.00 with legacy PyROOT
- ROOT 6.20.04 with legacy PyROOT
Platform: Ubuntu 18.04 LTS
Compiler:
- g++-9
Python version: 3.6.9
_CMake flags for ROOT: -D CMAKE_CXX_COMPILER="g++-9" -D CMAKE_C_COMPILER="gcc-9" -D CMAKE_CXX_STANDARD=17 -D python_version=3 -D minuit2=ON -D builtin_xrootd=ON

2

Hi Konstantin,

Thank you again for the complete reproducer! I’ll have a look and get back to you.

3

Hi Konstantin,

Just one question: so if I understand correctly from your comments in the code, the only test in which the behavior of old and new PyROOT are different is the second one? I just tried with upstream cppyy and that second test succeeds, so I need to investigate why it doesn’t in our PyROOT.

Also, I see you use using declarations in your code, there is a known issue related to that which is now being fixed:
https://sft.its.cern.ch/jira/browse/ROOT-10872

but I can’t say for sure right now that this is the cause of the behavior you observe, I only mention it as reference.

4

Dear Enric,
thanks for the quick reply!

Just one question: so if I understand correctly from your comments in the code, the only test in which the behavior of old and new PyROOT are different is the second one? I just tried with upstream cppyy and that second test succeeds, so I need to investigate why it doesn’t in our PyROOT.

That was a starting case for me.
Originally, the code I work on uses mixin-classes to add operator[](const std::string&) to sequence containter class that implements only index-based access. And one of classes that uses such mixin also defines operator(). After it all operators from mixin are not visible in Python.

Also, I see you use using declarations in your code, there is a known issue related to that which is now being fixed:
Loading...

Using declaration has no effect on the problem, I’ve added it just for completeness.

But even in case of legacy PyROOT it seems strange to me that presence of operator() in derived class blocks operator() inherited from base class.

Best wishes,
Konstantin

5

Hi,

I can make a few comments after doing a bit of investigation:

  • The addition of using Base::operator[] in Problematic does have an influence. In upstream cppyy, this addition is what makes operator[] callable in Problematic if operator() is also redefined. This is not how it should be, obviously. If the using declaration is not there, it seems operator()(double) is tried when calling operator[](string) in a Problematic object, which is bad and needs to be fixed. This “hiding” of operator[] when operator() is redefined does not happen in old PyROOT, just new.

  • In test 1 and test 3, the choice of operator()(double) is what should happen. This also happens in C++, where this program will fail to compile because of p(s):

#include <string>
#include <iostream>

struct Base {
    Base() {};

    const std::string& operator[](const std::string& x) {
        return x;
    };

    const std::string& operator()(const std::string& x) {
        return x;
    };

    int operator()(int x) {
        return x;
    };

};

struct Problematic: public Base {
    Problematic(): Base{} {};

    double operator()(double x) {
        return x+1;
    };

};

int main() {
  Problematic p;
  std::string s("AAA");

  std::cout << p[s] << std::endl;
  std::cout << p(1) << std::endl;
  std::cout << p(s) << std::endl;

  return 0;
}

and, if we comment out the last print, the program will compile but operator()(double) will be invoked when calling p(1).

6

I opened this JIRA ticket:

https://sft.its.cern.ch/jira/browse/ROOT-11012

to follow the issue.

7

Hi Enric,

thanks for the update!
I will follow JIRA ticket for further information

8

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.