fix some tests and linting errors

PyO3 · Nov 3, 2024 · 5499d10 · 5499d10
1 parent d0961fe
commit 5499d10
Show file tree

Hide file tree

Showing 11 changed files with 94 additions and 49 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -12,7 +12,7 @@ categories = ["api-bindings", "development-tools::ffi"]
 license = "MIT OR Apache-2.0"
 exclude = ["/.gitignore", ".cargo/config", "/codecov.yml", "/Makefile", "/pyproject.toml", "/noxfile.py", "/.github", "/tests/test_compile_error.rs", "/tests/ui"]
 edition = "2021"
-rust-version = "1.63"
+rust-version = "1.65"
 
 [dependencies]
 cfg-if = "1.0"

diff --git a/guide/src/class/metaclass.md b/guide/src/class/metaclass.md
@@ -29,9 +29,8 @@ impl MyMetaclass {
         slf: Bound<'_, Metaclass>,
         _args: Bound<'_, PyTuple>,
         _kwargs: Option<Bound<'_, PyDict>>,
-    ) -> PyResult<()> {
+    ) {
         slf.borrow_mut().counter = 5;
-        Ok(())
     }
 
     fn __getitem__(&self, item: u64) -> u64 {

diff --git a/src/impl_/pyclass.rs b/src/impl_/pyclass.rs
@@ -1,3 +1,5 @@
+#[cfg(Py_3_12)]
+use crate::pycell::impl_::PyClassObjectContents;
 use crate::{
     conversion::IntoPyObject,
     exceptions::{PyAttributeError, PyNotImplementedError, PyRuntimeError, PyValueError},
@@ -8,10 +10,7 @@ use crate::{
         pyclass_init::PyObjectInit,
         pymethods::{PyGetterDef, PyMethodDefType},
     },
-    pycell::{
-        impl_::{InternalPyClassObjectLayout, PyClassObjectContents},
-        PyBorrowError,
-    },
+    pycell::{impl_::InternalPyClassObjectLayout, PyBorrowError},
     types::{any::PyAnyMethods, PyBool},
     Borrowed, BoundObject, Py, PyAny, PyClass, PyErr, PyRef, PyResult, PyTypeInfo, Python,
 };
@@ -1197,20 +1196,9 @@ pub enum PyObjectOffset {
     /// An offset relative to the start of the subclass-specific data.
     /// Only allowed when basicsize is negative (which is only allowed for python >=3.12).
     /// <https://docs.python.org/3.12/c-api/structures.html#c.Py_RELATIVE_OFFSET>
-    #[cfg(Py_3_12)]
     Relative(ffi::Py_ssize_t),
 }
 
-impl PyObjectOffset {
-    pub fn to_value_and_is_relative(&self) -> (ffi::Py_ssize_t, bool) {
-        match self {
-            PyObjectOffset::Absolute(offset) => (*offset, false),
-            #[cfg(Py_3_12)]
-            PyObjectOffset::Relative(offset) => (*offset, true),
-        }
-    }
-}
-
 impl std::ops::Add<usize> for PyObjectOffset {
     type Output = PyObjectOffset;
 
@@ -1221,7 +1209,6 @@ impl std::ops::Add<usize> for PyObjectOffset {
 
         match self {
             PyObjectOffset::Absolute(offset) => PyObjectOffset::Absolute(offset + rhs),
-            #[cfg(Py_3_12)]
             PyObjectOffset::Relative(offset) => PyObjectOffset::Relative(offset + rhs),
         }
     }
@@ -1321,11 +1308,16 @@ impl<
     pub fn generate(&self, name: &'static CStr, doc: &'static CStr) -> PyMethodDefType {
         use crate::pyclass::boolean_struct::private::Boolean;
         if ClassT::Frozen::VALUE {
-            let (offset, is_relative) = Offset::offset().to_value_and_is_relative();
-            let flags = if is_relative {
-                ffi::Py_READONLY | ffi::Py_RELATIVE_OFFSET
-            } else {
-                ffi::Py_READONLY
+            let (offset, flags) = match Offset::offset() {
+                PyObjectOffset::Absolute(offset) => (offset, ffi::Py_READONLY),
+                #[cfg(Py_3_12)]
+                PyObjectOffset::Relative(offset) => {
+                    (offset, ffi::Py_READONLY | ffi::Py_RELATIVE_OFFSET)
+                }
+                #[cfg(not(Py_3_12))]
+                PyObjectOffset::Relative(_) => {
+                    panic!("relative offsets not valid before python 3.12");
+                }
             };
             PyMethodDefType::StructMember(ffi::PyMemberDef {
                 name: name.as_ptr(),
@@ -1560,6 +1552,10 @@ where
             let contents = class_obj.contents_mut() as *mut PyClassObjectContents<ClassT>;
             (contents.cast::<u8>(), offset)
         }
+        #[cfg(not(Py_3_12))]
+        PyObjectOffset::Relative(_) => {
+            panic!("relative offsets not valid before python 3.12");
+        }
     };
     // Safety: conditions for pointer addition must be met
     unsafe { base.add(offset as usize) }.cast::<FieldT>()

diff --git a/src/pycell/impl_.rs b/src/pycell/impl_.rs
@@ -248,6 +248,14 @@ pub trait PyClassObjectLayout<T>: PyLayout<T> {
 }
 
 #[doc(hidden)]
+#[cfg_attr(
+    all(diagnostic_namespace),
+    diagnostic::on_unimplemented(
+        message = "the class layout is not valid",
+        label = "required for `#[pyclass(extends=...)]`",
+        note = "the python version being built against influences which layouts are valid",
+    )
+)]
 pub trait InternalPyClassObjectLayout<T: PyClassImpl>: PyClassObjectLayout<T> {
     /// Obtain a pointer to the contents of an uninitialized PyObject of this type
     /// Safety: the provided object must have the layout that the implementation is expecting
@@ -551,6 +559,7 @@ impl<T: PyClassImpl> InternalPyClassObjectLayout<T> for PyVariableClassObject<T>
 
 unsafe impl<T: PyClassImpl> PyLayout<T> for PyVariableClassObject<T> {}
 
+#[cfg(Py_3_12)]
 impl<T: PyClassImpl> PyClassObjectLayout<T> for PyVariableClassObject<T>
 where
     <T::BaseType as PyClassBaseType>::LayoutAsBase: PyClassObjectLayout<T::BaseType>,

diff --git a/src/pyclass/create_type_object.rs b/src/pyclass/create_type_object.rs
@@ -19,7 +19,7 @@ use std::{
     collections::HashMap,
     ffi::{CStr, CString},
     os::raw::{c_char, c_int, c_ulong, c_void},
-    ptr,
+    ptr::{self, addr_of_mut},
 };
 
 pub(crate) struct PyClassTypeObject {
@@ -260,7 +260,11 @@ impl PyTypeBuilder {
                 }
 
                 get_dict = get_dict_impl;
-                closure = dict_offset as _;
+                if let PyObjectOffset::Absolute(offset) = dict_offset {
+                    closure = offset as _;
+                } else {
+                    unreachable!("PyObjectOffset::Relative requires >=3.12");
+                }
             }
 
             property_defs.push(ffi::PyGetSetDef {
@@ -370,11 +374,16 @@ impl PyTypeBuilder {
         {
             #[inline(always)]
             fn offset_def(name: &'static CStr, offset: PyObjectOffset) -> ffi::PyMemberDef {
-                let (offset, is_relative) = offset.to_value_and_is_relative();
-                let flags = if is_relative {
-                    ffi::Py_READONLY | ffi::Py_RELATIVE_OFFSET
-                } else {
-                    ffi::Py_READONLY
+                let (offset, flags) = match offset {
+                    PyObjectOffset::Absolute(offset) => (offset, ffi::Py_READONLY),
+                    #[cfg(Py_3_12)]
+                    PyObjectOffset::Relative(offset) => {
+                        (offset, ffi::Py_READONLY | ffi::Py_RELATIVE_OFFSET)
+                    }
+                    #[cfg(not(Py_3_12))]
+                    PyObjectOffset::Relative(_) => {
+                        panic!("relative offsets not valid before python 3.12");
+                    }
                 };
                 ffi::PyMemberDef {
                     name: name.as_ptr().cast(),
@@ -414,16 +423,19 @@ impl PyTypeBuilder {
                         Some(PyObjectOffset::Absolute(offset)) => {
                             (*type_object).tp_dictoffset = offset;
                         }
-                        // PyObjectOffset::Relative requires >=3.12
-                        _ => {}
+                        Some(PyObjectOffset::Relative(_)) => {
+                            panic!("PyObjectOffset::Relative requires >=3.12")
+                        }
+                        None => {}
                     }
-
                     match weaklist_offset {
                         Some(PyObjectOffset::Absolute(offset)) => {
                             (*type_object).tp_weaklistoffset = offset;
                         }
-                        // PyObjectOffset::Relative requires >=3.12
-                        _ => {}
+                        Some(PyObjectOffset::Relative(_)) => {
+                            panic!("PyObjectOffset::Relative requires >=3.12")
+                        }
+                        None => {}
                     }
                 }));
         }
@@ -447,7 +459,7 @@ impl PyTypeBuilder {
 
         // Safety: self.tp_base must be a valid PyTypeObject
         let is_metaclass =
-            unsafe { ffi::PyType_IsSubtype(self.tp_base, &raw mut ffi::PyType_Type) } != 0;
+            unsafe { ffi::PyType_IsSubtype(self.tp_base, addr_of_mut!(ffi::PyType_Type)) } != 0;
         if is_metaclass {
             // if the pyclass derives from `type` (is a metaclass) then `tp_new` must not be set.
             // Metaclasses that override tp_new are not supported.

diff --git a/src/type_object.rs b/src/type_object.rs
@@ -2,7 +2,6 @@
 
 use crate::ffi_ptr_ext::FfiPtrExt;
 use crate::impl_::pyclass::PyClassImpl;
-use crate::pycell::impl_::InternalPyClassObjectLayout;
 use crate::types::any::PyAnyMethods;
 use crate::types::{PyAny, PyType};
 use crate::{ffi, Bound, Python};
@@ -44,8 +43,9 @@ pub unsafe trait PyTypeInfo: Sized {
     /// Module name, if any.
     const MODULE: Option<&'static str>;
 
-    /// The type of object layout to use for ancestors or descendents of this type
-    type Layout<T: PyClassImpl>: InternalPyClassObjectLayout<T>;
+    /// The type of object layout to use for ancestors or descendants of this type.
+    /// should implement `InternalPyClassObjectLayout<T>` in order to actually use it as a layout.
+    type Layout<T: PyClassImpl>;
 
     /// Returns the PyTypeObject instance for this type.
     fn type_object_raw(py: Python<'_>) -> *mut ffi::PyTypeObject;

diff --git a/tests/test_class_basics.rs b/tests/test_class_basics.rs
@@ -466,10 +466,9 @@ fn no_dunder_dict_support_setattr() {
             .setattr("a", 1)
             .unwrap_err()
             .to_string();
-        assert_eq!(
-            &err,
+        assert!(err.contains(
             "AttributeError: 'builtins.NoDunderDictSupport' object has no attribute 'a'"
-        );
+        ));
     });
 }
 

diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs
@@ -23,6 +23,9 @@ fn test_compile_errors() {
     t.compile_fail("tests/ui/reject_generics.rs");
     t.compile_fail("tests/ui/deprecations.rs");
     t.compile_fail("tests/ui/invalid_closure.rs");
+    // only possible to extend variable sized types after 3.12
+    #[cfg(not(Py_3_12))]
+    t.compile_fail("tests/ui/invalid_extend_variable_sized.rs");
     t.compile_fail("tests/ui/pyclass_send.rs");
     t.compile_fail("tests/ui/invalid_argument_attributes.rs");
     t.compile_fail("tests/ui/invalid_intopy_derive.rs");

diff --git a/tests/test_inheritance.rs b/tests/test_inheritance.rs
@@ -200,11 +200,10 @@ mod inheriting_type {
             slf: Bound<'_, Metaclass>,
             _args: Bound<'_, PyTuple>,
             _kwargs: Option<Bound<'_, PyDict>>,
-        ) -> PyResult<()> {
+        ) {
             let mut slf = slf.borrow_mut();
             assert_eq!(slf.counter, 999);
             slf.counter = 5;
-            Ok(())
         }
 
         fn __getitem__(&self, item: u64) -> u64 {
@@ -283,7 +282,7 @@ mod inheriting_type {
     }
 
     #[test]
-    #[should_panic = "Metaclasses must specify __init__"]
+    #[should_panic(expected = "Metaclasses must specify __init__")]
     fn inherit_type_missing_init() {
         use pyo3::types::PyType;
 
@@ -311,7 +310,7 @@ mod inheriting_type {
     }
 
     #[test]
-    #[should_panic = "Metaclasses must not specify __new__ (use __init__ instead)"]
+    #[should_panic(expected = "Metaclasses must not specify __new__ (use __init__ instead)")]
     fn inherit_type_with_new() {
         use pyo3::types::PyType;
 
@@ -332,8 +331,7 @@ mod inheriting_type {
                 _slf: Bound<'_, MetaclassWithNew>,
                 _args: Bound<'_, PyTuple>,
                 _kwargs: Option<Bound<'_, PyDict>>,
-            ) -> PyResult<()> {
-                Ok(())
+            ) {
             }
         }
 

diff --git a/tests/ui/invalid_extend_variable_sized.rs b/tests/ui/invalid_extend_variable_sized.rs
@@ -0,0 +1,14 @@
+use pyo3::prelude::*;
+use pyo3::types::{PyDict, PyTuple, PyType};
+
+#[pyclass(extends=PyType)]
+#[derive(Default)]
+struct MyClass {}
+
+#[pymethods]
+impl MyClass {
+    #[pyo3(signature = (*_args, **_kwargs))]
+    fn __init__(&mut self, _args: &Bound<'_, PyTuple>, _kwargs: Option<&Bound<'_, PyDict>>) {}
+}
+
+fn main() {}
diff --git a/tests/ui/invalid_extend_variable_sized.stderr b/tests/ui/invalid_extend_variable_sized.stderr
@@ -0,0 +1,15 @@
+error[E0277]: the class layout is not valid
+ --> tests/ui/invalid_extend_variable_sized.rs:4:1
+  |
+4 | #[pyclass(extends=PyType)]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^ required for `#[pyclass(extends=...)]`
+  |
+  = help: the trait `pyo3::pycell::impl_::InternalPyClassObjectLayout<MyClass>` is not implemented for `PyVariableClassObject<MyClass>`
+  = note: the python version being built against influences which layouts are valid
+  = help: the trait `pyo3::pycell::impl_::InternalPyClassObjectLayout<T>` is implemented for `PyStaticClassObject<T>`
+note: required by a bound in `PyClassImpl::Layout`
+ --> src/impl_/pyclass.rs
+  |
+  |     type Layout: InternalPyClassObjectLayout<Self>;
+  |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyClassImpl::Layout`
+  = note: this error originates in the attribute macro `pyclass` (in Nightly builds, run with -Z macro-backtrace for more info)