Fix failing python enums with LLVM18

[devajithvs]

While working on an update to LLVM 18 for Cling, I found an issue with the following code snippet:

import cppyy

cppyy.cppdef("""\
enum {
    kSingleKey     = 1,
    kOverwrite     = 2,
    kWriteDelete   = 4,
}; """)

print(cppyy.gbl.kSingleKey)

This would print garbage values instead of 1 (after the cling update to LLVM18)

This is due to a fix in LLVM 18 that fixes a memory leak 142f270. We relied on this memory leak to provide the offset values, but this no longer works after the update.

A draft PR in ROOT to test for performance issues and failing tests: PR #16000.

There is a temporary fix has been implemented to address the failing tests (in ROOT) for LLVM 18 by reintroducing the memory leak: de5d141. But this is better fixed in cppyy.

[hahnjo]

@guitargeek @wlav any updates on this? This is one of the remaining blockers for the LLVM 18 upgrade in ROOT, and I hope we can avoid it getting on the critical path...

[wlav]

Can look into it, but the proposed fix isn't very on point:

• Fixing it only in the client code means that the ROOT/meta API remains broken
• As proposed, it affects all lookups, including static/global data, even as the call is very inefficient
• Putting the function call inside a loop (!!) for the data member cache is kind of out there, efficiency-wise, but also if the returned value is not fixed anymore, then the cached lookup based on the offset will fail (or worse, succeed against an incorrect address)
• Enum lookups in ROOT/meta have massive side-effects (incl. potential parsing of headers and deserializing of modules), so even if the extra lookups are only flagged for enums, it would still be problematic
• The patch contains a removal of unrelated code

Since enums are constant, a proper solution could be to cache their value (e.g. the PyObject wrapper, once constructed) on the client side; then make the change only for enum constants and we'll get somewhere.

[devajithvs]

Thanks for the pointers @wlav

This will be fixed in ROOT Meta with https://github.com/root-project/root/pull/15696/commits/2c2d4dc2bf3ebe0f475b05a66a377c2467525d9d

[wlav]

I don't think that's a good solution: an enum's type can be specified and could be long long. I'll also note that the use of long for caching the value has portability issues as it's 8 bytes on Linux/Mac and 4 bytes on Windows. Doesn't matter for me, as I have my own fork of ROOT/meta, so I'll do my own thing, but it may affect others.

[hahnjo]

I don't think that's a good solution: an enum's type can be specified and could be long long. I'll also note that the use of long for caching the value has portability issues as it's 8 bytes on Linux/Mac and 4 bytes on Windows. Doesn't matter for me, as I have my own fork of ROOT/meta, so I'll do my own thing, but it may affect others.

Yes, maybe we should store it as int64_t (I was confused because Longptr_t doesn't actually mean pointer to long). On the other hand, it all depends on the code that actually dereferences the pointer, and based on the comments it sounds like this is a long for enums.

[hahnjo]

I think this can be closed.