Skip to content
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

[CPyCppyy] Remove implicit conversion of any ctypes ptr type to void* #16816

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

guitargeek
Copy link
Contributor

The IsCTypesArrayOrPointer gives false positives in Python 3.13, resulting the void pointer converter to take the wrong code path and crash. See:
wlav/cppyy#272

This code path is used for implicit conversion from other ctypes pointer types to void*, which is not strictly required. One can always do an explicit cast: ctypes.cast(my_ptr, ctypes.c_void_p ).

Given that this a niche feature that broke Python 3.13 support for functions taking void*, which is quite common, it can be argued that it's better to remove this implicit conversion.

This commit fixes the following tests under Python 3.13:

roottest-python-basic-datatype
roottest-python-cpp-cpp

This reverts the following commit from upstream:
wlav/CPyCppyy@80a0205

See also the Fedora 41 CI PR:
#16748

Copy link

github-actions bot commented Nov 4, 2024

Test Results

    18 files      18 suites   3d 18h 17m 11s ⏱️
 2 700 tests  2 605 ✅ 1 💤    94 ❌
46 147 runs  44 685 ✅ 0 💤 1 462 ❌

For more details on these failures, see this check.

Results for commit 5beb41b.

♻️ This comment has been updated with latest results.

Copy link
Member

@vepadulano vepadulano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and thanks for taking care of the synchronisation!

The `IsCTypesArrayOrPointer` gives false positives in Python 3.13,
resulting the void pointer converter to take the wrong code path and
crash. See:
wlav/cppyy#272

This code path is used for implicit conversion from other `ctypes`
pointer types to `void*`, which is not strictly required. One can
always do an explicit cast:  `ctypes.cast(my_ptr, ctypes.c_void_p )`.

Given that this a niche feature that broke Python 3.13 support for
functions taking `void*`, which is quite common, it can be argued that
it's better to remove this implicit conversion.

This commit fixes the following tests under Python 3.13:

```
roottest-python-basic-datatype
roottest-python-cpp-cpp
```

This reverts the following commit from upstream:
wlav/CPyCppyy@80a0205
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants