[cppyy] Add converters and low-level views for fixed width integers#18492
[cppyy] Add converters and low-level views for fixed width integers#18492aaronj0 merged 2 commits intoroot-project:masterfrom
Conversation
Test Results 19 files 19 suites 4d 16h 39m 46s ⏱️ Results for commit 7b1108b. ♻️ This comment has been updated with latest results. |
9f88d46 to
57c2724
Compare
guitargeek
left a comment
There was a problem hiding this comment.
Thank you very much for implementing better support for fixed-width integers! Looks good to me, but please also add a test 🙂
|
Oh and don't forget to fix also the compiler warnings, otherwise the |
d051d60 to
10be20c
Compare
|
The following issue: #9809 causes failures on windows x86, where the arrays assigned on the C++ side itself are indexed/read wrongly: root [0] int test[]={2,4,6,8,10};
root [1] test[4]
(int) 2
root [2] test[4]=20;
root [3] test[4]
(int) 20
root [4] test[0]
(int) 20
root [5] test
(int [5]) { 20, 4, 6, 8, 10 }
root [6]Hence validating those buffers fail and the test will have to be disabled on win x86. |
|
Looks like the failures on win 64 are unrelated: |
fixes: #17841
The support for the fixed-width types
int16_tandint32_tindeed diverges from upstream cppyy and usesunsigned longwhich triggers the incorrect numpy view. Despite not having converters for the same, upstream relies on patches (or older behaviour) along theCppyy::ResolveName -> TClassEdit::ResolveTypedefcode path (see ResolveTypeDefImpl) that resolved fixed width integer types to their standard type counterparts (int16_t[][]toshort[][]andint32_t[][]toint[][]) when creating converters. Trying to follow the same code branch in ROOT can have a large blast radius in the type system. Two non-intrusive ways to fix this issue are either aliasing to existing short and int converters, or adding the converters and low-level views for both types. This PR performs the latter