internal: introduce ErrorAlreadySet marker when FFI calls return an exception

src/ffi_ptr_ext.rs

@@ -1,3 +1,4 @@
+use crate::internal::err::ErrorAlreadySet;
 use crate::sealed::Sealed;
 use crate::{
     ffi,
@@ -11,6 +12,12 @@ pub(crate) trait FfiPtrExt: Sealed {
     /// If the pointer is NULL, this function will fetch an error.
     unsafe fn assume_owned_or_err(self, py: Python<'_>) -> PyResult<Bound<'_, PyAny>>;
 
+    /// Similar to the above, but doesn't fetch the error.
+    unsafe fn assume_owned_or_err_set(
+        self,
+        py: Python<'_>,
+    ) -> Result<Bound<'_, PyAny>, ErrorAlreadySet<'_>>;
+
     /// Same as `assume_owned_or_err`, but doesn't fetch an error on NULL.
     unsafe fn assume_owned_or_opt(self, py: Python<'_>) -> Option<Bound<'_, PyAny>>;
 
@@ -43,6 +50,14 @@ impl FfiPtrExt for *mut ffi::PyObject {
         unsafe { Bound::from_owned_ptr_or_err(py, self) }
     }
 
+    #[inline]
+    unsafe fn assume_owned_or_err_set(
+        self,
+        py: Python<'_>,
+    ) -> Result<Bound<'_, PyAny>, ErrorAlreadySet<'_>> {
+        unsafe { Bound::from_owned_ptr_or_opt(py, self).ok_or_else(|| ErrorAlreadySet::new(py)) }
+    }
+
     #[inline]
     unsafe fn assume_owned_or_opt(self, py: Python<'_>) -> Option<Bound<'_, PyAny>> {
         unsafe { Bound::from_owned_ptr_or_opt(py, self) }

src/internal.rs

@@ -1,5 +1,6 @@
 //! Holding place for code which is not intended to be reachable from outside of PyO3.
 
+pub(crate) mod err;
 pub(crate) mod get_slot;
 pub(crate) mod pyclass_init;
 pub(crate) mod state;

src/internal/err.rs

@@ -0,0 +1,39 @@
+use std::mem::ManuallyDrop;
+
+use crate::{ffi, PyErr, Python};
+
+/// Denotes that an error occurred and the Python interpreter has an error set
+/// in the thread state.
+///
+/// While this type is alive, it is unsafe to do most Python C API calls.
+///
+/// Dropping this type will clear the error in the Python interpreter. This
+/// type also has a conversion to `PyErr` to fetch the error.
+pub struct ErrorAlreadySet<'py>(Python<'py>);
+
+impl<'py> ErrorAlreadySet<'py> {
+    /// # Safety
+    /// - Caller is responsible to ensure that no Python C APIs are called while
+    ///   this type is alive (except for those specifically for error handling).
+    #[inline]
+    pub unsafe fn new(py: Python<'py>) -> Self {
+        Self(py)
+    }
+}
+
+impl Drop for ErrorAlreadySet<'_> {
+    #[inline]
+    fn drop(&mut self) {
+        // SAFETY: `self.0` proves the thread is attached to the interpreter.
+        unsafe { ffi::PyErr_Clear() }
+    }
+}
+
+impl From<ErrorAlreadySet<'_>> for PyErr {
+    #[inline]
+    fn from(err: ErrorAlreadySet<'_>) -> Self {
+        // Avoid calling `PyErr_Clear` after fetching
+        let err = ManuallyDrop::new(err);
+        PyErr::fetch(err.0)
+    }
+}

src/py_result_ext.rs

@@ -1,13 +1,16 @@
-use crate::{Bound, PyAny, PyResult, PyTypeCheck};
+use crate::{internal::err::ErrorAlreadySet, Bound, PyAny, PyErr, PyResult, PyTypeCheck};
 
 pub(crate) trait PyResultExt<'py>: crate::sealed::Sealed {
-    fn cast_into<T: PyTypeCheck>(self) -> PyResult<Bound<'py, T>>;
-    unsafe fn cast_into_unchecked<T>(self) -> PyResult<Bound<'py, T>>;
+    type Error;
+    fn cast_into<T: PyTypeCheck>(self) -> Result<Bound<'py, T>, PyErr>;
+    unsafe fn cast_into_unchecked<T>(self) -> Result<Bound<'py, T>, Self::Error>;
 }
 
 impl<'py> PyResultExt<'py> for PyResult<Bound<'py, PyAny>> {
+    type Error = PyErr;
+
     #[inline]
-    fn cast_into<T: PyTypeCheck>(self) -> PyResult<Bound<'py, T>> where {
+    fn cast_into<T: PyTypeCheck>(self) -> PyResult<Bound<'py, T>> {
         self.and_then(|instance| instance.cast_into().map_err(Into::into))
     }
 
@@ -16,3 +19,17 @@ impl<'py> PyResultExt<'py> for PyResult<Bound<'py, PyAny>> {
         self.map(|instance| unsafe { instance.cast_into_unchecked() })
     }
 }
