Skip to content

Commit

Permalink
review: mejrs
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Jun 21, 2024
1 parent 49aad6c commit 6b16b4e
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 7 deletions.
3 changes: 2 additions & 1 deletion pyo3-macros-backend/src/pymethod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
87 changes: 81 additions & 6 deletions src/impl_/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
)
)]
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"),
}
}
}
1 change: 1 addition & 0 deletions src/impl_/pymethods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down

0 comments on commit 6b16b4e

Please sign in to comment.