review: mejrs

Author: davidhewitt

pyo3-macros-backend/src/pymethod.rs

@@ -783,7 +783,8 @@ pub fn impl_py_getter_def(
             let method_def = quote_spanned! {ty.span()=>
                 #cfg_attrs
                 {
-                    use #pyo3_path::impl_::pyclass::Tester;
+                    #[allow(unused_imports)]  // might not be used if all probes are positve
+                    use #pyo3_path::impl_::pyclass::Probe;
 
                     struct Offset;
                     unsafe impl #pyo3_path::impl_::pyclass::OffsetCalculator<#cls, #ty> for Offset {

src/impl_/pyclass.rs

@@ -1279,7 +1279,6 @@ impl<ClassT: PyClass, FieldT: ToPyObject, Offset: OffsetCalculator<ClassT, Field
     diagnostic::on_unimplemented(
         message = "`{Self}` cannot be converted to a Python object",
         label = "required by `#[pyo3(get)]` to create a readable property from a field of type `{Self}`",
-        note = "`Py<T>` fields are always converible to Python objects",
         note = "implement `ToPyObject` or `IntoPy<PyObject> + Clone` for `{Self}` to define the conversion",
     )
 )]
@@ -1313,24 +1312,24 @@ impl<ClassT: PyClass, FieldT, Offset: OffsetCalculator<ClassT, FieldT>>
 /// The true case is defined in the zero-sized type's impl block, which is
 /// gated on some property like trait bound or only being implemented
 /// for fixed concrete types.
-pub trait Tester {
+pub trait Probe {
     const VALUE: bool = false;
 }
 
-macro_rules! tester {
+macro_rules! probe {
     ($name:ident) => {
         pub struct $name<T>(PhantomData<T>);
-        impl<T> Tester for $name<T> {}
+        impl<T> Probe for $name<T> {}
     };
 }
 
-tester!(IsPyT);
+probe!(IsPyT);
 
 impl<T> IsPyT<Py<T>> {
     pub const VALUE: bool = true;
 }
 
-tester!(IsToPyObject);
+probe!(IsToPyObject);
 
 impl<T: ToPyObject> IsToPyObject<T> {
     pub const VALUE: bool = true;
@@ -1379,3 +1378,79 @@ fn pyo3_get_value<
     // _holder is preventing mutable aliasing
     Ok((unsafe { &*value }).clone().into_py(py).into_ptr())
 }
+
+#[cfg(test)]
+#[cfg(feature = "macros")]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn get_py_for_frozen_class() {
+        #[crate::pyclass(crate = "crate", frozen)]
+        struct FrozenClass {
+            #[pyo3(get)]
+            value: Py<PyAny>,
+        }
+
+        let mut methods = Vec::new();
+        let mut slots = Vec::new();
+
+        for items in FrozenClass::items_iter() {
+            methods.extend(items.methods.iter().map(|m| match m {
+                MaybeRuntimePyMethodDef::Static(m) => m.clone(),
+                MaybeRuntimePyMethodDef::Runtime(r) => r(),
+            }));
+            slots.extend_from_slice(items.slots);
+        }
+
+        assert_eq!(methods.len(), 1);
+        assert!(slots.is_empty());
+
+        match methods.first() {
+            Some(PyMethodDefType::StructMember(member)) => {
+                assert_eq!(unsafe { CStr::from_ptr(member.name) }, ffi::c_str!("value"));
+                assert_eq!(member.type_code, ffi::Py_T_OBJECT_EX);
+                assert_eq!(
+                    member.offset,
+                    (memoffset::offset_of!(PyClassObject<FrozenClass>, contents)
+                        + memoffset::offset_of!(FrozenClass, value))
+                        as ffi::Py_ssize_t
+                );
+                assert_eq!(member.flags, ffi::Py_READONLY);
+            }
+            _ => panic!("Expected a StructMember"),
+        }
+    }
+
+    #[test]
+    fn get_py_for_non_frozen_class() {
+        #[crate::pyclass(crate = "crate")]
+        struct FrozenClass {
+            #[pyo3(get)]
+            value: Py<PyAny>,
+        }
+
+        let mut methods = Vec::new();
+        let mut slots = Vec::new();
+
+        for items in FrozenClass::items_iter() {
+            methods.extend(items.methods.iter().map(|m| match m {
+                MaybeRuntimePyMethodDef::Static(m) => m.clone(),
+                MaybeRuntimePyMethodDef::Runtime(r) => r(),
+            }));
+            slots.extend_from_slice(items.slots);
+        }
+
+        assert_eq!(methods.len(), 1);
+        assert!(slots.is_empty());
+
+        match methods.first() {
+            Some(PyMethodDefType::Getter(getter)) => {
+                assert_eq!(getter.name, ffi::c_str!("value"));
+                assert_eq!(getter.doc, ffi::c_str!(""));
+                // tests for the function pointer are in test_getter_setter.py
+            }
+            _ => panic!("Expected a StructMember"),
+        }
+    }
+}

src/impl_/pymethods.rs

@@ -52,6 +52,7 @@ impl IPowModulo {
 
 /// `PyMethodDefType` represents different types of Python callable objects.
 /// It is used by the `#[pymethods]` attribute.
+#[cfg_attr(test, derive(Clone))]
 pub enum PyMethodDefType {
     /// Represents class method
     Class(PyMethodDef),

tests/ui/invalid_property_args.stderr

@@ -53,7 +53,6 @@ error[E0277]: `PhantomData<i32>` cannot be converted to a Python object
    |            ^ required by `#[pyo3(get)]` to create a readable property from a field of type `PhantomData<i32>`
    |
    = help: the trait `IntoPy<Py<PyAny>>` is not implemented for `PhantomData<i32>`, which is required by `PhantomData<i32>: PyO3GetField`
-   = note: `Py<T>` fields are always converible to Python objects
    = note: implement `ToPyObject` or `IntoPy<PyObject> + Clone` for `PhantomData<i32>` to define the conversion
    = note: required for `PhantomData<i32>` to implement `PyO3GetField`
 note: required by a bound in `PyClassGetterGenerator::<ClassT, FieldT, Offset, false, false>::generate`