Skip to content

Commit a1ee7f5

Browse files
authored
Fix __dict__ getset type (#6010)
1 parent cd58d15 commit a1ee7f5

File tree

2 files changed

+102
-63
lines changed

2 files changed

+102
-63
lines changed

Lib/test/test_typing.py

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2883,8 +2883,6 @@ def method(self) -> int: ...
28832883

28842884
self.assertNotIsSubclass(NotImpl, Foo)
28852885

2886-
# TODO: RUSTPYTHON
2887-
@unittest.expectedFailure
28882886
def test_pep695_generics_can_be_runtime_checkable(self):
28892887
@runtime_checkable
28902888
class HasX(Protocol):
@@ -3332,8 +3330,6 @@ class D(PNonCall): ...
33323330
with self.assertRaisesRegex(TypeError, non_callable_members_illegal):
33333331
issubclass(D, PNonCall)
33343332

3335-
# TODO: RUSTPYTHON
3336-
@unittest.expectedFailure
33373333
def test_no_weird_caching_with_issubclass_after_isinstance(self):
33383334
@runtime_checkable
33393335
class Spam(Protocol):
@@ -3400,8 +3396,6 @@ def __getattr__(self, attr):
34003396
):
34013397
issubclass(Eggs, Spam)
34023398

3403-
# TODO: RUSTPYTHON
3404-
@unittest.expectedFailure
34053399
def test_no_weird_caching_with_issubclass_after_isinstance_pep695(self):
34063400
@runtime_checkable
34073401
class Spam[T](Protocol):

vm/src/builtins/type.rs

Lines changed: 102 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,26 +1019,7 @@ impl Constructor for PyType {
10191019
attributes.insert(identifier!(vm, __hash__), vm.ctx.none.clone().into());
10201020
}
10211021

