Merge pull request #3763 from Icxolu/slice-traceback

implement `PyTracebackMethods` and `PySliceMethods`

guide/src/python_from_rust.md

@@ -479,7 +479,7 @@ class House(object):
             }
             Err(e) => {
                 house
-                    .call_method1("__exit__", (e.get_type(py), e.value(py), e.traceback(py)))
+                    .call_method1("__exit__", (e.get_type(py), e.value(py), e.traceback_bound(py)))
                     .unwrap();
             }
         }

src/err/err_state.rs

@@ -2,7 +2,7 @@ use crate::{
     exceptions::{PyBaseException, PyTypeError},
     ffi,
     types::{PyTraceback, PyType},
-    IntoPy, Py, PyAny, PyObject, PyTypeInfo, Python,
+    Bound, IntoPy, Py, PyAny, PyObject, PyTypeInfo, Python,
 };
 
 #[derive(Clone)]
@@ -26,15 +26,21 @@ impl PyErrStateNormalized {
     }
 
     #[cfg(not(Py_3_12))]
-    pub(crate) fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
+    pub(crate) fn ptraceback<'py>(&self, py: Python<'py>) -> Option<Bound<'py, PyTraceback>> {
         self.ptraceback
             .as_ref()
-            .map(|traceback| traceback.as_ref(py))
+            .map(|traceback| traceback.bind(py).clone())
     }
 
     #[cfg(Py_3_12)]
-    pub(crate) fn ptraceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
+    pub(crate) fn ptraceback<'py>(&self, py: Python<'py>) -> Option<Bound<'py, PyTraceback>> {
-        unsafe { py.from_owned_ptr_or_opt(ffi::PyException_GetTraceback(self.pvalue.as_ptr())) }
+        use crate::ffi_ptr_ext::FfiPtrExt;
+        use crate::types::any::PyAnyMethods;
+        unsafe {
+            ffi::PyException_GetTraceback(self.pvalue.as_ptr())
+                .assume_owned_or_opt(py)
+                .map(|b| b.downcast_into_unchecked())
+        }
     }
 
     #[cfg(Py_3_12)]

src/err/mod.rs

@@ -272,6 +272,18 @@ impl PyErr {
         exc
     }
 
+    /// Deprecated form of [`PyErr::traceback_bound`].
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PyErr::traceback` will be replaced by `PyErr::traceback_bound` in a future PyO3 version"
+        )
+    )]
+    pub fn traceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
+        self.normalized(py).ptraceback(py).map(|b| b.into_gil_ref())
+    }
+
     /// Returns the traceback of this exception object.
     ///
     /// # Examples
@@ -280,10 +292,10 @@ impl PyErr {
     ///
     /// Python::with_gil(|py| {
     ///     let err = PyTypeError::new_err(("some type error",));
-    ///     assert!(err.traceback(py).is_none());
+    ///     assert!(err.traceback_bound(py).is_none());
     /// });
     /// ```
-    pub fn traceback<'py>(&'py self, py: Python<'py>) -> Option<&'py PyTraceback> {
+    pub fn traceback_bound<'py>(&self, py: Python<'py>) -> Option<Bound<'py, PyTraceback>> {
         self.normalized(py).ptraceback(py)
     }
 
@@ -476,10 +488,16 @@ impl PyErr {
 
         #[cfg(not(Py_3_12))]
         unsafe {
+            // keep the bound `traceback` alive for entire duration of
+            // PyErr_Display. if we inline this, the `Bound` will be dropped
+            // after the argument got evaluated, leading to call with a dangling
+            // pointer.
+            let traceback = self.traceback_bound(py);
             ffi::PyErr_Display(
                 self.get_type(py).as_ptr(),
                 self.value(py).as_ptr(),
-                self.traceback(py)
+                traceback
+                    .as_ref()
                     .map_or(std::ptr::null_mut(), |traceback| traceback.as_ptr()),
             )
         }
@@ -658,15 +676,15 @@ impl PyErr {
     ///
     /// # Examples
     /// ```rust
-    /// use pyo3::{exceptions::PyTypeError, PyErr, Python};
+    /// use pyo3::{exceptions::PyTypeError, PyErr, Python, prelude::PyAnyMethods};
     /// Python::with_gil(|py| {
     ///     let err: PyErr = PyTypeError::new_err(("some type error",));
     ///     let err_clone = err.clone_ref(py);
     ///     assert!(err.get_type(py).is(err_clone.get_type(py)));
     ///     assert!(err.value(py).is(err_clone.value(py)));
-    ///     match err.traceback(py) {
+    ///     match err.traceback_bound(py) {
-    ///         None => assert!(err_clone.traceback(py).is_none()),
+    ///         None => assert!(err_clone.traceback_bound(py).is_none()),
-    ///         Some(tb) => assert!(err_clone.traceback(py).unwrap().is(tb)),
+    ///         Some(tb) => assert!(err_clone.traceback_bound(py).unwrap().is(&tb)),
     ///     }
     /// });
     /// ```
@@ -747,7 +765,7 @@ impl std::fmt::Debug for PyErr {
             f.debug_struct("PyErr")
                 .field("type", self.get_type(py))
                 .field("value", self.value(py))
-                .field("traceback", &self.traceback(py))
+                .field("traceback", &self.traceback_bound(py))
                 .finish()
         })
     }

