Skip to content

Commit

Permalink
fix __dict__ on Python 3.9 with limited API
Browse files Browse the repository at this point in the history
  • Loading branch information
davidhewitt committed Jun 14, 2024
1 parent f66124a commit 81c3ef6
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 27 deletions.
1 change: 1 addition & 0 deletions newsfragments/4251.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix `__dict__` attribute missing for `#[pyclass(dict)]` instances when building for `abi3` on Python 3.9.
12 changes: 12 additions & 0 deletions pytests/src/pyclasses.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,24 @@ impl AssertingBaseClass {
#[pyclass]
struct ClassWithoutConstructor;

#[pyclass(dict, weakref)]
struct ClassWithDictAndWeakref;

#[pymethods]
impl ClassWithDictAndWeakref {
#[new]
fn new() -> Self {
ClassWithDictAndWeakref
}
}

#[pymodule]
pub fn pyclasses(m: &Bound<'_, PyModule>) -> PyResult<()> {
m.add_class::<EmptyClass>()?;
m.add_class::<PyClassIter>()?;
m.add_class::<AssertingBaseClass>()?;
m.add_class::<ClassWithoutConstructor>()?;
m.add_class::<ClassWithDictAndWeakref>()?;

Ok(())
}
8 changes: 8 additions & 0 deletions pytests/tests/test_pyclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,3 +84,11 @@ def test_no_constructor_defined_propagates_cause(cls: Type):
assert exc_info.type is TypeError
assert exc_info.value.args == ("No constructor defined",)
assert exc_info.value.__context__ is original_error


def test_dict():
d = pyclasses.ClassWithDictAndWeakref()
assert d.__dict__ == {}

d.foo = 42
assert d.__dict__ == {"foo": 42}
72 changes: 52 additions & 20 deletions src/pyclass/create_type_object.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use pyo3_ffi::PyType_IS_GC;

use crate::{
exceptions::PyTypeError,
ffi,
Expand Down Expand Up @@ -68,7 +66,7 @@ where
has_setitem: false,
has_traverse: false,
has_clear: false,
has_dict: false,
dictoffset: None,
class_flags: 0,
#[cfg(all(not(Py_3_9), not(Py_LIMITED_API)))]
buffer_procs: Default::default(),
Expand Down Expand Up @@ -121,7 +119,7 @@ struct PyTypeBuilder {
has_setitem: bool,
has_traverse: bool,
has_clear: bool,
has_dict: bool,
dictoffset: Option<ffi::Py_ssize_t>,
class_flags: c_ulong,
// Before Python 3.9, need to patch in buffer methods manually (they don't work in slots)
#[cfg(all(not(Py_3_9), not(Py_LIMITED_API)))]
Expand Down Expand Up @@ -218,16 +216,53 @@ impl PyTypeBuilder {
})
.collect::<PyResult<_>>()?;

// PyPy doesn't automatically add __dict__ getter / setter.
// PyObject_GenericGetDict not in the limited API until Python 3.10.
if self.has_dict {
#[cfg(not(any(PyPy, all(Py_LIMITED_API, not(Py_3_10)))))]
// PyPy automatically adds __dict__ getter / setter.
#[cfg(not(PyPy))]
if let Some(dictoffset) = self.dictoffset {
let get_dict;
let closure;
// PyObject_GenericGetDict not in the limited API until Python 3.10.
#[cfg(any(not(Py_LIMITED_API), Py_3_10))]
{
let _ = dictoffset;
get_dict = ffi::PyObject_GenericGetDict;
closure = ptr::null_mut();
}

// ... so we write a basic implementation ourselves
#[cfg(not(any(not(Py_LIMITED_API), Py_3_10)))]
{
extern "C" fn get_dict_impl(
object: *mut ffi::PyObject,
closure: *mut c_void,
) -> *mut ffi::PyObject {
unsafe {
trampoline(|_| {
let dictoffset = closure as isize;
// we don't support negative dictoffset here; PyO3 doesn't set it negative
assert!(dictoffset > 0);
let dict_ptr = object
.cast::<u8>()
.offset(dictoffset)
.cast::<*mut ffi::PyObject>();
if (*dict_ptr).is_null() {
std::ptr::write(dict_ptr, ffi::PyDict_New());
}
Ok(ffi::_Py_XNewRef(*dict_ptr))
})
}
}

get_dict = get_dict_impl;
closure = dictoffset as _;
}

property_defs.push(ffi::PyGetSetDef {
name: "__dict__\0".as_ptr().cast(),
get: Some(ffi::PyObject_GenericGetDict),
get: Some(get_dict),
set: Some(ffi::PyObject_GenericSetDict),
doc: ptr::null(),
closure: ptr::null_mut(),
closure,
});
}

Expand Down Expand Up @@ -315,20 +350,17 @@ impl PyTypeBuilder {
dict_offset: Option<ffi::Py_ssize_t>,
#[allow(unused_variables)] weaklist_offset: Option<ffi::Py_ssize_t>,
) -> Self {
self.has_dict = dict_offset.is_some();
self.dictoffset = dict_offset;

#[cfg(Py_3_9)]
{
#[inline(always)]
fn offset_def(
name: &'static str,
offset: ffi::Py_ssize_t,
) -> ffi::structmember::PyMemberDef {
ffi::structmember::PyMemberDef {
name: name.as_ptr() as _,
type_code: ffi::structmember::T_PYSSIZET,
fn offset_def(name: &'static str, offset: ffi::Py_ssize_t) -> ffi::PyMemberDef {
ffi::PyMemberDef {
name: name.as_ptr().cast(),
type_code: ffi::Py_T_PYSSIZET,
offset,
flags: ffi::structmember::READONLY,
flags: ffi::Py_READONLY,
doc: std::ptr::null_mut(),
}
}
Expand Down Expand Up @@ -391,7 +423,7 @@ impl PyTypeBuilder {
unsafe { self.push_slot(ffi::Py_tp_new, no_constructor_defined as *mut c_void) }
}

let tp_dealloc = if self.has_traverse || unsafe { PyType_IS_GC(self.tp_base) == 1 } {
let tp_dealloc = if self.has_traverse || unsafe { ffi::PyType_IS_GC(self.tp_base) == 1 } {
self.tp_dealloc_with_gc
} else {
self.tp_dealloc
Expand Down
13 changes: 6 additions & 7 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@ struct DunderDictSupport {
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn dunder_dict_support() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -456,9 +456,8 @@ fn dunder_dict_support() {
});
}

// Accessing inst.__dict__ only supported in limited API from Python 3.10
#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_10)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn access_dunder_dict() {
Python::with_gil(|py| {
let inst = Py::new(
Expand Down Expand Up @@ -486,7 +485,7 @@ struct InheritDict {
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn inherited_dict() {
Python::with_gil(|py| {
let inst = Py::new(
Expand Down Expand Up @@ -517,7 +516,7 @@ struct WeakRefDunderDictSupport {
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn weakref_dunder_dict_support() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -541,7 +540,7 @@ struct WeakRefSupport {
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn weakref_support() {
Python::with_gil(|py| {
let inst = Py::new(
Expand All @@ -566,7 +565,7 @@ struct InheritWeakRef {
}

#[test]
#[cfg_attr(all(Py_LIMITED_API, not(Py_3_9)), ignore)]
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
fn inherited_weakref() {
Python::with_gil(|py| {
let inst = Py::new(
Expand Down

0 comments on commit 81c3ef6

Please sign in to comment.