From 028c05fc17304d4756f564e4233c2df9f096501f Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 26 Dec 2025 23:49:23 +0900 Subject: [PATCH 1/3] itemsize --- crates/derive-impl/src/pyclass.rs | 20 +++++++++++++++++--- crates/vm/src/builtins/bytes.rs | 1 + crates/vm/src/builtins/int.rs | 1 + crates/vm/src/builtins/tuple.rs | 11 ++--------- crates/vm/src/builtins/type.rs | 21 +++++++++++++++------ crates/vm/src/class.rs | 2 ++ crates/vm/src/types/slot.rs | 2 +- 7 files changed, 39 insertions(+), 19 deletions(-) diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index 1a01721391e..82057d40f9e 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -165,6 +165,7 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul with_impl, with_method_defs, with_slots, + itemsize, } = extract_impl_attrs(attr, &impl_ty)?; let payload_ty = attr_payload.unwrap_or(payload_guess); let method_def = &context.method_items; @@ -189,9 +190,17 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul #(#class_extensions)* } }, - parse_quote! { - fn __extend_slots(slots: &mut ::rustpython_vm::types::PyTypeSlots) { - #slots_impl + { + let itemsize_impl = itemsize.as_ref().map(|size| { + quote! { + slots.itemsize = #size; + } + }); + parse_quote! { + fn __extend_slots(slots: &mut ::rustpython_vm::types::PyTypeSlots) { + #itemsize_impl + #slots_impl + } } }, ]; @@ -1618,6 +1627,7 @@ struct ExtractedImplAttrs { with_impl: TokenStream, with_method_defs: Vec, with_slots: TokenStream, + itemsize: Option, } fn extract_impl_attrs(attr: PunctuatedNestedMeta, item: &Ident) -> Result { @@ -1636,6 +1646,7 @@ fn extract_impl_attrs(attr: PunctuatedNestedMeta, item: &Ident) -> Result Result Result { } #[pyclass( + itemsize = 1, flags(BASETYPE, _MATCH_SELF), with( Py, diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index 8fe85267cd0..46a0ff4774d 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -320,6 +320,7 @@ impl PyInt { } #[pyclass( + itemsize = 4, flags(BASETYPE, _MATCH_SELF), with(PyRef, Comparable, Hashable, Constructor, AsNumber, Representable) )] diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index adc5b483de3..13335428b35 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -257,16 +257,9 @@ impl PyTuple> { } #[pyclass( + itemsize = std::mem::size_of::(), flags(BASETYPE, SEQUENCE, _MATCH_SELF), - with( - AsMapping, - AsSequence, - Hashable, - Comparable, - Iterable, - Constructor, - Representable - ) + with(AsMapping, AsSequence, Hashable, Comparable, Iterable, Constructor, Representable) )] impl PyTuple { #[pymethod] diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index de34678ae45..65999bfe4bd 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -948,6 +948,11 @@ impl PyType { self.slots.basicsize } + #[pygetset] + fn __itemsize__(&self) -> usize { + self.slots.itemsize + } + #[pygetset] pub fn __name__(&self, vm: &VirtualMachine) -> PyStrRef { self.name_inner( @@ -2088,12 +2093,16 @@ fn solid_base<'a>(typ: &'a Py, vm: &VirtualMachine) -> &'a Py { vm.ctx.types.object_type }; - // TODO: requires itemsize comparison too - if typ.__basicsize__() != base.__basicsize__() { - typ - } else { - base - } + // Check for extra instance variables (CPython's extra_ivars) + let t_size = typ.__basicsize__(); + let b_size = base.__basicsize__(); + let t_itemsize = typ.slots.itemsize; + let b_itemsize = base.slots.itemsize; + + // Has extra ivars if: sizes differ AND (has items OR t_size > b_size) + let has_extra_ivars = t_size != b_size && (t_itemsize > 0 || b_itemsize > 0 || t_size > b_size); + + if has_extra_ivars { typ } else { base } } fn best_base<'a>(bases: &'a [PyTypeRef], vm: &VirtualMachine) -> PyResult<&'a Py> { diff --git a/crates/vm/src/class.rs b/crates/vm/src/class.rs index da860a96289..577fd7d6844 100644 --- a/crates/vm/src/class.rs +++ b/crates/vm/src/class.rs @@ -70,6 +70,7 @@ pub trait PyClassDef { const TP_NAME: &'static str; const DOC: Option<&'static str> = None; const BASICSIZE: usize; + const ITEMSIZE: usize = 0; const UNHASHABLE: bool = false; // due to restriction of rust trait system, object.__base__ is None @@ -210,6 +211,7 @@ pub trait PyClassImpl: PyClassDef { flags: Self::TP_FLAGS, name: Self::TP_NAME, basicsize: Self::BASICSIZE, + itemsize: Self::ITEMSIZE, doc: Self::DOC, methods: Self::METHOD_DEFS, ..Default::default() diff --git a/crates/vm/src/types/slot.rs b/crates/vm/src/types/slot.rs index 13a43ed9c95..0d6b17ae99d 100644 --- a/crates/vm/src/types/slot.rs +++ b/crates/vm/src/types/slot.rs @@ -128,7 +128,7 @@ pub struct PyTypeSlots { pub(crate) name: &'static str, // tp_name with . for print, not class name pub basicsize: usize, - // tp_itemsize + pub itemsize: usize, // tp_itemsize // Methods to implement standard operations From 55f99eeb5d639db51749f6ec0a389e09d8448bfb Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Fri, 26 Dec 2025 23:52:59 +0900 Subject: [PATCH 2/3] badslot --- Lib/test/test_builtin.py | 2 - crates/vm/src/builtins/type.rs | 80 +++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index b957f79b87c..7d8c7a5e016 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -2481,8 +2481,6 @@ def test_bad_args(self): with self.assertRaises(TypeError): type('A', (int, str), {}) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_bad_slots(self): with self.assertRaises(TypeError): type('A', (), {'__slots__': b'x'}) diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 65999bfe4bd..7aa873f18d8 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -1383,11 +1383,89 @@ impl Constructor for PyType { tuple.try_into_typed(vm)? }; - // Validate that all slots are valid identifiers + // Check if base has itemsize > 0 - can't add arbitrary slots to variable-size types + // Types like int, bytes, tuple have itemsize > 0 and don't allow custom slots + // But types like weakref.ref have itemsize = 0 and DO allow slots + let has_custom_slots = slots + .iter() + .any(|s| s.as_str() != "__dict__" && s.as_str() != "__weakref__"); + if has_custom_slots && base.slots.itemsize > 0 { + return Err(vm.new_type_error(format!( + "nonempty __slots__ not supported for subtype of '{}'", + base.name() + ))); + } + + // Validate slot names and track duplicates + let mut seen_dict = false; + let mut seen_weakref = false; for slot in slots.iter() { + // Use isidentifier for validation (handles Unicode properly) if !slot.isidentifier() { return Err(vm.new_type_error("__slots__ must be identifiers".to_owned())); } + + let slot_name = slot.as_str(); + + // Check for duplicate __dict__ + if slot_name == "__dict__" { + if seen_dict { + return Err(vm.new_type_error( + "__dict__ slot disallowed: we already got one".to_owned(), + )); + } + seen_dict = true; + } + + // Check for duplicate __weakref__ + if slot_name == "__weakref__" { + if seen_weakref { + return Err(vm.new_type_error( + "__weakref__ slot disallowed: we already got one".to_owned(), + )); + } + seen_weakref = true; + } + + // Check if slot name conflicts with class attributes + if attributes.contains_key(vm.ctx.intern_str(slot_name)) { + return Err(vm.new_value_error(format!( + "'{}' in __slots__ conflicts with a class variable", + slot_name + ))); + } + } + + // Check if base class already has __dict__ - can't redefine it + if seen_dict && base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) { + return Err( + vm.new_type_error("__dict__ slot disallowed: we already got one".to_owned()) + ); + } + + // Check if base class already has __weakref__ - can't redefine it + // A base has weakref support if: + // 1. It's a heap type without explicit __slots__ (automatic weakref), OR + // 2. It's a heap type with __weakref__ in its __slots__ + if seen_weakref { + let base_has_weakref = if let Some(ref ext) = base.heaptype_ext { + match &ext.slots { + // Heap type without __slots__ - has automatic weakref + None => true, + // Heap type with __slots__ - check if __weakref__ is in slots + Some(base_slots) => base_slots.iter().any(|s| s.as_str() == "__weakref__"), + } + } else { + // Builtin type - check if it has __weakref__ descriptor + let weakref_name = vm.ctx.intern_str("__weakref__"); + base.attributes.read().contains_key(weakref_name) + }; + + if base_has_weakref { + return Err(vm.new_type_error( + "__weakref__ slot disallowed: we already got one".to_owned(), + )); + } } // Check if __dict__ is in slots From 8a89e3c9a61387f47adeab444fbf590c6874e113 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Sat, 27 Dec 2025 15:39:04 +0000 Subject: [PATCH 3/3] Auto-format: cargo fmt --all --- crates/vm/src/builtins/type.rs | 229 +++++++++++++++++---------------- 1 file changed, 115 insertions(+), 114 deletions(-) diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 7aa873f18d8..b2e866b8cf1 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -1352,143 +1352,144 @@ impl Constructor for PyType { attributes.insert(identifier!(vm, __hash__), vm.ctx.none.clone().into()); } - let (heaptype_slots, add_dict): (Option>>, bool) = - if let Some(x) = attributes.get(identifier!(vm, __slots__)) { - // Check if __slots__ is bytes - not allowed - if x.class().is(vm.ctx.types.bytes_type) { - return Err(vm.new_type_error( - "__slots__ items must be strings, not 'bytes'".to_owned(), - )); - } - - let slots = if x.class().is(vm.ctx.types.str_type) { - let x = unsafe { x.downcast_unchecked_ref::() }; - PyTuple::new_ref_typed(vec![x.to_owned()], &vm.ctx) - } else { - let iter = x.get_iter(vm)?; - let elements = { - let mut elements = Vec::new(); - while let PyIterReturn::Return(element) = iter.next(vm)? { - // Check if any slot item is bytes - if element.class().is(vm.ctx.types.bytes_type) { - return Err(vm.new_type_error( - "__slots__ items must be strings, not 'bytes'".to_owned(), - )); - } - elements.push(element); - } - elements - }; - let tuple = elements.into_pytuple(vm); - tuple.try_into_typed(vm)? - }; - - // Check if base has itemsize > 0 - can't add arbitrary slots to variable-size types - // Types like int, bytes, tuple have itemsize > 0 and don't allow custom slots - // But types like weakref.ref have itemsize = 0 and DO allow slots - let has_custom_slots = slots - .iter() - .any(|s| s.as_str() != "__dict__" && s.as_str() != "__weakref__"); - if has_custom_slots && base.slots.itemsize > 0 { - return Err(vm.new_type_error(format!( - "nonempty __slots__ not supported for subtype of '{}'", - base.name() - ))); - } - - // Validate slot names and track duplicates - let mut seen_dict = false; - let mut seen_weakref = false; - for slot in slots.iter() { - // Use isidentifier for validation (handles Unicode properly) - if !slot.isidentifier() { - return Err(vm.new_type_error("__slots__ must be identifiers".to_owned())); - } - - let slot_name = slot.as_str(); + let (heaptype_slots, add_dict): (Option>>, bool) = if let Some(x) = + attributes.get(identifier!(vm, __slots__)) + { + // Check if __slots__ is bytes - not allowed + if x.class().is(vm.ctx.types.bytes_type) { + return Err( + vm.new_type_error("__slots__ items must be strings, not 'bytes'".to_owned()) + ); + } - // Check for duplicate __dict__ - if slot_name == "__dict__" { - if seen_dict { + let slots = if x.class().is(vm.ctx.types.str_type) { + let x = unsafe { x.downcast_unchecked_ref::() }; + PyTuple::new_ref_typed(vec![x.to_owned()], &vm.ctx) + } else { + let iter = x.get_iter(vm)?; + let elements = { + let mut elements = Vec::new(); + while let PyIterReturn::Return(element) = iter.next(vm)? { + // Check if any slot item is bytes + if element.class().is(vm.ctx.types.bytes_type) { return Err(vm.new_type_error( - "__dict__ slot disallowed: we already got one".to_owned(), + "__slots__ items must be strings, not 'bytes'".to_owned(), )); } - seen_dict = true; + elements.push(element); } + elements + }; + let tuple = elements.into_pytuple(vm); + tuple.try_into_typed(vm)? + }; - // Check for duplicate __weakref__ - if slot_name == "__weakref__" { - if seen_weakref { - return Err(vm.new_type_error( - "__weakref__ slot disallowed: we already got one".to_owned(), - )); - } - seen_weakref = true; - } + // Check if base has itemsize > 0 - can't add arbitrary slots to variable-size types + // Types like int, bytes, tuple have itemsize > 0 and don't allow custom slots + // But types like weakref.ref have itemsize = 0 and DO allow slots + let has_custom_slots = slots + .iter() + .any(|s| s.as_str() != "__dict__" && s.as_str() != "__weakref__"); + if has_custom_slots && base.slots.itemsize > 0 { + return Err(vm.new_type_error(format!( + "nonempty __slots__ not supported for subtype of '{}'", + base.name() + ))); + } - // Check if slot name conflicts with class attributes - if attributes.contains_key(vm.ctx.intern_str(slot_name)) { - return Err(vm.new_value_error(format!( - "'{}' in __slots__ conflicts with a class variable", - slot_name - ))); - } + // Validate slot names and track duplicates + let mut seen_dict = false; + let mut seen_weakref = false; + for slot in slots.iter() { + // Use isidentifier for validation (handles Unicode properly) + if !slot.isidentifier() { + return Err(vm.new_type_error("__slots__ must be identifiers".to_owned())); } - // Check if base class already has __dict__ - can't redefine it - if seen_dict && base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) { - return Err( - vm.new_type_error("__dict__ slot disallowed: we already got one".to_owned()) - ); - } + let slot_name = slot.as_str(); - // Check if base class already has __weakref__ - can't redefine it - // A base has weakref support if: - // 1. It's a heap type without explicit __slots__ (automatic weakref), OR - // 2. It's a heap type with __weakref__ in its __slots__ - if seen_weakref { - let base_has_weakref = if let Some(ref ext) = base.heaptype_ext { - match &ext.slots { - // Heap type without __slots__ - has automatic weakref - None => true, - // Heap type with __slots__ - check if __weakref__ is in slots - Some(base_slots) => base_slots.iter().any(|s| s.as_str() == "__weakref__"), - } - } else { - // Builtin type - check if it has __weakref__ descriptor - let weakref_name = vm.ctx.intern_str("__weakref__"); - base.attributes.read().contains_key(weakref_name) - }; + // Check for duplicate __dict__ + if slot_name == "__dict__" { + if seen_dict { + return Err(vm.new_type_error( + "__dict__ slot disallowed: we already got one".to_owned(), + )); + } + seen_dict = true; + } - if base_has_weakref { + // Check for duplicate __weakref__ + if slot_name == "__weakref__" { + if seen_weakref { return Err(vm.new_type_error( "__weakref__ slot disallowed: we already got one".to_owned(), )); } + seen_weakref = true; } - // Check if __dict__ is in slots - let dict_name = "__dict__"; - let has_dict = slots.iter().any(|s| s.as_str() == dict_name); - - // Filter out __dict__ from slots - let filtered_slots = if has_dict { - let filtered: Vec = slots - .iter() - .filter(|s| s.as_str() != dict_name) - .cloned() - .collect(); - PyTuple::new_ref_typed(filtered, &vm.ctx) + // Check if slot name conflicts with class attributes + if attributes.contains_key(vm.ctx.intern_str(slot_name)) { + return Err(vm.new_value_error(format!( + "'{}' in __slots__ conflicts with a class variable", + slot_name + ))); + } + } + + // Check if base class already has __dict__ - can't redefine it + if seen_dict && base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) { + return Err( + vm.new_type_error("__dict__ slot disallowed: we already got one".to_owned()) + ); + } + + // Check if base class already has __weakref__ - can't redefine it + // A base has weakref support if: + // 1. It's a heap type without explicit __slots__ (automatic weakref), OR + // 2. It's a heap type with __weakref__ in its __slots__ + if seen_weakref { + let base_has_weakref = if let Some(ref ext) = base.heaptype_ext { + match &ext.slots { + // Heap type without __slots__ - has automatic weakref + None => true, + // Heap type with __slots__ - check if __weakref__ is in slots + Some(base_slots) => base_slots.iter().any(|s| s.as_str() == "__weakref__"), + } } else { - slots + // Builtin type - check if it has __weakref__ descriptor + let weakref_name = vm.ctx.intern_str("__weakref__"); + base.attributes.read().contains_key(weakref_name) }; - (Some(filtered_slots), has_dict) + if base_has_weakref { + return Err(vm.new_type_error( + "__weakref__ slot disallowed: we already got one".to_owned(), + )); + } + } + + // Check if __dict__ is in slots + let dict_name = "__dict__"; + let has_dict = slots.iter().any(|s| s.as_str() == dict_name); + + // Filter out __dict__ from slots + let filtered_slots = if has_dict { + let filtered: Vec = slots + .iter() + .filter(|s| s.as_str() != dict_name) + .cloned() + .collect(); + PyTuple::new_ref_typed(filtered, &vm.ctx) } else { - (None, false) + slots }; + (Some(filtered_slots), has_dict) + } else { + (None, false) + }; + // FIXME: this is a temporary fix. multi bases with multiple slots will break object let base_member_count = bases .iter()