-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix IsEnum
check in cppyy/metacling
#17775
base: master
Are you sure you want to change the base?
Conversation
This is the first step towards performing pure clang based reflection in cppyy, bypassing the limitations of the type system.
ef687d8
to
39a2782
Compare
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 59ef22e. ♻️ This comment has been updated with latest results. |
Note that by relying on CppInterOp will not allow to backport to 6.32 and 6.34 releases... |
39a2782
to
59ef22e
Compare
@@ -1078,9 +1081,13 @@ bool Cppyy::IsAbstract(TCppType_t klass) | |||
bool Cppyy::IsEnum(const std::string& type_name) | |||
{ | |||
if (type_name.empty()) return false; | |||
std::string tn_short = TClassEdit::ShortType(type_name.c_str(), 1); | |||
if (tn_short.empty()) return false; | |||
return gInterpreter->ClassInfo_IsEnum(tn_short.c_str()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: this solve the problem 'only' for Python. It would be even better to fix TCling::ClassInfo_IsEnum
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! The initial fix was in TCling(https://github.com/root-project/root/actions/runs/13424841558), that caused a certain set of test failures. I was curious to know which of them happened only via the python interface leading me to temporarily patch clingwrapper instead
Fixes #10454, and the following reproducer no longer seg faults:
This PR also enables InterOp API in clingwrapper, which is the first step towards performing clang-based reflection in cppyy, bypassing the limitations of the type system.
I am currently testing in TCling whether this is ready to use for
ClassInfo_IsEnum
(expecting some failures). Ideally we would patchCppyy::IsEnum
to fix this issue, which I will test in a following commit after the CI run, hence the changes in clingwrapper's cmake.cc @vepadulano