-
Notifications
You must be signed in to change notification settings - Fork 831
chore: Fix 1.58 release #2100
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
chore: Fix 1.58 release #2100
Conversation
@@ -34,6 +34,7 @@ pub struct PyBuffer<T>(Pin<Box<ffi::Py_buffer>>, PhantomData<T>); | |||
|
|||
// PyBuffer is thread-safe: the shape of the buffer is immutable while a Py_buffer exists. | |||
// Accessing the buffer contents is protected using the GIL. | |||
#[allow(clippy::non_send_fields_in_send_ty)] |
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.
See rust-lang/rust-clippy#8045
We could also just make the fields Send
, because right now the only types that implement Element
are threadsafe already. However that would be a breaking change for anyone who has implemented Element
on their own types.
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.
Good question. Should we mark this with a FIXME or TODO to decide later?
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.
Actually that's not going to silence the lint, because it's triggering on ffi::Py_buffer
, and we can't make that Send. 🤔
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.
Ah ok. This is just one of the many cases where Python ffi makes clippy uneasy, we'll just have to live with it.
// #[cfg_attr(PyPy, link_name = "PyPyUnicode_FromFormatV")] | ||
// pub fn PyUnicode_FromFormatV(format: *const c_char, vargs: va_list) -> *mut PyObject; |
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 not sure what's up with this..it seems that all functions using the va_list
are commented out?
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 guess it's because support in std is not stable? We could enable it behind an optional flag.
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.
Ah, I had no idea that existed. Given that it's been on nightly for 3 years now and it doesn't look like it's close to stabilization, I think I prefer just not having these.
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 picking this up!
// #[cfg_attr(PyPy, link_name = "PyPyUnicode_FromFormatV")] | ||
// pub fn PyUnicode_FromFormatV(format: *const c_char, vargs: va_list) -> *mut PyObject; |
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 guess it's because support in std is not stable? We could enable it behind an optional flag.
@@ -34,6 +34,7 @@ pub struct PyBuffer<T>(Pin<Box<ffi::Py_buffer>>, PhantomData<T>); | |||
|
|||
// PyBuffer is thread-safe: the shape of the buffer is immutable while a Py_buffer exists. | |||
// Accessing the buffer contents is protected using the GIL. | |||
#[allow(clippy::non_send_fields_in_send_ty)] |
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.
Good question. Should we mark this with a FIXME or TODO to decide later?
No description provided.