src/exceptions.rs

@@ -100,6 +100,7 @@ macro_rules! import_exception {
         impl $name {
             fn type_object_raw(py: $crate::Python<'_>) -> *mut $crate::ffi::PyTypeObject {
                 use $crate::sync::GILOnceCell;
+                use $crate::prelude::PyTracebackMethods;
                 static TYPE_OBJECT: GILOnceCell<$crate::Py<$crate::types::PyType>> =
                     GILOnceCell::new();
 
@@ -109,7 +110,7 @@ macro_rules! import_exception {
                             .import(stringify!($module))
                             .unwrap_or_else(|err| {
                                 let traceback = err
-                                    .traceback(py)
+                                    .traceback_bound(py)
                                     .map(|tb| tb.format().expect("raised exception will have a traceback"))
                                     .unwrap_or_default();
                                 ::std::panic!("Can not import module {}: {}\n{}", stringify!($module), err, traceback);

src/prelude.rs

@@ -38,4 +38,5 @@ pub use crate::types::module::PyModuleMethods;
 pub use crate::types::sequence::PySequenceMethods;
 pub use crate::types::set::PySetMethods;
 pub use crate::types::string::PyStringMethods;
+pub use crate::types::traceback::PyTracebackMethods;
 pub use crate::types::tuple::PyTupleMethods;

src/types/mod.rs

@@ -304,6 +304,6 @@ pub(crate) mod sequence;
 pub(crate) mod set;
 mod slice;
 pub(crate) mod string;
-mod traceback;
+pub(crate) mod traceback;
 pub(crate) mod tuple;
 mod typeobject;

src/types/slice.rs

@@ -1,6 +1,8 @@
 use crate::err::{PyErr, PyResult};
 use crate::ffi::{self, Py_ssize_t};
-use crate::{PyAny, PyObject, Python, ToPyObject};
+use crate::ffi_ptr_ext::FfiPtrExt;
+use crate::types::any::PyAnyMethods;
+use crate::{Bound, PyAny, PyNativeType, PyObject, Python, ToPyObject};
 use std::os::raw::c_long;
 
 /// Represents a Python `slice`.
@@ -42,23 +44,52 @@ impl PySliceIndices {
 }
 
 impl PySlice {
-    /// Constructs a new slice with the given elements.
+    /// Deprecated form of `PySlice::new_bound`.
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PySlice::new` will be replaced by `PySlice::new_bound` in a future PyO3 version"
+        )
+    )]
     pub fn new(py: Python<'_>, start: isize, stop: isize, step: isize) -> &PySlice {
+        Self::new_bound(py, start, stop, step).into_gil_ref()
+    }
+
+    /// Constructs a new slice with the given elements.
+    pub fn new_bound(py: Python<'_>, start: isize, stop: isize, step: isize) -> Bound<'_, PySlice> {
         unsafe {
-            let ptr = ffi::PySlice_New(
+            ffi::PySlice_New(
                 ffi::PyLong_FromSsize_t(start),
                 ffi::PyLong_FromSsize_t(stop),
                 ffi::PyLong_FromSsize_t(step),
-            );
+            )
+            .assume_owned(py)
+            .downcast_into_unchecked()
+        }
+    }
+
+    /// Deprecated form of `PySlice::full_bound`.
+    #[cfg_attr(
+        not(feature = "gil-refs"),
+        deprecated(
+            since = "0.21.0",
+            note = "`PySlice::full` will be replaced by `PySlice::full_bound` in a future PyO3 version"
+        )
+    )]
+    pub fn full(py: Python<'_>) -> &PySlice {
+        unsafe {
+            let ptr = ffi::PySlice_New(ffi::Py_None(), ffi::Py_None(), ffi::Py_None());
             py.from_owned_ptr(ptr)
         }
     }
 
     /// Constructs a new full slice that is equivalent to `::`.
-    pub fn full(py: Python<'_>) -> &PySlice {
+    pub fn full_bound(py: Python<'_>) -> Bound<'_, PySlice> {
         unsafe {
-            let ptr = ffi::PySlice_New(ffi::Py_None(), ffi::Py_None(), ffi::Py_None());
+            ffi::PySlice_New(ffi::Py_None(), ffi::Py_None(), ffi::Py_None())
-            py.from_owned_ptr(ptr)
+                .assume_owned(py)
+                .downcast_into_unchecked()
         }
     }
 
@@ -67,6 +98,25 @@ impl PySlice {
     /// slice in its `slicelength` member.
     #[inline]
     pub fn indices(&self, length: c_long) -> PyResult<PySliceIndices> {
+        self.as_borrowed().indices(length)
+    }
+}
+
+/// Implementation of functionality for [`PySlice`].
+///
+/// These methods are defined for the `Bound<'py, PyTuple>` smart pointer, so to use method call
+/// syntax these methods are separated into a trait, because stable Rust does not yet support
+/// `arbitrary_self_types`.
+#[doc(alias = "PySlice")]
+pub trait PySliceMethods<'py> {
+    /// Retrieves the start, stop, and step indices from the slice object,
+    /// assuming a sequence of length `length`, and stores the length of the
+    /// slice in its `slicelength` member.
+    fn indices(&self, length: c_long) -> PyResult<PySliceIndices>;
+}
+
+impl<'py> PySliceMethods<'py> for Bound<'py, PySlice> {
+    fn indices(&self, length: c_long) -> PyResult<PySliceIndices> {
         // non-negative Py_ssize_t should always fit into Rust usize
         unsafe {
             let mut slicelength: isize = 0;
@@ -97,7 +147,7 @@ impl PySlice {
 
 impl ToPyObject for PySliceIndices {
     fn to_object(&self, py: Python<'_>) -> PyObject {
-        PySlice::new(py, self.start, self.stop, self.step).into()
+        PySlice::new_bound(py, self.start, self.stop, self.step).into()
     }
 }
 
@@ -108,7 +158,7 @@ mod tests {
     #[test]
     fn test_py_slice_new() {
         Python::with_gil(|py| {
-            let slice = PySlice::new(py, isize::MIN, isize::MAX, 1);
+            let slice = PySlice::new_bound(py, isize::MIN, isize::MAX, 1);
             assert_eq!(
                 slice.getattr("start").unwrap().extract::<isize>().unwrap(),
                 isize::MIN
@@ -127,7 +177,7 @@ mod tests {
     #[test]
     fn test_py_slice_full() {
         Python::with_gil(|py| {
-            let slice = PySlice::full(py);
+            let slice = PySlice::full_bound(py);
             assert!(slice.getattr("start").unwrap().is_none(),);
             assert!(slice.getattr("stop").unwrap().is_none(),);
             assert!(slice.getattr("step").unwrap().is_none(),);

src/types/traceback.rs

@@ -1,7 +1,7 @@
 use crate::err::{error_on_minusone, PyResult};
-use crate::ffi;
 use crate::types::PyString;
-use crate::PyAny;
+use crate::{ffi, Bound};
+use crate::{PyAny, PyNativeType};
 
 /// Represents a Python traceback.
 #[repr(transparent)]
@@ -24,14 +24,14 @@ impl PyTraceback {
     /// The following code formats a Python traceback and exception pair from Rust:
     ///
     /// ```rust
-    /// # use pyo3::{Python, PyResult};
+    /// # use pyo3::{Python, PyResult, prelude::PyTracebackMethods};
     /// # let result: PyResult<()> =
     /// Python::with_gil(|py| {
     ///     let err = py
     ///         .run("raise Exception('banana')", None, None)
     ///         .expect_err("raise will create a Python error");
     ///
-    ///     let traceback = err.traceback(py).expect("raised exception will have a traceback");
+    ///     let traceback = err.traceback_bound(py).expect("raised exception will have a traceback");
     ///     assert_eq!(
     ///         format!("{}{}", traceback.format()?, err),
     ///         "\
@@ -46,6 +46,53 @@ impl PyTraceback {
     /// # result.expect("example failed");
     /// ```
     pub fn format(&self) -> PyResult<String> {
+        self.as_borrowed().format()
+    }
+}
+
+/// Implementation of functionality for [`PyTraceback`].
+///
+/// These methods are defined for the `Bound<'py, PyTraceback>` smart pointer, so to use method call
+/// syntax these methods are separated into a trait, because stable Rust does not yet support
+/// `arbitrary_self_types`.
+#[doc(alias = "PyTraceback")]
+pub trait PyTracebackMethods<'py> {
+    /// Formats the traceback as a string.
+    ///
+    /// This does not include the exception type and value. The exception type and value can be
+    /// formatted using the `Display` implementation for `PyErr`.
+    ///
+    /// # Example
+    ///
+    /// The following code formats a Python traceback and exception pair from Rust:
+    ///
+    /// ```rust
+    /// # use pyo3::{Python, PyResult, prelude::PyTracebackMethods};
+    /// # let result: PyResult<()> =
+    /// Python::with_gil(|py| {
+    ///     let err = py
+    ///         .run("raise Exception('banana')", None, None)
+    ///         .expect_err("raise will create a Python error");
+    ///
+    ///     let traceback = err.traceback_bound(py).expect("raised exception will have a traceback");
+    ///     assert_eq!(
+    ///         format!("{}{}", traceback.format()?, err),
+    ///         "\
+    /// Traceback (most recent call last):
+    ///   File \"<string>\", line 1, in <module>
+    /// Exception: banana\
+    /// "
+    ///     );
+    ///     Ok(())
+    /// })
+    /// # ;
+    /// # result.expect("example failed");
+    /// ```
+    fn format(&self) -> PyResult<String>;
+}
+
+impl<'py> PyTracebackMethods<'py> for Bound<'py, PyTraceback> {
+    fn format(&self) -> PyResult<String> {
         let py = self.py();
         let string_io = py
             .import(intern!(py, "io"))?
@@ -65,7 +112,10 @@ impl PyTraceback {
 
 #[cfg(test)]
 mod tests {
-    use crate::{prelude::*, types::PyDict};
+    use crate::{
+        prelude::*,
+        types::{traceback::PyTracebackMethods, PyDict},
+    };
 
     #[test]
     fn format_traceback() {
@@ -75,7 +125,7 @@ mod tests {
                 .expect_err("raising should have given us an error");
 
             assert_eq!(
-                err.traceback(py).unwrap().format().unwrap(),
+                err.traceback_bound(py).unwrap().format().unwrap(),
                 "Traceback (most recent call last):\n  File \"<string>\", line 1, in <module>\n"
             );
         })
@@ -99,7 +149,7 @@ except Exception as e:
             .unwrap();
             let err = PyErr::from_value(locals.get_item("err").unwrap().unwrap());
             let traceback = err.value(py).getattr("__traceback__").unwrap();
-            assert!(err.traceback(py).unwrap().is(traceback));
+            assert!(err.traceback_bound(py).unwrap().is(traceback));
         })
     }
 
@@ -119,10 +169,10 @@ def f():
             .unwrap();
             let f = locals.get_item("f").unwrap().unwrap();
             let err = f.call0().unwrap_err();
-            let traceback = err.traceback(py).unwrap();
+            let traceback = err.traceback_bound(py).unwrap();
             let err_object = err.clone_ref(py).into_py(py).into_ref(py);
 
-            assert!(err_object.getattr("__traceback__").unwrap().is(traceback));
+            assert!(err_object.getattr("__traceback__").unwrap().is(&traceback));
         })
     }
 }

tests/ui/invalid_result_conversion.stderr

@@ -5,8 +5,8 @@ error[E0277]: the trait bound `PyErr: From<MyError>` is not satisfied
    | ^^^^^^^^^^^^^ the trait `From<MyError>` is not implemented for `PyErr`
    |
    = help: the following other types implement trait `From<T>`:
-             <PyErr as From<PyBorrowError>>
              <PyErr as From<std::io::Error>>
+             <PyErr as From<PyBorrowError>>
              <PyErr as From<PyBorrowMutError>>
              <PyErr as From<PyDowncastError<'a>>>
              <PyErr as From<DowncastError<'_, '_>>>