Merge branch 'PyO3:main' into herquan_cr1

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Deprecate `ToBorrowedObject` trait (it is only used as a wrapper for `ToPyObject`). [#2333](https://github.com/PyO3/pyo3/pull/2333)
 - `impl<T, const N: usize> IntoPy<PyObject> for [T; N]` now requires `T: IntoPy` rather than `T: ToPyObject`. [#2326](https://github.com/PyO3/pyo3/pull/2326)
 - Correct `wrap_pymodule` to match normal namespacing rules: it no longer "sees through" glob imports of `use submodule::*` when `submodule::submodule` is a `#[pymodule]`. [#2363](https://github.com/PyO3/pyo3/pull/2363)
+- Allow `#[classattr]` methods to be fallible. [#2385](https://github.com/PyO3/pyo3/pull/2385)
 
 ### Fixed
 

guide/src/class.md

@@ -583,8 +583,7 @@ impl MyClass {
 ## Class attributes
 
 To create a class attribute (also called [class variable][classattr]), a method without
-any arguments can be annotated with the `#[classattr]` attribute. The return type must be `T` for
-some `T` that implements `IntoPy<PyObject>`.
+any arguments can be annotated with the `#[classattr]` attribute.
 
 ```rust
 # use pyo3::prelude::*;
@@ -604,6 +603,9 @@ Python::with_gil(|py| {
 });
 ```
 
+> Note: if the method has a `Result` return type and returns an `Err`, PyO3 will panic during
+class creation.
+
 If the class attribute is defined with `const` code only, one can also annotate associated
 constants:
 

pyo3-macros-backend/src/pyimpl.rs

@@ -171,9 +171,9 @@ pub fn gen_py_const(cls: &syn::Type, spec: &ConstSpec) -> TokenStream {
             _pyo3::class::PyClassAttributeDef::new(
                 #python_name,
                 _pyo3::impl_::pymethods::PyClassAttributeFactory({
-                    fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyObject {
+                    fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
                         #deprecations
-                        _pyo3::IntoPy::into_py(#cls::#member, py)
+                        ::std::result::Result::Ok(_pyo3::IntoPy::into_py(#cls::#member, py))
                     }
                     __wrap
                 })

pyo3-macros-backend/src/pymethod.rs

@@ -349,9 +349,14 @@ fn impl_py_class_attribute(cls: &syn::Type, spec: &FnSpec<'_>) -> TokenStream {
             _pyo3::class::PyClassAttributeDef::new(
                 #python_name,
                 _pyo3::impl_::pymethods::PyClassAttributeFactory({
-                    fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyObject {
+                    fn __wrap(py: _pyo3::Python<'_>) -> _pyo3::PyResult<_pyo3::PyObject> {
                         #deprecations
-                        _pyo3::IntoPy::into_py(#cls::#name(), py)
+                        let mut ret = #cls::#name();
+                        if false {
+                            use _pyo3::impl_::ghost::IntoPyResult;
+                            ret.assert_into_py_result();
+                        }
+                        _pyo3::callback::convert(py, ret)
                     }
                     __wrap
                 })

src/exceptions.rs

@@ -5,8 +5,8 @@
 //! The structs in this module represent Python's built-in exceptions, while the modules comprise
 //! structs representing errors defined in Python code.
 //!
-//! The latter are created with the [`import_exception`] macro, which you can use yourself
-//! to import Python exceptions.
+//! The latter are created with the [`import_exception`](crate::import_exception) macro, which you
+//! can use yourself to import Python exceptions.
 
 use crate::{ffi, PyResult, Python};
 use std::ffi::CStr;

src/impl_/pymethods.rs

@@ -76,7 +76,7 @@ pub struct PyGetter(pub ffi::getter);
 #[derive(Clone, Copy, Debug)]
 pub struct PySetter(pub ffi::setter);
 #[derive(Clone, Copy)]
-pub struct PyClassAttributeFactory(pub for<'p> fn(Python<'p>) -> PyObject);
+pub struct PyClassAttributeFactory(pub for<'p> fn(Python<'p>) -> PyResult<PyObject>);
 
 // TODO: it would be nice to use CStr in these types, but then the constructors can't be const fn
 // until `CStr::from_bytes_with_nul_unchecked` is const fn.

src/instance.rs

@@ -541,8 +541,9 @@ impl<T> Py<T> {
     ///
     /// This is equivalent to the Python expression `self.attr_name`.
     ///
-    /// If calling this method becomes performance-critical, the [`intern!`] macro can be used
-    /// to intern `attr_name`, thereby avoiding repeated temporary allocations of Python strings.
+    /// If calling this method becomes performance-critical, the [`intern!`](crate::intern) macro
+    /// can be used to intern `attr_name`, thereby avoiding repeated temporary allocations of
+    /// Python strings.
     ///
     /// # Example: `intern!`ing the attribute name
     ///
@@ -577,8 +578,8 @@ impl<T> Py<T> {
     ///
     /// This is equivalent to the Python expression `self.attr_name = value`.
     ///
-    /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used
-    /// to intern `attr_name`.
+    /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
+    /// macro can be used to intern `attr_name`.
     ///
     /// # Example: `intern!`ing the attribute name
     ///
@@ -660,8 +661,8 @@ impl<T> Py<T> {
     ///
     /// This is equivalent to the Python expression `self.name(*args, **kwargs)`.
     ///
-    /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used
-    /// to intern `name`.
+    /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
+    /// macro can be used to intern `name`.
     pub fn call_method<N, A>(
         &self,
         py: Python<'_>,
@@ -691,8 +692,8 @@ impl<T> Py<T> {
     ///
     /// This is equivalent to the Python expression `self.name(*args)`.
     ///
-    /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used
-    /// to intern `name`.
+    /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
+    /// macro can be used to intern `name`.
     pub fn call_method1<N, A>(&self, py: Python<'_>, name: N, args: A) -> PyResult<PyObject>
     where
         N: IntoPy<Py<PyString>>,
@@ -705,8 +706,8 @@ impl<T> Py<T> {
     ///
     /// This is equivalent to the Python expression `self.name()`.
     ///
-    /// To avoid repeated temporary allocations of Python strings, the [`intern!`] macro can be used
-    /// to intern `name`.
+    /// To avoid repeated temporary allocations of Python strings, the [`intern!`](crate::intern)
+    /// macro can be used to intern `name`.
     pub fn call_method0<N>(&self, py: Python<'_>, name: N) -> PyResult<PyObject>
     where
         N: IntoPy<Py<PyString>>,

src/type_object.rs

@@ -133,8 +133,8 @@ impl LazyStaticType {
             return;
         }
 
+        let thread_id = thread::current().id();
         {
-            let thread_id = thread::current().id();
             let mut threads = self.initializing_threads.lock();
             if threads.contains(&thread_id) {
                 // Reentrant call: just return the type object, even if the
@@ -144,26 +144,47 @@ impl LazyStaticType {
             threads.push(thread_id);
         }
 
+        struct InitializationGuard<'a> {
+            initializing_threads: &'a Mutex<Vec<ThreadId>>,
+            thread_id: ThreadId,
+        }
+        impl Drop for InitializationGuard<'_> {
+            fn drop(&mut self) {
+                let mut threads = self.initializing_threads.lock();
+                threads.retain(|id| *id != self.thread_id);
+            }
+        }
+
+        let guard = InitializationGuard {
+            initializing_threads: &self.initializing_threads,
+            thread_id,
+        };
+
         // Pre-compute the class attribute objects: this can temporarily
         // release the GIL since we're calling into arbitrary user code. It
         // means that another thread can continue the initialization in the
         // meantime: at worst, we'll just make a useless computation.
         let mut items = vec![];
         for_all_items(&mut |class_items| {
-            items.extend(class_items.methods.iter().filter_map(|def| {
+            for def in class_items.methods {
                 if let PyMethodDefType::ClassAttribute(attr) = def {
                     let key = extract_cstr_or_leak_cstring(
                         attr.name,
                         "class attribute name cannot contain nul bytes",
                     )
                     .unwrap();
 
-                    let val = (attr.meth.0)(py);
-                    Some((key, val))
-                } else {
-                    None
+                    match (attr.meth.0)(py) {
+                        Ok(val) => items.push((key, val)),
+                        Err(e) => panic!(
+                            "An error occurred while initializing `{}.{}`: {}",
+                            name,
+                            attr.name.trim_end_matches('\0'),
+                            e
+                        ),
+                    }
                 }
-            }));
+            }
         });
 
         // Now we hold the GIL and we can assume it won't be released until we
@@ -173,6 +194,7 @@ impl LazyStaticType {
 
             // Initialization successfully complete, can clear the thread list.
             // (No further calls to get_or_init() will try to init, on any thread.)
+            std::mem::forget(guard);
             *self.initializing_threads.lock() = Vec::new();
             result
         });

src/types/datetime.rs

@@ -153,9 +153,9 @@ pub trait PyTimeAccess {
     /// Returns whether this date is the later of two moments with the
     /// same representation, during a repeated interval.
     ///
-    /// This typically occurs at the end of daylight savings time, or during
-    /// leap seconds. Only valid if the represented time is ambiguous. See
-    /// [PEP 495](https://www.python.org/dev/peps/pep-0495/) for more detail.
+    /// This typically occurs at the end of daylight savings time. Only valid if the
+    /// represented time is ambiguous.
+    /// See [PEP 495](https://www.python.org/dev/peps/pep-0495/) for more detail.
     #[cfg(not(PyPy))]
     fn get_fold(&self) -> bool;
 }
@@ -267,7 +267,12 @@ impl PyDateTime {
     }
 
     /// Alternate constructor that takes a `fold` parameter. A `true` value for this parameter
-    /// signifies a leap second
+    /// signifies this this datetime is the later of two moments with the same representation,
+    /// during a repeated interval.
+    ///
+    /// This typically occurs at the end of daylight savings time. Only valid if the
+    /// represented time is ambiguous.
+    /// See [PEP 495](https://www.python.org/dev/peps/pep-0495/) for more detail.
     #[cfg(not(PyPy))]
     #[allow(clippy::too_many_arguments)]
     pub fn new_with_fold<'p>(
@@ -413,7 +418,7 @@ impl PyTime {
     }
 
     #[cfg(not(PyPy))]
-    /// Alternate constructor that takes a `fold` argument
+    /// Alternate constructor that takes a `fold` argument. See [`PyDateTime::new_with_fold`].
     pub fn new_with_fold<'p>(
         py: Python<'p>,
         hour: u8,

tests/test_class_attributes.rs

@@ -1,6 +1,6 @@
 #![cfg(feature = "macros")]
 
-use pyo3::prelude::*;
+use pyo3::{exceptions::PyValueError, prelude::*};
 
 mod common;
 
@@ -89,3 +89,25 @@ fn recursive_class_attributes() {
     py_assert!(py, foo_obj, "foo_obj.bar.x == 2");
     py_assert!(py, bar_obj, "bar_obj.a_foo.x == 3");
 }
+
+#[test]
+#[should_panic(
+    expected = "An error occurred while initializing `BrokenClass.fails_to_init`: \
+                ValueError: failed to create class attribute"
+)]
+fn test_fallible_class_attribute() {
+    #[pyclass]
+    struct BrokenClass;
+
+    #[pymethods]
+    impl BrokenClass {
+        #[classattr]
+        fn fails_to_init() -> PyResult<i32> {
+            Err(PyValueError::new_err("failed to create class attribute"))
+        }
+    }
+
+    Python::with_gil(|py| {
+        py.get_type::<BrokenClass>();
+    })
+}