Skip to content

Commit 7edc3bb

Browse files
youknowoneyouknowone
authored
tp_itemsize (#6544)
1 parent 012799f commit 7edc3bb

File tree

8 files changed

+164
-67
lines changed

8 files changed

+164
-67
lines changed

‎Lib/test/test_builtin.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,8 +2481,6 @@ def test_bad_args(self):
24812481
with self.assertRaises(TypeError):
24822482
type('A', (int, str), {})
24832483

2484-
# TODO: RUSTPYTHON
2485-
@unittest.expectedFailure
24862484
def test_bad_slots(self):
24872485
with self.assertRaises(TypeError):
24882486
type('A', (), {'__slots__': b'x'})

‎crates/derive-impl/src/pyclass.rs‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul
165165
with_impl,
166166
with_method_defs,
167167
with_slots,
168+
itemsize,
168169
} = extract_impl_attrs(attr, &impl_ty)?;
169170
let payload_ty = attr_payload.unwrap_or(payload_guess);
170171
let method_def = &context.method_items;
@@ -189,9 +190,17 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul
189190
#(#class_extensions)*
190191
}
191192
},
192-
parse_quote! {
193-
fn __extend_slots(slots: &mut ::rustpython_vm::types::PyTypeSlots) {
194-
#slots_impl
193+
{
194+
let itemsize_impl = itemsize.as_ref().map(|size| {
195+
quote! {
196+
slots.itemsize = #size;
197+
}
198+
});
199+
parse_quote! {
200+
fn __extend_slots(slots: &mut ::rustpython_vm::types::PyTypeSlots) {
201+
#itemsize_impl
202+
#slots_impl
203+
}
195204
}
196205
},
197206
];
@@ -1618,6 +1627,7 @@ struct ExtractedImplAttrs {
16181627
with_impl: TokenStream,
16191628
with_method_defs: Vec<TokenStream>,
16201629
with_slots: TokenStream,
1630+
itemsize: Option<syn::Expr>,
16211631
}
16221632

16231633
fn extract_impl_attrs(attr: PunctuatedNestedMeta, item: &Ident) -> Result<ExtractedImplAttrs> {
@@ -1636,6 +1646,7 @@ fn extract_impl_attrs(attr: PunctuatedNestedMeta, item: &Ident) -> Result<Extrac
16361646
}
16371647
}];
16381648
let mut payload = None;
1649+
let mut itemsize = None;
16391650

16401651
for attr in attr {
16411652
match attr {
@@ -1721,6 +1732,8 @@ fn extract_impl_attrs(attr: PunctuatedNestedMeta, item: &Ident) -> Result<Extrac
17211732
} else {
17221733
bail_span!(value, "payload must be a string literal")
17231734
}
1735+
} else if path.is_ident("itemsize") {
1736+
itemsize = Some(value);
17241737
} else {
17251738
bail_span!(path, "Unknown pyimpl attribute")
17261739
}
@@ -1741,6 +1754,7 @@ fn extract_impl_attrs(attr: PunctuatedNestedMeta, item: &Ident) -> Result<Extrac
17411754
with_slots: quote! {
17421755
#(#with_slots)*
17431756
},
1757+
itemsize,
17441758
})
17451759
}
17461760

‎crates/vm/src/builtins/bytes.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ impl PyRef<PyBytes> {
187187
}
188188

189189
#[pyclass(
190+
itemsize = 1,
190191
flags(BASETYPE, _MATCH_SELF),
191192
with(
192193
Py,

‎crates/vm/src/builtins/int.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,7 @@ impl PyInt {
320320
}
321321

322322
#[pyclass(
323+
itemsize = 4,
323324
flags(BASETYPE, _MATCH_SELF),
324325
with(PyRef, Comparable, Hashable, Constructor, AsNumber, Representable)
325326
)]

‎crates/vm/src/builtins/tuple.rs‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -257,16 +257,9 @@ impl<T> PyTuple<PyRef<T>> {
257257
}
258258

