luxolus
/
pyo3
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix `__dict__` on Python 3.9 with limited API (#4251) 

* fix `__dict__` on Python 3.9 with limited API

* [review] Icxolu suggestions

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>

* [review] Icxolu

* missing import

---------

Co-authored-by: Icxolu <10486322+Icxolu@users.noreply.github.com>
This commit is contained in:
David Hewitt 2024-06-16 08:57:44 +01:00 committed by GitHub
parent 591cdb0bf8
commit 0b2f19b3c9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 82 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
newsfragments/4251.fixed.md Normal file
View File

@ -0,0 +1 @@
Fix `__dict__` attribute missing for `#[pyclass(dict)]` instances when building for `abi3` on Python 3.9.

12
pytests/src/pyclasses.rs
View File

@ -66,12 +66,24 @@ impl AssertingBaseClass {
#[pyclass]
struct ClassWithoutConstructor;
#[pyclass(dict)]
struct ClassWithDict;
#[pymethods]
impl ClassWithDict {
#[new]
fn new() -> Self {
ClassWithDict
}
}
#[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::<ClassWithDict>()?;
Ok(())
}

8
pytests/tests/test_pyclasses.py
View File

@ -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.ClassWithDict()
assert d.__dict__ == {}
d.foo = 42
assert d.__dict__ == {"foo": 42}

75
src/pyclass/create_type_object.rs
View File

@ -1,5 +1,3 @@
use pyo3_ffi::PyType_IS_GC;
use crate::{
exceptions::PyTypeError,
ffi,
@ -68,7 +66,7 @@ where
has_setitem: false,
has_traverse: false,
has_clear: false,
has_dict: false,
dict_offset: None,
class_flags: 0,
#[cfg(all(not(Py_3_9), not(Py_LIMITED_API)))]
buffer_procs: Default::default(),
@ -121,7 +119,7 @@ struct PyTypeBuilder {
has_setitem: bool,
has_traverse: bool,
has_clear: bool,
has_dict: bool,
dict_offset: 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)))]
@ -218,16 +216,56 @@ 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))]
// Supported on unlimited API for all versions, and on 3.9+ for limited API
#[cfg(any(Py_3_9, not(Py_LIMITED_API)))]
if let Some(dict_offset) = self.dict_offset {
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 _ = dict_offset;
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 dict_offset = closure as ffi::Py_ssize_t;
// we don't support negative dict_offset here; PyO3 doesn't set it negative
assert!(dict_offset > 0);
// TODO: use `.byte_offset` on MSRV 1.75
let dict_ptr = object
.cast::<u8>()
.offset(dict_offset)
.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 = dict_offset 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,
});
}
@ -315,20 +353,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.dict_offset = 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(),
}
}
@ -391,7 +426,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

13
tests/test_class_basics.rs
View File

@ -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(
@ -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(
@ -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(
@ -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(
@ -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(
@ -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(