Merge pull request #2074 from davidhewitt/python-tweaks

opt: improve handle_panic generated code

CHANGELOG.md

@@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - The `create_exception!` macro can now take an optional docstring. This docstring, if supplied, is visible to users (with `.__doc__` and `help()`) and
   accompanies your error type in your crate's documentation.
 - Improve performance and error messages for `#[derive(FromPyObject)]` for enums. [#2068](https://github.com/PyO3/pyo3/pull/2068)
+- Tweak LLVM code for compile times for internal `handle_panic` helper. [#2073](https://github.com/PyO3/pyo3/pull/2073)
 
 ### Removed
 

src/callback.rs

@@ -10,7 +10,7 @@ use crate::{GILPool, IntoPyPointer};
 use crate::{IntoPy, PyObject, Python};
 use std::any::Any;
 use std::os::raw::c_int;
-use std::panic::{AssertUnwindSafe, UnwindSafe};
+use std::panic::UnwindSafe;
 use std::{isize, panic};
 
 /// A type which can be the return type of a python C-API callback
@@ -241,13 +241,8 @@ where
     R: PyCallbackOutput,
 {
     let pool = GILPool::new();
-    let unwind_safe_py = AssertUnwindSafe(pool.python());
+    let py = pool.python();
-    let panic_result = panic::catch_unwind(move || -> PyResult<_> {
+    panic_result_into_callback_output(py, panic::catch_unwind(move || -> PyResult<_> { body(py) }))
-        let py = *unwind_safe_py;
-        body(py)
-    });
-
-    panic_result_into_callback_output(pool.python(), panic_result)
 }
 
 fn panic_result_into_callback_output<R>(

src/gil.rs

@@ -2,7 +2,8 @@
 
 //! Interaction with Python's global interpreter lock
 
-use crate::{ffi, internal_tricks::Unsendable, Python};
+use crate::impl_::not_send::{NotSend, NOT_SEND};
+use crate::{ffi, Python};
 use parking_lot::{const_mutex, Mutex, Once};
 use std::cell::{Cell, RefCell};
 use std::{
@@ -157,6 +158,7 @@ where
 pub struct GILGuard {
     gstate: ffi::PyGILState_STATE,
     pool: ManuallyDrop<Option<GILPool>>,
+    _not_send: NotSend,
 }
 
 impl GILGuard {
@@ -230,6 +232,7 @@ impl GILGuard {
         GILGuard {
             gstate,
             pool: ManuallyDrop::new(pool),
+            _not_send: NOT_SEND,
         }
     }
 }
@@ -332,7 +335,7 @@ pub struct GILPool {
     /// Initial length of owned objects and anys.
     /// `Option` is used since TSL can be broken when `new` is called from `atexit`.
     start: Option<usize>,
-    no_send: Unsendable,
+    _not_send: NotSend,
 }
 
 impl GILPool {
@@ -351,11 +354,12 @@ impl GILPool {
         POOL.update_counts(Python::assume_gil_acquired());
         GILPool {
             start: OWNED_OBJECTS.try_with(|o| o.borrow().len()).ok(),
-            no_send: Unsendable::default(),
+            _not_send: NOT_SEND,
         }
     }
 
     /// Gets the Python token associated with this [`GILPool`].
+    #[inline]
     pub fn python(&self) -> Python {
         unsafe { Python::assume_gil_acquired() }
     }

src/impl_.rs

@@ -8,3 +8,4 @@ pub mod deprecations;
 pub mod freelist;
 #[doc(hidden)]
 pub mod frompyobject;
+pub(crate) mod not_send;

src/impl_/not_send.rs

@@ -0,0 +1,9 @@
+use std::marker::PhantomData;
+
+use crate::Python;
+
+/// A marker type that makes the type !Send.
+/// Workaround for lack of !Send on stable (https://github.com/rust-lang/rust/issues/68318).
+pub(crate) struct NotSend(PhantomData<*mut Python<'static>>);
+
+pub(crate) const NOT_SEND: NotSend = NotSend(PhantomData);

src/internal_tricks.rs

@@ -1,11 +1,5 @@
 use crate::ffi::{Py_ssize_t, PY_SSIZE_T_MAX};
 use std::ffi::{CStr, CString};
-use std::marker::PhantomData;
-use std::rc::Rc;
-
-/// A marker type that makes the type !Send.
-/// Temporal hack until https://github.com/rust-lang/rust/issues/13231 is resolved.
-pub(crate) type Unsendable = PhantomData<Rc<()>>;
 
 pub struct PrivateMarker;
 

src/panic.rs

@@ -17,6 +17,7 @@ Python interpreter to exit.
 
 impl PanicException {
     // Try to format the error in the same way panic does
+    #[cold]
     pub(crate) fn from_panic_payload(payload: Box<dyn Any + Send + 'static>) -> PyErr {
         if let Some(string) = payload.downcast_ref::<String>() {
             Self::new_err((string.clone(),))

src/python.rs

@@ -4,6 +4,7 @@
 
 use crate::err::{self, PyDowncastError, PyErr, PyResult};
 use crate::gil::{self, GILGuard, GILPool};
+use crate::impl_::not_send::NotSend;
 use crate::type_object::{PyTypeInfo, PyTypeObject};
 use crate::types::{PyAny, PyDict, PyModule, PyType};
 use crate::{ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom};
@@ -184,7 +185,7 @@ impl PartialOrd<(u8, u8, u8)> for PythonVersionInfo<'_> {
 /// [`Py::clone_ref`]: crate::Py::clone_ref
 /// [Memory Management]: https://pyo3.rs/main/memory.html#gil-bound-memory
 #[derive(Copy, Clone)]
-pub struct Python<'py>(PhantomData<&'py GILGuard>);
+pub struct Python<'py>(PhantomData<(&'py GILGuard, NotSend)>);
 
 impl Python<'_> {
     /// Acquires the global interpreter lock, allowing access to the Python interpreter. The

tests/test_compile_error.rs

@@ -29,6 +29,7 @@ fn _test_compile_errors() {
     t.compile_fail("tests/ui/invalid_pymodule_args.rs");
     t.compile_fail("tests/ui/missing_clone.rs");
     t.compile_fail("tests/ui/reject_generics.rs");
+    t.compile_fail("tests/ui/not_send.rs");
 
     tests_rust_1_49(&t);
     tests_rust_1_55(&t);

tests/ui/not_send.rs

@@ -0,0 +1,11 @@
+use pyo3::prelude::*;
+
+fn test_not_send_allow_threads(py: Python) {
+    py.allow_threads(|| { drop(py); });
+}
+
+fn main() {
+    Python::with_gil(|py| {
+        test_not_send_allow_threads(py);
+    })
+}

tests/ui/not_send.stderr

@@ -0,0 +1,14 @@
+error[E0277]: `*mut pyo3::Python<'static>` cannot be shared between threads safely
+ --> tests/ui/not_send.rs:4:8
+  |
+4 |     py.allow_threads(|| { drop(py); });
+  |        ^^^^^^^^^^^^^ `*mut pyo3::Python<'static>` cannot be shared between threads safely
+  |
+  = help: within `pyo3::Python<'_>`, the trait `Sync` is not implemented for `*mut pyo3::Python<'static>`
+  = note: required because it appears within the type `PhantomData<*mut pyo3::Python<'static>>`
+  = note: required because it appears within the type `pyo3::impl_::not_send::NotSend`
+  = note: required because it appears within the type `(&GILGuard, pyo3::impl_::not_send::NotSend)`
+  = note: required because it appears within the type `PhantomData<(&GILGuard, pyo3::impl_::not_send::NotSend)>`
+  = note: required because it appears within the type `pyo3::Python<'_>`
+  = note: required because of the requirements on the impl of `Send` for `&pyo3::Python<'_>`
+  = note: required because it appears within the type `[closure@$DIR/tests/ui/not_send.rs:4:22: 4:38]`