ffi: define compat for Py_NewRef and Py_XNewRef#4445
Merged
davidhewitt merged 5 commits intoPyO3:mainfrom Aug 17, 2024
Merged
Conversation
ngoldbaum
reviewed
Aug 16, 2024
Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
mejrs
approved these changes
Aug 16, 2024
Member
mejrs
left a comment
There was a problem hiding this comment.
Thanks!
I think we should add a test to ensure glob re-exports do not trigger import ambiguity:
mod test_export{
pub use pyo3_ffi::compat::*;
pub use pyo3_ffi::*;
}
Member
Author
|
Great idea. It turned out to be a little bit harder than just that pair of |
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 16, 2024
* ffi: define compat for `Py_NewRef` and `Py_XNewRef` * add missing inline hint Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * don't use std::ffi::c_int (requires MSRV 1.64) * add test to guard against ambiguity --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
davidhewitt
added a commit
that referenced
this pull request
Sep 3, 2024
* ffi: define compat for `Py_NewRef` and `Py_XNewRef` * add missing inline hint Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * don't use std::ffi::c_int (requires MSRV 1.64) * add test to guard against ambiguity * fix `Py_NewRef` cfg on PyPy --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
davidhewitt
added a commit
that referenced
this pull request
Sep 3, 2024
* ffi: define compat for `Py_NewRef` and `Py_XNewRef` * add missing inline hint Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * don't use std::ffi::c_int (requires MSRV 1.64) * add test to guard against ambiguity * fix `Py_NewRef` cfg on PyPy --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
davidhewitt
added a commit
that referenced
this pull request
Sep 15, 2024
* ffi: define compat for `Py_NewRef` and `Py_XNewRef` * add missing inline hint Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com> * don't use std::ffi::c_int (requires MSRV 1.64) * add test to guard against ambiguity * fix `Py_NewRef` cfg on PyPy --------- Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This improves the FFI definitions
Py_NewRefandPy_XNewRefin the following ways:#[doc(cfg(Py_3_10))]to the original definitions (which are either an extern symbol or an inline function depending on abi3 setting, and so would a cfg hint in the doc saying they were only available in one of these)Py_NewRefandPy_XNewReftopyo3_ffi::compat, so we can use them on all versions internally_Py_NewRefand_Py_XNewRefTo make the doc-cfgs on
pyo3_ffi::compatwork properly, I realised that I needed to use functions rather than re-exports when running thedocrsbuild. I decided to use a macro to make this implementation easier. This way thecompatfunctions are identical regardless of what Python version we build with. (cc @mejrs)This is what I saw before (clicking on these re-exports then shows a version cfg):
After: