root-project / root

The official repository for ROOT: analyzing, storing and visualizing big data, scientifically
https://root.cern
Other
2.69k stars 1.28k forks source link

[PyROOT] The cppyy version inside ROOT doesn't support `long long` #15872

Open guitargeek opened 4 months ago

guitargeek commented 4 months ago

Description

After removing a patch from the cppyy inside ROOT in 8d2a6df6d, the built-in cppyy somehow doesn't support long long anymore. This only became apparent now that the Long64_t converter is only registered when setting up PyROOT.

Note that the reproducer works in upstream cppyy. Maybe because the type name "long long" is two words, and upstream cppyy somehow fixed it by patching ROOT meta?

It's not an urgent problem, because it's easy to fix by importing ROOT, but it would be nice to understand what is going on. Because maybe it means that once this problem with long long is fixed, we don't need the converter aliases for PyROOT at all!

Reproducer

import cppyy

# import ROOT # setting up PyROOT would fix the problem
# ROOT.gInterpreter

cppyy.cppdef("""
#include <cstdint>
namespace cool {
typedef long long Bad_t;
typedef std::int64_t Goot_t;
const Bad_t bad_value = +9223372036854775807;
const Goot_t good_value = +9223372036854775807;
}
""")

print(cppyy.gbl.cool.bad_value)
print(cppyy.gbl.cool.good_value)

Will crash with:

Traceback (most recent call last):
  File "/home/rembserj/repro.py", line 16, in <module>
    print(cppyy.gbl.cool.bad_value)
          ^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: <namespace cppyy.gbl.cool at 0x64765cfce570> has no attribute 'bad_value'. Full details:
  type object 'cool' has no attribute 'bad_value'
  'cool::bad_value' is not a known C++ class
  C++ type cannot be converted from memory. Did you mean: 'good_value'?

ROOT version

ROOT master.

hahnjo commented 4 months ago

FWIW it's really only the Long64_t converter:

diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx
index 1083c92452..6ba86a5d24 100644
--- a/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx
+++ b/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx
@@ -3470,6 +3470,7 @@ public:
         gf["const " CCOMPLEX_D "&"] =       gf["const std::complex<double>&"];
         gf[CCOMPLEX_F " ptr"] =             gf["std::complex<float> ptr"];
         gf[CCOMPLEX_D " ptr"] =             gf["std::complex<double> ptr"];
+        gf["Long64_t"] =                    gf["long long"];

     // factories for special cases
         gf["TString"] =                     (cf_t)+[](cdims_t) { return new TStringConverter{}; };

... fixes the problem. Which is weird because the reproducer doesn't use that type and gf / gConvFactories is only used for lookup, but not registered anywhere.

vepadulano commented 4 months ago

After investigation, it turns out that the long long type is automatically converted to Long64_t. The stacktrace follows

#0  ROOT::TMetaUtils::TClingLookupHelper::GetPartiallyDesugaredNameWithScopeHandling (this=0x5555560e2b70, tname="cool::Bad_t", result="", dropstd=false)
    at /home/vpadulan/Programs/rootproject/rootsrc/core/clingutils/src/TClingUtils.cxx:607
#1  0x00007fffe984a3f6 in ResolveTypedefProcessType (tname=0x5555593a7a40 "const cool::Bad_t", cursor=17, constprefix=true, start_of_type=6, end_of_type=0, 
    mod_start_of_type=0, modified=@0x7fffffffc45b: false, result="") at /home/vpadulan/Programs/rootproject/rootsrc/core/foundation/src/TClassEdit.cxx:1477
#2  0x00007fffe984be6f in ResolveTypedefImpl (tname=0x5555593a7a40 "const cool::Bad_t", len=17, cursor=@0x7fffffffc45c: 17, modified=@0x7fffffffc45b: false, 
    result="") at /home/vpadulan/Programs/rootproject/rootsrc/core/foundation/src/TClassEdit.cxx:1741
#3  0x00007fffe984c37d in TClassEdit::ResolveTypedef[abi:cxx11](char const*, bool) (tname=0x5555593a7a40 "const cool::Bad_t")
    at /home/vpadulan/Programs/rootproject/rootsrc/core/foundation/src/TClassEdit.cxx:1775
#4  0x00007fffe9c7a37e in Cppyy::ResolveName (cppitem_name="const cool::Bad_t")
    at /home/vpadulan/Programs/rootproject/rootsrc/bindings/pyroot/cppyy/cppyy-backend/clingwrapper/src/clingwrapper.cxx:461
#5  0x00007fffda525a40 in CPyCppyy::CreateConverter (fullType="const cool::Bad_t", dims=...)
    at /home/vpadulan/Programs/rootproject/rootsrc/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx:3111
#6  0x00007fffda56f992 in CPyCppyy::CPPDataMember::Set (this=0x7fffda0ef870, scope=8, idata=0)
    at /home/vpadulan/Programs/rootproject/rootsrc/bindings/pyroot/cppyy/CPyCppyy/src/CPPDataMember.cxx:363
#7  0x00007fffda5907a2 in CPyCppyy::CPPDataMember_New (scope=8, idata=0)
    at /home/vpadulan/Programs/rootproject/rootsrc/bindings/pyroot/cppyy/CPyCppyy/src/CPPDataMember.h:63

Specifically in getAsStringInternal

644          dest.getAsStringInternal(result, policy);
(gdb) p result
$15 = ""
(gdb) n
646          unsigned long offset = 0;
(gdb) p result
$16 = "Long64_t"

None of this happens in upstream cppyy where we see

Breakpoint 3, Cppyy::ResolveName (cppitem_name="const cool::Bad_t") at src/clingwrapper.cxx:435
435 {
(gdb) finish
Run till exit from #0  Cppyy::ResolveName (cppitem_name="const cool::Bad_t") at src/clingwrapper.cxx:435
CPyCppyy::CreateConverter (fullType="const cool::Bad_t", dims=...) at /home/vpadulan/Programs/cppyyproject/CPyCppyy/src/Converters.cxx:3112
3112        if (resolvedType != fullType) {
Value returned is $40 = "const long long"

And the input type Bad_t is properly converted to the typedef long long.

This then appears to be a bug in ROOT meta

pcanal commented 4 months ago

it turns out that the long long type is automatically converted to Long64_t.

This is intentional.

vepadulano commented 4 months ago

This is intentional.

I was quite sure about it, it is still the culprit of the reported bug.