-
Notifications
You must be signed in to change notification settings - Fork 937
Add c_variadic bindings to pyo3-ffi
#5789
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks for implementing this!
| @@ -139,7 +139,7 @@ chrono-local = ["chrono/clock", "dep:iana-time-zone"] | |||
|
|
|||
|
|
|||
| # Optimizes PyObject to Vec conversion and so on. | |||
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.
This comment is now well out of date, let's re-write this to just link to https://pyo3.rs/latest/features#nightly
At the same time, please can we update the description of the nightly feature in features.md to include this? It is also possible there are other features in nightly missing from that description, I am not sure either way.
| format: *const c_char, | ||
| #[cfg(not(Py_3_13))] keywords: *mut *mut c_char, | ||
| #[cfg(Py_3_13)] keywords: *const *const c_char, | ||
| ... |
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.
I'm now confused, isn't this a variadic function declared on stable Rust? 😖
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.
I'm confused as well, but there is a clear difference in the signature. I'm not 100% fluent on this part of C I'm afraid.
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.
There are currently no plans for an api to create VaList's on the rust side. So these bindings are not useful without some c code calling into rust. That in mind, do we still want to move forward with this MR?
closes #5777