Add c_variadic bindings to pyo3-ffi#5789
Conversation
| @@ -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.
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.
I'm now confused, isn't this a variadic function declared on stable Rust? 😖
There was a problem hiding this comment.
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.
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?
There was a problem hiding this comment.
Looks like you are correct, so I think we should unconditionally skip all FFI declarations taking VaList for now.
It looks like this MR cleans up some names on the variadic ... functions but doesn't add new ones. I'm open to proceeding with just those name cleanups, I guess we don't need the nightly feature stuff now.
(I wonder if we should add a Contributing.md inside pyo3-ffi with notes for ourselves on these kinds of details about what we do / do not support?)
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Circling back here, this is confusion on my part:
- Declaring FFI variadic functions taking
...are already stable, indeed we have a bunch of those already defined. - The nightly feature is about defining new variadic extern "C" functions.
... I wonder if we should use PyArg_ParseTupleAndKeywords in pyo3-ffi/examples/string-sum?
Sorry for spending your time on my error!
| 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.
Looks like you are correct, so I think we should unconditionally skip all FFI declarations taking VaList for now.
It looks like this MR cleans up some names on the variadic ... functions but doesn't add new ones. I'm open to proceeding with just those name cleanups, I guess we don't need the nightly feature stuff now.
(I wonder if we should add a Contributing.md inside pyo3-ffi with notes for ourselves on these kinds of details about what we do / do not support?)
2 participants
closes #5777