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

Expand on the C api definitions #2409

Merged
merged 14 commits into from
Jun 4, 2022
Merged

Expand on the C api definitions #2409

merged 14 commits into from
Jun 4, 2022

Conversation

mejrs
Copy link
Member

@mejrs mejrs commented May 29, 2022

I'm not entirely sure whether I got the cfg's right. When do I need to add (or not add) a PyPy gate?

@mejrs mejrs added the CI-no-fail-fast If one job fails, allow the rest to keep testing label May 29, 2022
@mejrs mejrs force-pushed the capi branch 2 times, most recently from 12366d0 to 76f4494 Compare May 29, 2022 20:08
@davidhewitt
Copy link
Member

Nice, these changes look along the right lines to me! It would be nice to add some tests for the inlined functions in methodobject.rs

I'm not entirely sure whether I got the cfg's right. When do I need to add (or not add) a PyPy gate?

TBH I think struct definitions should be checked carefully whether they're PyPy compatible, because we could get UB/crashes if they're wrong. For functions, the worst that happens is that we get a linker error if there's a PyPy gate missing. So I tend to be a bit lax with PyPy gates on functions, though in principle they all could have either a not(PyPy) or a link_name = "PyPy..." attribute.

mejrs and others added 4 commits June 4, 2022 10:57
Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
@mejrs mejrs force-pushed the capi branch 3 times, most recently from d3d29c2 to 5da3b97 Compare June 4, 2022 15:08
@mejrs
Copy link
Member Author

mejrs commented Jun 4, 2022

I think that's all. I can't quite figure out how to squash them though. I suppose we can "squash and merge"?

@davidhewitt
Copy link
Member

davidhewitt commented Jun 4, 2022

👍 looks good to me, I'll click squash & merge. Thanks! (Hopefully with this PR pyo3-ffi-check will now be happy for 3.11.)

@davidhewitt davidhewitt merged commit 728d35a into PyO3:main Jun 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-no-fail-fast If one job fails, allow the rest to keep testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants