-
Notifications
You must be signed in to change notification settings - Fork 782
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
allow Bound<'_, T>
in #[pymethods] self
position
#3896
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 catching this one! I have a few small thoughts...
pyo3-macros-backend/src/method.rs
Outdated
@@ -207,12 +207,11 @@ impl SelfType { | |||
SelfType::TryFromPyCell(span) => { |
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.
It might be nice to rename this TryFromPyCell
variant to TryFromBoundRef
.
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 idea!
impl<'a, 'py, T: PyClass> TryFrom<BoundRef<'a, 'py, T>> for PyRef<'py, T> { | ||
type Error = PyBorrowError; | ||
#[inline] | ||
fn try_from(value: BoundRef<'a, 'py, T>) -> Result<Self, Self::Error> { | ||
value.0.clone().into_gil_ref().try_into() | ||
} | ||
} |
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 .into_gil_ref()
use here and below seems correct for now (and gets cleaned up after #3860) 👍
src/impl_/pymethods.rs
Outdated
@@ -490,6 +492,12 @@ impl<'a, 'py> BoundRef<'a, 'py, PyAny> { | |||
Bound::ref_from_ptr_or_opt(py, ptr).as_ref().map(BoundRef) | |||
} | |||
|
|||
pub unsafe fn downcast<T: PyTypeCheck>( |
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'd think this one is actually safe, given it's checked?
pub unsafe fn downcast<T: PyTypeCheck>( | |
pub fn downcast<T: PyTypeCheck>( |
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.
Your completely right, one of those copy pasta errors 😆 . I'll fix it
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.
Perfect, thanks! 👍
Part of #3684
This allows extracting
&Bound<'_, T>
andBound<'_, T>
in#[pymethods]
self
position. This further discourages use ofPyCell
and makesBound<'_, T>
the primary type for interacting with Python objects.