Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add #[track_caller] to all Py/Bound/Borrowed methods which panic #4098

Merged
merged 1 commit into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/4098.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `#[track_caller]` to all `Py<T>`, `Bound<'py, T>` and `Borrowed<'a, 'py, T>` methods which can panic.
1 change: 1 addition & 0 deletions src/err/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,6 +1091,7 @@ fn display_downcast_error(
)
}

#[track_caller]
pub fn panic_after_error(_py: Python<'_>) -> ! {
unsafe {
ffi::PyErr_Print();
Expand Down
2 changes: 2 additions & 0 deletions src/ffi_ptr_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ impl FfiPtrExt for *mut ffi::PyObject {
}

#[inline]
#[track_caller]
unsafe fn assume_owned(self, py: Python<'_>) -> Bound<'_, PyAny> {
Bound::from_owned_ptr(py, self)
}
Expand All @@ -57,6 +58,7 @@ impl FfiPtrExt for *mut ffi::PyObject {
}

#[inline]
#[track_caller]
unsafe fn assume_borrowed<'a>(self, py: Python<'_>) -> Borrowed<'a, '_, PyAny> {
Borrowed::from_ptr(py, self)
}
Expand Down
9 changes: 9 additions & 0 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ impl<'py> Bound<'py, PyAny> {
/// - `ptr` must be a valid pointer to a Python object
/// - `ptr` must be an owned Python reference, as the `Bound<'py, PyAny>` will assume ownership
#[inline]
#[track_caller]
pub unsafe fn from_owned_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self {
Self(py, ManuallyDrop::new(Py::from_owned_ptr(py, ptr)))
}
Expand Down Expand Up @@ -141,6 +142,7 @@ impl<'py> Bound<'py, PyAny> {
///
/// - `ptr` must be a valid pointer to a Python object
#[inline]
#[track_caller]
pub unsafe fn from_borrowed_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self {
Self(py, ManuallyDrop::new(Py::from_borrowed_ptr(py, ptr)))
}
Expand Down Expand Up @@ -242,6 +244,7 @@ where
/// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
/// [`try_borrow`](#method.try_borrow).
#[inline]
#[track_caller]
pub fn borrow(&self) -> PyRef<'py, T> {
PyRef::borrow(self)
}
Expand Down Expand Up @@ -276,6 +279,7 @@ where
/// Panics if the value is currently borrowed. For a non-panicking variant, use
/// [`try_borrow_mut`](#method.try_borrow_mut).
#[inline]
#[track_caller]
pub fn borrow_mut(&self) -> PyRefMut<'py, T>
where
T: PyClass<Frozen = False>,
Expand Down Expand Up @@ -573,6 +577,7 @@ impl<'a, 'py> Borrowed<'a, 'py, PyAny> {
/// the caller and it is the caller's responsibility to ensure that the reference this is
/// derived from is valid for the lifetime `'a`.
#[inline]
#[track_caller]
pub unsafe fn from_ptr(py: Python<'py>, ptr: *mut ffi::PyObject) -> Self {
Self(
NonNull::new(ptr).unwrap_or_else(|| crate::err::panic_after_error(py)),
Expand Down Expand Up @@ -1138,6 +1143,7 @@ where
/// Panics if the value is currently mutably borrowed. For a non-panicking variant, use
/// [`try_borrow`](#method.try_borrow).
#[inline]
#[track_caller]
pub fn borrow<'py>(&'py self, py: Python<'py>) -> PyRef<'py, T> {
self.bind(py).borrow()
}
Expand Down Expand Up @@ -1175,6 +1181,7 @@ where
/// Panics if the value is currently borrowed. For a non-panicking variant, use
/// [`try_borrow_mut`](#method.try_borrow_mut).
#[inline]
#[track_caller]
pub fn borrow_mut<'py>(&'py self, py: Python<'py>) -> PyRefMut<'py, T>
where
T: PyClass<Frozen = False>,
Expand Down Expand Up @@ -1585,6 +1592,7 @@ impl<T> Py<T> {
/// # Panics
/// Panics if `ptr` is null.
#[inline]
#[track_caller]
pub unsafe fn from_owned_ptr(py: Python<'_>, ptr: *mut ffi::PyObject) -> Py<T> {
match NonNull::new(ptr) {
Some(nonnull_ptr) => Py(nonnull_ptr, PhantomData),
Expand Down Expand Up @@ -1628,6 +1636,7 @@ impl<T> Py<T> {
/// # Panics
/// Panics if `ptr` is null.
#[inline]
#[track_caller]
pub unsafe fn from_borrowed_ptr(py: Python<'_>, ptr: *mut ffi::PyObject) -> Py<T> {
match Self::from_borrowed_ptr_or_opt(py, ptr) {
Some(slf) => slf,
Expand Down
2 changes: 2 additions & 0 deletions src/pycell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ impl<'py, T: PyClass> PyRef<'py, T> {
self.inner.clone().into_ptr()
}

#[track_caller]
pub(crate) fn borrow(obj: &Bound<'py, T>) -> Self {
Self::try_borrow(obj).expect("Already mutably borrowed")
}
Expand Down Expand Up @@ -848,6 +849,7 @@ impl<'py, T: PyClass<Frozen = False>> PyRefMut<'py, T> {
}

#[inline]
#[track_caller]
pub(crate) fn borrow(obj: &Bound<'py, T>) -> Self {
Self::try_borrow(obj).expect("Already borrowed")
}
Expand Down
Loading