1022-
// All *classes* should have a dict. Exceptions are *instances* of
1023-
// classes that define __slots__ and instances of built-in classes
1024-
// (with exceptions, e.g function)
1025-
// Also, type subclasses don't need their own __dict__ descriptor
1026-
// since they inherit it from type
1027-
if !base_is_type {
1028-
let __dict__ = identifier!(vm, __dict__);
1029-
attributes.entry(__dict__).or_insert_with(|| {
1030-
vm.ctx
1031-
.new_static_getset(
1032-
"__dict__",
1033-
vm.ctx.types.type_type,
1034-
subtype_get_dict,
1035-
subtype_set_dict,
1036-
)
1037-
.into()
1038-
});
1039-
}
1040-
1041-
let heaptype_slots: Option<PyRef<PyTuple<PyStrRef>>> =
1022+
let (heaptype_slots, add_dict): (Option<PyRef<PyTuple<PyStrRef>>>, bool) =
10421023
if let Some(x) = attributes.get(identifier!(vm, __slots__)) {
10431024
let slots = if x.class().is(vm.ctx.types.str_type) {
10441025
let x = unsafe { x.downcast_unchecked_ref::<PyStr>() };
@@ -1055,9 +1036,26 @@ impl Constructor for PyType {
10551036
let tuple = elements.into_pytuple(vm);
10561037
tuple.try_into_typed(vm)?
10571038
};
1058-
Some(slots)
1039+
1040+
// Check if __dict__ is in slots
1041+
let dict_name = "__dict__";
1042+
let has_dict = slots.iter().any(|s| s.as_str() == dict_name);
1043+
1044+
// Filter out __dict__ from slots
1045+
let filtered_slots = if has_dict {
1046+
let filtered: Vec<PyStrRef> = slots
1047+
.iter()
1048+
.filter(|s| s.as_str() != dict_name)
1049+
.cloned()
1050+
.collect();
1051+
PyTuple::new_ref_typed(filtered, &vm.ctx)
1052+
} else {
1053+
slots
1054+
};
1055+
1056+
(Some(filtered_slots), has_dict)
10591057
} else {
1060-
None
1058+
(None, false)
10611059
};
10621060

10631061
// FIXME: this is a temporary fix. multi bases with multiple slots will break object
@@ -1070,8 +1068,10 @@ impl Constructor for PyType {
10701068
let member_count: usize = base_member_count + heaptype_member_count;
10711069

10721070
let mut flags = PyTypeFlags::heap_type_flags();
1073-
// Only add HAS_DICT and MANAGED_DICT if __slots__ is not defined.
1074-
if heaptype_slots.is_none() {
1071+
// Add HAS_DICT and MANAGED_DICT if:
1072+
// 1. __slots__ is not defined, OR
1073+
// 2. __dict__ is in __slots__
1074+
if heaptype_slots.is_none() || add_dict {
10751075
flags |= PyTypeFlags::HAS_DICT | PyTypeFlags::MANAGED_DICT;
10761076
}
10771077

@@ -1141,6 +1141,25 @@ impl Constructor for PyType {
11411141
cell.set(Some(typ.clone().into()));
11421142
};
11431143

1144+
// All *classes* should have a dict. Exceptions are *instances* of
1145+
// classes that define __slots__ and instances of built-in classes
1146+
// (with exceptions, e.g function)
1147+
// Also, type subclasses don't need their own __dict__ descriptor
1148+
// since they inherit it from type
1149+
1150+
// Add __dict__ descriptor after type creation to ensure correct __objclass__
1151+
if !base_is_type {
1152+
let __dict__ = identifier!(vm, __dict__);
1153+
if !typ.attributes.read().contains_key(&__dict__) {
1154+
unsafe {
1155+
let descriptor =
1156+
vm.ctx
1157+
.new_getset("__dict__", &typ, subtype_get_dict, subtype_set_dict);
1158+
typ.attributes.write().insert(__dict__, descriptor.into());
1159+
}
1160+
}
1161+
}
1162+
11441163
// avoid deadlock
11451164
let attributes = typ
11461165
.attributes
@@ -1446,51 +1465,77 @@ impl Representable for PyType {
14461465
}
14471466
}
14481467

1449-
fn find_base_dict_descr(cls: &Py<PyType>, vm: &VirtualMachine) -> Option<PyObjectRef> {
1450-
cls.iter_base_chain().skip(1).find_map(|cls| {
1451-
// TODO: should actually be some translation of:
1452-
// cls.slot_dictoffset != 0 && !cls.flags.contains(HEAPTYPE)
1453-
if cls.is(vm.ctx.types.type_type) {
1454-
cls.get_attr(identifier!(vm, __dict__))
1455-
} else {
1456-
None
1468+
// = get_builtin_base_with_dict
1469+
fn get_builtin_base_with_dict(typ: &Py<PyType>, vm: &VirtualMachine) -> Option<PyTypeRef> {
1470+
let mut current = Some(typ.to_owned());
1471+
while let Some(t) = current {
1472+
// In CPython: type->tp_dictoffset != 0 && !(type->tp_flags & Py_TPFLAGS_HEAPTYPE)
1473+
// Special case: type itself is a builtin with dict support
1474+
if t.is(vm.ctx.types.type_type) {
1475+
return Some(t);
1476+
}
1477+
// We check HAS_DICT flag (equivalent to tp_dictoffset != 0) and HEAPTYPE
1478+
if t.slots.flags.contains(PyTypeFlags::HAS_DICT)
1479+
&& !t.slots.flags.contains(PyTypeFlags::HEAPTYPE)
1480+
{
1481+
return Some(t);
14571482
}
1458-
})
1483+
current = t.__base__();
1484+
}
1485+
None
1486+
}
1487+
1488+
// = get_dict_descriptor
1489+
fn get_dict_descriptor(base: &Py<PyType>, vm: &VirtualMachine) -> Option<PyObjectRef> {
1490+
let dict_attr = identifier!(vm, __dict__);
1491+
// Use _PyType_Lookup (which is lookup_ref in RustPython)
1492+
base.lookup_ref(dict_attr, vm)
1493+
}
1494+
1495+
// = raise_dict_descr_error
1496+
fn raise_dict_descriptor_error(obj: &PyObject, vm: &VirtualMachine) -> PyBaseExceptionRef {
1497+
vm.new_type_error(format!(
1498+
"this __dict__ descriptor does not support '{}' objects",
1499+
obj.class().name()
1500+
))
14591501
}
14601502

14611503
fn subtype_get_dict(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult {
1462-
// TODO: obj.class().as_pyref() need to be supported
1463-
let ret = match find_base_dict_descr(obj.class(), vm) {
1464-
Some(descr) => vm.call_get_descriptor(&descr, obj).unwrap_or_else(|| {
1465-
Err(vm.new_type_error(format!(
1466-
"this __dict__ descriptor does not support '{}' objects",
1467-
descr.class()
1468-
)))
1469-
})?,
1470-
None => object::object_get_dict(obj, vm)?.into(),
1471-
};
1472-
Ok(ret)
1504+
let base = get_builtin_base_with_dict(obj.class(), vm);
1505+
1506+
if let Some(base_type) = base {
1507+
if let Some(descr) = get_dict_descriptor(&base_type, vm) {
1508+
// Call the descriptor's tp_descr_get
1509+
vm.call_get_descriptor(&descr, obj.clone())
1510+
.unwrap_or_else(|| Err(raise_dict_descriptor_error(&obj, vm)))
1511+
} else {
1512+
Err(raise_dict_descriptor_error(&obj, vm))
1513+
}
1514+
} else {
1515+
// PyObject_GenericGetDict
1516+
object::object_get_dict(obj, vm).map(Into::into)
1517+
}
14731518
}
14741519

1520+
// = subtype_setdict
14751521
fn subtype_set_dict(obj: PyObjectRef, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> {
1476-
let cls = obj.class();
1477-
match find_base_dict_descr(cls, vm) {
1478-
Some(descr) => {
1522+
let base = get_builtin_base_with_dict(obj.class(), vm);
1523+
1524+
if let Some(base_type) = base {
1525+
if let Some(descr) = get_dict_descriptor(&base_type, vm) {
1526+
// Call the descriptor's tp_descr_set
14791527
let descr_set = descr
14801528
.class()
14811529
.mro_find_map(|cls| cls.slots.descr_set.load())
1482-
.ok_or_else(|| {
1483-
vm.new_type_error(format!(
1484-
"this __dict__ descriptor does not support '{}' objects",
1485-
cls.name()
1486-
))
1487-
})?;
1530+
.ok_or_else(|| raise_dict_descriptor_error(&obj, vm))?;
14881531
descr_set(&descr, obj, PySetterValue::Assign(value), vm)
1532+
} else {
1533+
Err(raise_dict_descriptor_error(&obj, vm))
14891534
}
1490-
None => {
1491-
object::object_set_dict(obj, value.try_into_value(vm)?, vm)?;
1492-
Ok(())
1493-
}
1535+
} else {
1536+
// PyObject_GenericSetDict
1537+
object::object_set_dict(obj, value.try_into_value(vm)?, vm)?;
1538+
Ok(())
14941539
}
14951540
}
14961541

0 commit comments

Comments
 (0)