+
+impl<'py> PyResultExt<'py> for Result<Bound<'py, PyAny>, ErrorAlreadySet<'py>> {
+    type Error = ErrorAlreadySet<'py>;
+
+    #[inline]
+    fn cast_into<T: PyTypeCheck>(self) -> Result<Bound<'py, T>, PyErr> {
+        self?.cast_into().map_err(Into::into)
+    }
+
+    #[inline]
+    unsafe fn cast_into_unchecked<T>(self) -> Result<Bound<'py, T>, ErrorAlreadySet<'py>> {
+        self.map(|instance| unsafe { instance.cast_into_unchecked() })
+    }
+}

src/sealed.rs

@@ -1,4 +1,5 @@
 use crate::impl_::pyfunction::PyFunctionDef;
+use crate::internal::err::ErrorAlreadySet;
 #[cfg(all(not(Py_LIMITED_API), not(PyPy), not(GraalPy)))]
 use crate::types::PyFrame;
 use crate::types::{
@@ -23,6 +24,7 @@ impl Sealed for *mut ffi::PyObject {}
 
 // for PyResultExt
 impl Sealed for PyResult<Bound<'_, PyAny>> {}
+impl<'py> Sealed for Result<Bound<'py, PyAny>, ErrorAlreadySet<'py>> {}
 
 // for Py(...)Methods
 impl Sealed for Bound<'_, PyAny> {}

src/types/set.rs

@@ -207,7 +207,7 @@ impl<'py> PySetMethods<'py> for Bound<'py, PySet> {
     }
 
     fn pop(&self) -> Option<Bound<'py, PyAny>> {
-        let element = unsafe { ffi::PySet_Pop(self.as_ptr()).assume_owned_or_err(self.py()) };
+        let element = unsafe { ffi::PySet_Pop(self.as_ptr()).assume_owned_or_err_set(self.py()) };
         element.ok()
     }
 

src/types/tuple.rs

@@ -3,7 +3,9 @@ use crate::ffi_ptr_ext::FfiPtrExt;
 #[cfg(feature = "experimental-inspect")]
 use crate::inspect::{type_hint_subscript, PyStaticExpr};
 use crate::instance::Borrowed;
+use crate::internal::err::ErrorAlreadySet;
 use crate::internal_tricks::get_ssize_index;
+use crate::py_result_ext::PyResultExt;
 #[cfg(feature = "experimental-inspect")]
 use crate::type_object::PyTypeInfo;
 use crate::types::{sequence::PySequenceMethods, PyList, PySequence};
@@ -40,26 +42,22 @@ fn try_new_from_iter<'py>(
     mut elements: impl ExactSizeIterator<Item = PyResult<Bound<'py, PyAny>>>,
 ) -> PyResult<Bound<'py, PyTuple>> {
     #[cfg(not(RustPython))]