259259
#[pyclass(
260+
itemsize = std::mem::size_of::<crate::PyObjectRef>(),
260261
flags(BASETYPE, SEQUENCE, _MATCH_SELF),
261-
with(
262-
AsMapping,
263-
AsSequence,
264-
Hashable,
265-
Comparable,
266-
Iterable,
267-
Constructor,
268-
Representable
269-
)
262+
with(AsMapping, AsSequence, Hashable, Comparable, Iterable, Constructor, Representable)
270263
)]
271264
impl PyTuple {
272265
#[pymethod]

‎crates/vm/src/builtins/type.rs‎

Lines changed: 140 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,11 @@ impl PyType {
948948
self.slots.basicsize
949949
}
950950

951+
#[pygetset]
952+
fn __itemsize__(&self) -> usize {
953+
self.slots.itemsize
954+
}
955+
951956
#[pygetset]
952957
pub fn __name__(&self, vm: &VirtualMachine) -> PyStrRef {
953958
self.name_inner(
@@ -1347,65 +1352,144 @@ impl Constructor for PyType {
13471352
attributes.insert(identifier!(vm, __hash__), vm.ctx.none.clone().into());
13481353
}
13491354

1350-
let (heaptype_slots, add_dict): (Option<PyRef<PyTuple<PyStrRef>>>, bool) =
1351-
if let Some(x) = attributes.get(identifier!(vm, __slots__)) {
1352-
// Check if __slots__ is bytes - not allowed
1353-
if x.class().is(vm.ctx.types.bytes_type) {
1354-
return Err(vm.new_type_error(
1355-
"__slots__ items must be strings, not 'bytes'".to_owned(),
1356-
));
1357-
}
1355+
let (heaptype_slots, add_dict): (Option<PyRef<PyTuple<PyStrRef>>>, bool) = if let Some(x) =
1356+
attributes.get(identifier!(vm, __slots__))
1357+
{
1358+
// Check if __slots__ is bytes - not allowed
1359+
if x.class().is(vm.ctx.types.bytes_type) {
1360+
return Err(
1361+
vm.new_type_error("__slots__ items must be strings, not 'bytes'".to_owned())
1362+
);
1363+
}
13581364

1359-
let slots = if x.class().is(vm.ctx.types.str_type) {
1360-
let x = unsafe { x.downcast_unchecked_ref::<PyStr>() };
1361-
PyTuple::new_ref_typed(vec![x.to_owned()], &vm.ctx)
1362-
} else {
1363-
let iter = x.get_iter(vm)?;
1364-
let elements = {
1365-
let mut elements = Vec::new();
1366-
while let PyIterReturn::Return(element) = iter.next(vm)? {
1367-
// Check if any slot item is bytes
1368-
if element.class().is(vm.ctx.types.bytes_type) {
1369-
return Err(vm.new_type_error(
1370-
"__slots__ items must be strings, not 'bytes'".to_owned(),
1371-
));
1372-
}
1373-
elements.push(element);
1365+
let slots = if x.class().is(vm.ctx.types.str_type) {
1366+
let x = unsafe { x.downcast_unchecked_ref::<PyStr>() };
1367+
PyTuple::new_ref_typed(vec![x.to_owned()], &vm.ctx)
1368+
} else {
1369+
let iter = x.get_iter(vm)?;
1370+
let elements = {
1371+
let mut elements = Vec::new();
1372+
while let PyIterReturn::Return(element) = iter.next(vm)? {
1373+
// Check if any slot item is bytes
1374+
if element.class().is(vm.ctx.types.bytes_type) {
1375+
return Err(vm.new_type_error(
1376+
"__slots__ items must be strings, not 'bytes'".to_owned(),
1377+
));
13741378
}
1375-
elements
1376-
};
1377-
let tuple = elements.into_pytuple(vm);
1378-
tuple.try_into_typed(vm)?
1379+
elements.push(element);
1380+
}
1381+
elements
13791382
};
1383+
let tuple = elements.into_pytuple(vm);
1384+
tuple.try_into_typed(vm)?
1385+
};
1386+
1387+
// Check if base has itemsize > 0 - can't add arbitrary slots to variable-size types
1388+
// Types like int, bytes, tuple have itemsize > 0 and don't allow custom slots
1389+
// But types like weakref.ref have itemsize = 0 and DO allow slots
1390+
let has_custom_slots = slots
1391+
.iter()
1392+
.any(|s| s.as_str() != "__dict__" && s.as_str() != "__weakref__");
1393+
if has_custom_slots && base.slots.itemsize > 0 {
1394+
return Err(vm.new_type_error(format!(
1395+
"nonempty __slots__ not supported for subtype of '{}'",
1396+
base.name()
1397+
)));
1398+
}
13801399

1381-
// Validate that all slots are valid identifiers
1382-
for slot in slots.iter() {
1383-
if !slot.isidentifier() {
1384-
return Err(vm.new_type_error("__slots__ must be identifiers".to_owned()));
1400+
// Validate slot names and track duplicates
1401+
let mut seen_dict = false;
1402+
let mut seen_weakref = false;
1403+
for slot in slots.iter() {
1404+
// Use isidentifier for validation (handles Unicode properly)
1405+
if !slot.isidentifier() {
1406+
return Err(vm.new_type_error("__slots__ must be identifiers".to_owned()));
1407+
}
1408+
1409+
let slot_name = slot.as_str();
1410+
1411+
// Check for duplicate __dict__
1412+
if slot_name == "__dict__" {
1413+
if seen_dict {
1414+
return Err(vm.new_type_error(
1415+
"__dict__ slot disallowed: we already got one".to_owned(),
1416+
));
13851417
}
1418+
seen_dict = true;
13861419
}
13871420

1388-
// Check if __dict__ is in slots
1389-
let dict_name = "__dict__";
1390-
let has_dict = slots.iter().any(|s| s.as_str() == dict_name);
1391-
1392-
// Filter out __dict__ from slots
1393-
let filtered_slots = if has_dict {
1394-
let filtered: Vec<PyStrRef> = slots
1395-
.iter()
1396-
.filter(|s| s.as_str() != dict_name)
1397-
.cloned()
1398-
.collect();
1399-
PyTuple::new_ref_typed(filtered, &vm.ctx)
1421+
// Check for duplicate __weakref__
1422+
if slot_name == "__weakref__" {
1423+
if seen_weakref {
1424+
return Err(vm.new_type_error(
1425+
"__weakref__ slot disallowed: we already got one".to_owned(),
1426+
));
1427+
}
1428+
seen_weakref = true;
1429+
}
1430+
1431+
// Check if slot name conflicts with class attributes
1432+
if attributes.contains_key(vm.ctx.intern_str(slot_name)) {
1433+
return Err(vm.new_value_error(format!(
1434+
"'{}' in __slots__ conflicts with a class variable",
1435+
slot_name
1436+
)));
1437+
}
1438+
}
1439+
1440+
// Check if base class already has __dict__ - can't redefine it
1441+
if seen_dict && base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
1442+
return Err(
1443+
vm.new_type_error("__dict__ slot disallowed: we already got one".to_owned())
1444+
);
1445+
}
1446+
1447+
// Check if base class already has __weakref__ - can't redefine it
1448+
// A base has weakref support if:
1449+
// 1. It's a heap type without explicit __slots__ (automatic weakref), OR
1450+
// 2. It's a heap type with __weakref__ in its __slots__
1451+
if seen_weakref {
1452+
let base_has_weakref = if let Some(ref ext) = base.heaptype_ext {
1453+
match &ext.slots {
1454+
// Heap type without __slots__ - has automatic weakref
1455+
None => true,
1456+
// Heap type with __slots__ - check if __weakref__ is in slots
1457+
Some(base_slots) => base_slots.iter().any(|s| s.as_str() == "__weakref__"),
1458+
}
14001459
} else {
1401-
slots
1460+
// Builtin type - check if it has __weakref__ descriptor
1461+
let weakref_name = vm.ctx.intern_str("__weakref__");
1462+
base.attributes.read().contains_key(weakref_name)
14021463
};
14031464

1404-
(Some(filtered_slots), has_dict)
1465+
if base_has_weakref {
1466+
return Err(vm.new_type_error(
1467+
"__weakref__ slot disallowed: we already got one".to_owned(),
1468+
));
1469+
}
1470+
}
1471+
1472+
// Check if __dict__ is in slots
1473+
let dict_name = "__dict__";
1474+
let has_dict = slots.iter().any(|s| s.as_str() == dict_name);
1475+
1476+
// Filter out __dict__ from slots
1477+
let filtered_slots = if has_dict {
1478+
let filtered: Vec<PyStrRef> = slots
1479+
.iter()
1480+
.filter(|s| s.as_str() != dict_name)
1481+
.cloned()
1482+
.collect();
1483+
PyTuple::new_ref_typed(filtered, &vm.ctx)
14051484
} else {
1406-
(None, false)
1485+
slots
14071486
};
14081487

1488+
(Some(filtered_slots), has_dict)
1489+
} else {
1490+
(None, false)
1491+
};
1492+
14091493
// FIXME: this is a temporary fix. multi bases with multiple slots will break object
14101494
let base_member_count = bases
14111495
.iter()
@@ -2094,12 +2178,16 @@ fn solid_base<'a>(typ: &'a Py<PyType>, vm: &VirtualMachine) -> &'a Py<PyType> {
20942178
vm.ctx.types.object_type
20952179
};
20962180

2097-
// TODO: requires itemsize comparison too
2098-
if typ.__basicsize__() != base.__basicsize__() {
2099-
typ
2100-
} else {
2101-
base
2102-
}
2181+
// Check for extra instance variables (CPython's extra_ivars)
2182+
let t_size = typ.__basicsize__();
2183+
let b_size = base.__basicsize__();
2184+
let t_itemsize = typ.slots.itemsize;
2185+
let b_itemsize = base.slots.itemsize;
2186+
2187+
// Has extra ivars if: sizes differ AND (has items OR t_size > b_size)
2188+
let has_extra_ivars = t_size != b_size && (t_itemsize > 0 || b_itemsize > 0 || t_size > b_size);
2189+
2190+
if has_extra_ivars { typ } else { base }
21032191
}
21042192

21052193
fn best_base<'a>(bases: &'a [PyTypeRef], vm: &VirtualMachine) -> PyResult<&'a Py<PyType>> {

‎crates/vm/src/class.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ pub trait PyClassDef {
7070
const TP_NAME: &'static str;
7171
const DOC: Option<&'static str> = None;
7272
const BASICSIZE: usize;
73+
const ITEMSIZE: usize = 0;
7374
const UNHASHABLE: bool = false;
7475

7576
// due to restriction of rust trait system, object.__base__ is None
@@ -210,6 +211,7 @@ pub trait PyClassImpl: PyClassDef {
210211
flags: Self::TP_FLAGS,
211212
name: Self::TP_NAME,
212213
basicsize: Self::BASICSIZE,
214+
itemsize: Self::ITEMSIZE,
213215
doc: Self::DOC,
214216
methods: Self::METHOD_DEFS,
215217
..Default::default()

‎crates/vm/src/types/slot.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ pub struct PyTypeSlots {
128128
pub(crate) name: &'static str, // tp_name with <module>.<class> for print, not class name
129129

130130
pub basicsize: usize,
131-
// tp_itemsize
131+
pub itemsize: usize, // tp_itemsize
132132

133133
// Methods to implement standard operations
134134

0 commit comments

Comments
 (0)