I’ve been updating the code I work on to a newer version of ROOT (6.22.00) with new PyROOT implementation as stated by the release notes.
During the transition, I’ve discovered a case where it seems to me that the new PyROOT selects inconsistent overloads when calling constructors for custom objects.
Here is a minimal example in Python with omitting all irrelevant functionality to reproduce this behavior. It is t is also attached as ctor_example.py (1.4 KB):
#!/usr/bin/python3
import ROOT
def main():
ROOT.gInterpreter.Declare('''
#include <iostream>
#include <memory>
#include <complex>
template<typename T>
class arrayview {
public:
arrayview(std::size_t size) : m_size(size) {
std::cout << "Called single argument ctor with size " << size << std::endl;
if(!size) {
return ;
}
m_buffer.reset(new T[size]);
}
arrayview(std::complex<T> other) : arrayview(2) {
std::cout << "Called std::complex ctor" << std::endl;
this->complex() = other;
}
std::complex<T>& complex() { return *reinterpret_cast<std::complex<T>*>(m_buffer.get()); }
private:
std::unique_ptr<T[]> m_buffer{nullptr};
std::size_t m_size{0u};
};
''')
import cppyy
print("Trying to construct objects:\n")
av_double = cppyy.gbl.arrayview('double')
av_float = cppyy.gbl.arrayview('float')
# ROOT 6.22.00 new PyROOT -- will invoke wrong ctor from std::complex<double>
# ROOT 6.22.00 legacy PyROOT -- will invoke correct ctor from size_t
print("Using arrayview<double>(5):")
av_double(5)
print()
# ROOT 6.22.00 new PyROOT -- will invoke correct ctor from size_t
# ROOT 6.22.00 legacy PyROOT -- will invoke correct ctor from size_t
print("Using arrayview<float>(5):")
av_float(5)
if __name__ == "__main__":
main()
The observed behavior is that wrong constructor overload, arrayview<double>::arrayview(std::complex<double>), is invoked instead of arrayview<double>::arrayview(size_t).
Meanwhile for the same call to arrayview<float> the correct constructor, arrayview<float>::arrayview(size_t), is invoked.
I’ve checked the legacy PyROOT with ROOT 6.22.00 compiled with -D pyroot_legacy=ON and ROOT 6.20.04; both choose the same arrayview<double>::arrayview(size_t) overload.
I also tried to reproduce such behavior in root interactive prompt, using the almost the same code from ctor_example.cpp (890 Bytes)
.L ctor_example.cpp;
arrayview<double> view(5); // calls correct overload for arrayview<double>::arrayview(size_t)
arrayview<float> flview(5); // calls correct overload for arrayview<float>::arrayview(size_t)
I’ve also tried compiling ctor_example.cpp (890 Bytes) with g++-9, g++-10, clang++-9 and observed consistent behavior: all compilers selected arrayview<double>::arrayview(size_t) overload instead of arrayview<double>::arrayview(std::complex) favored by new PyROOT.
Please, can anyone help me understand whether the observed behavior of the new PyROOT is correct?
_ROOT Versions:
- 6.22.00 built with new PyROOT
- 6.22.00 built with legacy PyROOT
- 6.20.04 built with legacy PyROOT
_Platform: Ubuntu 18.04 LTS
_Compiler: gcc-9/gcc-10/clang-9
_Python version: 3.6.9
All ROOTS are build with the following CMake flags: -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
Thank you for reporting this. The logic to do overload resolution has changed quite a bit in the new PyROOT. I will have a look at your reproducer and get back to you.
This indeed looks like a bug, since there is a priori no reason to have different behaviours in the instantiations of double and float, but I need to find out why it happens.
In the meantime, if this is blocking you in any way, you can use the following hack:
With this you would be forcing the constructor call to always pick the size_t overload. Again, it’s just a workaround in case you need it now. I’ll post here when I have more news.
Hi Enric,
thanks for quick reply!
I will check the way you suggested.
Also I found out that if I switch constructor from arrayview<double>::arrayview(size_t) --> arrayview<double>::arrayview(int) the problem goes away too.
It looks like somehow size_t causes the trouble.
I found a few other cases of problems with migration to new PyROOT. I will try to find minimal reproducers for them too.
Should I report them to this thread or to send them to you directly?
I can now report a more complete explanation of the behaviour. There are two key factors:
size_t is unsigned long, and when cppyy finds a parameter that is a long, it decreases the priority of the overload (because it wants to pick first a hypothetical overload which has an int). This is done here:
Even though there are two rounds of overload resolution (first round no implicit conversions are allowed, second round they are), cppyy has an explicit converter for complex<double>. The fact of having a converter makes the overload being valid in the first round (it is not treated as implicit conversion anymore):
If you combine the fact that the size_t overload has a decreased priority (which makes the complex overload being tried first) and the fact that there is a converter for complex<double> (which makes the complex overload succeed at first round), you end up with this behaviour.
complex<float> is a different case because it does not have a dedicated converter, so it can only succeed at second round, but by that time the size_t overload has already succeeded in first round.
If you use int instead of size_t, as you reported, the priority of the int overload is not decreased and therefore it is tried before the complex overload.
This looks surprising from the point of view of the user expectations, but there might be a reason for having an explicit converter for complex<double>, I’d need to check with the cppyy developers.