-    unsafe {
+    {
         // PyTuple_New checks for overflow but has a bad error message, so we check ourselves
         let len: Py_ssize_t = elements
             .len()
             .try_into()
             .expect("out of range integral type conversion attempted on `elements.len()`");
 
-        let ptr = ffi::PyTuple_New(len);
-
-        // - Panics if the ptr is null
-        // - Cleans up the tuple if `convert` or the asserts panic
-        let tup = ptr.assume_owned(py).cast_into_unchecked();
+        // SAFETY: this function will fully initialize the tuple, or drop it on error / panic paths
+        let tup = unsafe { PyTuple::new_uninit(py, len)? };
 
         let mut counter: Py_ssize_t = 0;
 
-        for obj in (&mut elements).take(len as usize) {
-            #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))]
-            ffi::PyTuple_SET_ITEM(ptr, counter, obj?.into_ptr());
-            #[cfg(any(Py_LIMITED_API, PyPy, GraalPy))]
-            ffi::PyTuple_SetItem(ptr, counter, obj?.into_ptr());
+        for result in (&mut elements).take(len as usize) {
+            let obj = result?;
+            // SAFETY: `tup` needs `len` elements initialized
+            unsafe { tup.as_borrowed().set_item_unchecked(counter, obj) };
             counter += 1;
         }
 
@@ -70,12 +68,14 @@ fn try_new_from_iter<'py>(
     }
 
     #[cfg(RustPython)]
-    unsafe {
+    {
         let elements = elements.collect::<PyResult<Vec<_>>>()?;
         // SAFETY: list is layout compatible with *const *mut crate::PyObject
-        ffi::PyTuple_FromArray(elements.as_ptr().cast(), elements.len() as _)
-            .assume_owned_or_err(py)
-            .cast_into_unchecked()
+        unsafe {
+            ffi::PyTuple_FromArray(elements.as_ptr().cast(), elements.len() as _)
+                .assume_owned_or_err(py)
+                .cast_into_unchecked()
+        }
     }
 }
 
@@ -154,7 +154,40 @@ impl PyTuple {
 
     /// Constructs an empty tuple (on the Python side, a singleton object).
     pub fn empty(py: Python<'_>) -> Bound<'_, PyTuple> {
-        unsafe { ffi::PyTuple_New(0).assume_owned(py).cast_into_unchecked() }
+        // SAFETY: can never fail to construct empty tuple, and it's immediately fully init
+        #[cfg(not(Py_3_13))]
+        unsafe {
+            Self::new_uninit(py, 0).unwrap_unchecked()
+        }
+
+        #[cfg(Py_3_13)]
+        // SAFETY:
+        // - thread is attached to the interpreter
+        // - constant is known to be owned and of the correct type
+        unsafe {
+            ffi::Py_GetConstant(ffi::Py_CONSTANT_EMPTY_TUPLE)
+                .assume_owned_unchecked(py)
+                .cast_into_unchecked()
+        }
+    }
+
+    /// # Safety
+    /// - Caller must set all elements with `set_item_unchecked` before exposing the
+    ///   returned tuple to user code.
+    /// - `ErrorAlreadySet` must be handled by caller
+    #[inline]
+    unsafe fn new_uninit<'py>(
+        py: Python<'py>,
+        len: Py_ssize_t,
+    ) -> Result<Bound<'py, PyTuple>, ErrorAlreadySet<'py>> {
+        // SAFETY:
+        // - thread is attached to the interpreter
+        // - success return is guaranteed to be a tuple
+        unsafe {
+            ffi::PyTuple_New(len)
+                .assume_owned_or_err_set(py)
+                .cast_into_unchecked()
+        }
     }
 }
 
@@ -365,6 +398,32 @@ impl<'a, 'py> Borrowed<'a, 'py, PyTuple> {
         }
     }
 
+    /// # Safety
+    /// - `self` must be a tuple under initialization with `PyTuple::new_uninit` that is not yet
+    ///   shared with external code.
+    /// - `self` mut not already have a value set at `index`.
+    /// - `index` must be within the bounds of the tuple.
+    #[inline]
+    unsafe fn set_item_unchecked(self, index: Py_ssize_t, obj: Bound<'_, PyAny>) {
+        #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))]
+        // SAFETY:
+        // - Caller-provided invariants and smart pointers guarantee correctness
+        // - `PyTuple_SET_ITEM` steals a reference to `obj`
+        unsafe {
+            ffi::PyTuple_SET_ITEM(self.as_ptr(), index, obj.into_ptr())
+        }
+
+        // SAFETY:
+        // - Caller-provided invariants and smart pointers guarantee correctness
+        // - `PyTuple_SetItem` steals a reference to `obj`
+        // - `PyTuple_SetItem` returns -1 if `index` is out of bounds, but we are
+        //   not checking for errors here.
+        #[cfg(any(Py_LIMITED_API, PyPy, GraalPy))]
+        unsafe {
+            ffi::PyTuple_SetItem(self.as_ptr(), index, obj.into_ptr());
+        }
+    }
+
     pub(crate) fn iter_borrowed(self) -> BorrowedTupleIterator<'a, 'py> {
         BorrowedTupleIterator::new(self)
     }
@@ -662,7 +721,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
         );
 
         fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-            Ok(array_into_tuple(py, [$(self.$n.into_bound_py_any(py)?),+]))
+            Ok(array_into_tuple(py, [$(self.$n.into_bound_py_any(py)?),+])?)
         }
     }
 
@@ -681,7 +740,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
         );
 
         fn into_pyobject(self, py: Python<'py>) -> Result<Self::Output, Self::Error> {
-            Ok(array_into_tuple(py, [$(self.$n.into_bound_py_any(py)?),+]))
+            Ok(array_into_tuple(py, [$(self.$n.into_bound_py_any(py)?),+])?)
         }
     }
 
@@ -965,26 +1024,26 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
 fn array_into_tuple<'py, const N: usize>(
     py: Python<'py>,
     array: [Bound<'py, PyAny>; N],
-) -> Bound<'py, PyTuple> {
+) -> Result<Bound<'py, PyTuple>, ErrorAlreadySet<'py>> {
+    let len: Py_ssize_t = N.try_into().expect("0 < N <= 12");
+
     #[cfg(not(RustPython))]
-    unsafe {
-        let ptr = ffi::PyTuple_New(N.try_into().expect("0 < N <= 12"));
-        let tup = ptr.assume_owned(py).cast_into_unchecked();
+    {
+        // SAFETY: this function will fully initialize the tuple
+        let tup = unsafe { PyTuple::new_uninit(py, len)? };
         for (index, obj) in array.into_iter().enumerate() {
-            #[cfg(not(any(Py_LIMITED_API, PyPy, GraalPy)))]
-            ffi::PyTuple_SET_ITEM(ptr, index as ffi::Py_ssize_t, obj.into_ptr());
-            #[cfg(any(Py_LIMITED_API, PyPy, GraalPy))]
-            ffi::PyTuple_SetItem(ptr, index as ffi::Py_ssize_t, obj.into_ptr());
+            // SAFETY: index is guaranteed to be < N
+            unsafe { tup.as_borrowed().set_item_unchecked(index as _, obj) };
         }
-        tup
+        Ok(tup)
     }
 
     // SAFETY: array is layout compatible with *const *mut crate::PyObject
     // and does not steal the bound reference.
     #[cfg(RustPython)]
     unsafe {
-        ffi::PyTuple_FromArray(array.as_ptr().cast(), N.try_into().expect("0 < N <= 12"))
-            .assume_owned(py)
+        ffi::PyTuple_FromArray(array.as_ptr().cast(), len)
+            .assume_owned_or_err_set(py)
             .cast_into_unchecked()
     }
 }