Skip to content

Commit b98f062

Browse files
youknowoneyouknowone
authored
Update issubclass and make mro representation fit to CPython (#5866)
1 parent 9fcc471 commit b98f062

File tree

4 files changed

+39
-37
lines changed

4 files changed

+39
-37
lines changed

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

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ unsafe impl crate::object::Traverse for PyType {
5050
fn traverse(&self, tracer_fn: &mut crate::object::TraverseFn<'_>) {
5151
self.base.traverse(tracer_fn);
5252
self.bases.traverse(tracer_fn);
53-
self.mro.traverse(tracer_fn);
53+
// mro contains self as mro[0], so skip traversing to avoid circular reference
54+
// self.mro.traverse(tracer_fn);
5455
self.subclasses.traverse(tracer_fn);
5556
self.attributes
5657
.read_recursive()
@@ -341,6 +342,7 @@ impl PyType {
341342
metaclass,
342343
None,
343344
);
345+
new_type.mro.write().insert(0, new_type.clone());
344346

345347
new_type.init_slots(ctx);
346348

@@ -393,6 +395,7 @@ impl PyType {
393395
metaclass,
394396
None,
395397
);
398+
new_type.mro.write().insert(0, new_type.clone());
396399

397400
// Note: inherit_slots is called in PyClassImpl::init_class after
398401
// slots are fully initialized by make_slots()
@@ -413,9 +416,8 @@ impl PyType {
413416
}
414417

415418
pub(crate) fn init_slots(&self, ctx: &Context) {
416-
// Inherit slots from MRO
417-
// Note: self.mro does NOT include self, so we iterate all elements
418-
let mro: Vec<_> = self.mro.read().iter().cloned().collect();
419+
// Inherit slots from MRO (mro[0] is self, so skip it)
420+
let mro: Vec<_> = self.mro.read()[1..].to_vec();
419421
for base in mro.iter() {
420422
self.inherit_slots(base);
421423
}
@@ -424,7 +426,8 @@ impl PyType {
424426
#[allow(clippy::mutable_key_type)]
425427
let mut slot_name_set = std::collections::HashSet::new();
426428

427-
for cls in self.mro.read().iter() {
429+
// mro[0] is self, so skip it; self.attributes is checked separately below
430+
for cls in self.mro.read()[1..].iter() {
428431
for &name in cls.attributes.read().keys() {
429432
if name.as_bytes().starts_with(b"__") && name.as_bytes().ends_with(b"__") {
430433
slot_name_set.insert(name);
@@ -503,18 +506,12 @@ impl PyType {
503506
/// Equivalent to CPython's find_name_in_mro
504507
/// Look in tp_dict of types in MRO - bypasses descriptors and other attribute access machinery
505508
fn find_name_in_mro(&self, name: &'static PyStrInterned) -> Option<PyObjectRef> {
506-
// First check in our own dict
507-
if let Some(value) = self.attributes.read().get(name) {
508-
return Some(value.clone());
509-
}
510-
511-
// Then check in MRO
512-
for base in self.mro.read().iter() {
513-
if let Some(value) = base.attributes.read().get(name) {
509+
// mro[0] is self, so we just iterate through the entire MRO
510+
for cls in self.mro.read().iter() {
511+
if let Some(value) = cls.attributes.read().get(name) {
514512
return Some(value.clone());
515513
}
516514
}
517-
518515
None
519516
}
520517

@@ -530,18 +527,15 @@ impl PyType {
530527
}
531528

532529
pub fn get_super_attr(&self, attr_name: &'static PyStrInterned) -> Option<PyObjectRef> {
533-
self.mro
534-
.read()
530+
self.mro.read()[1..]
535531
.iter()
536532
.find_map(|class| class.attributes.read().get(attr_name).cloned())
537533
}
538534

539535
// This is the internal has_attr implementation for fast lookup on a class.
540536
pub fn has_attr(&self, attr_name: &'static PyStrInterned) -> bool {
541537
self.attributes.read().contains_key(attr_name)
542-
|| self
543-
.mro
544-
.read()
538+
|| self.mro.read()[1..]
545539
.iter()
546540
.any(|c| c.attributes.read().contains_key(attr_name))
547541
}
@@ -550,10 +544,8 @@ impl PyType {
550544
// Gather all members here:
551545
let mut attributes = PyAttributes::default();
552546

553-
for bc in core::iter::once(self)
554-
.chain(self.mro.read().iter().map(|cls| -> &Self { cls }))
555-
.rev()
556-
{
547+
// mro[0] is self, so we iterate through the entire MRO in reverse
548+
for bc in self.mro.read().iter().map(|cls| -> &Self { cls }).rev() {
557549
for (name, value) in bc.attributes.read().iter() {
558550
attributes.insert(name.to_owned(), value.clone());
559551
}
@@ -661,22 +653,21 @@ impl Py<PyType> {
661653
/// so only use this if `cls` is known to have not overridden the base __subclasscheck__ magic
662654
/// method.
663655
pub fn fast_issubclass(&self, cls: &impl Borrow<PyObject>) -> bool {
664-
self.as_object().is(cls.borrow()) || self.mro.read().iter().any(|c| c.is(cls.borrow()))
656+
self.as_object().is(cls.borrow()) || self.mro.read()[1..].iter().any(|c| c.is(cls.borrow()))
665657
}
666658

667659
pub fn mro_map_collect<F, R>(&self, f: F) -> Vec<R>
668660
where
669661
F: Fn(&Self) -> R,
670662
{
671-
core::iter::once(self)
672-
.chain(self.mro.read().iter().map(|x| x.deref()))
673-
.map(f)
674-
.collect()
663+
self.mro.read().iter().map(|x| x.deref()).map(f).collect()
675664
}
676665

677666
pub fn mro_collect(&self) -> Vec<PyRef<PyType>> {
678-
core::iter::once(self)
679-
.chain(self.mro.read().iter().map(|x| x.deref()))
667+
self.mro
668+
.read()
669+
.iter()
670+
.map(|x| x.deref())
680671
.map(|x| x.to_owned())
681672
.collect()
682673
}
@@ -745,8 +736,11 @@ impl PyType {
745736
*zelf.bases.write() = bases;
746737
// Recursively update the mros of this class and all subclasses
747738
fn update_mro_recursively(cls: &PyType, vm: &VirtualMachine) -> PyResult<()> {
748-
*cls.mro.write() =
739+
let mut mro =
749740
PyType::resolve_mro(&cls.bases.read()).map_err(|msg| vm.new_type_error(msg))?;
741+
// Preserve self (mro[0]) when updating MRO
742+
mro.insert(0, cls.mro.read()[0].to_owned());
743+
*cls.mro.write() = mro;
750744
for subclass in cls.subclasses.write().iter() {
751745
let subclass = subclass.upgrade().unwrap();
752746
let subclass: &Py<PyType> = subclass.downcast_ref().unwrap();

‎crates/vm/src/object/core.rs‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1310,12 +1310,16 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) {
13101310
ptr::write(&mut (*type_type_ptr).typ, PyAtomicRef::from(type_type));
13111311

13121312
let object_type = PyTypeRef::from_raw(object_type_ptr.cast());
1313+
// object's mro is [object]
1314+
(*object_type_ptr).payload.mro = PyRwLock::new(vec![object_type.clone()]);
13131315

1314-
(*type_type_ptr).payload.mro = PyRwLock::new(vec![object_type.clone()]);
13151316
(*type_type_ptr).payload.bases = PyRwLock::new(vec![object_type.clone()]);
13161317
(*type_type_ptr).payload.base = Some(object_type.clone());
13171318

13181319
let type_type = PyTypeRef::from_raw(type_type_ptr.cast());
1320+
// type's mro is [type, object]
1321+
(*type_type_ptr).payload.mro =
1322+
PyRwLock::new(vec![type_type.clone(), object_type.clone()]);
13191323

13201324
(type_type, object_type)
13211325
}
@@ -1331,6 +1335,8 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) {
13311335
heaptype_ext: None,
13321336
};
13331337
let weakref_type = PyRef::new_ref(weakref_type, type_type.clone(), None);
1338+
// weakref's mro is [weakref, object]
1339+
weakref_type.mro.write().insert(0, weakref_type.clone());
13341340

13351341
object_type.subclasses.write().push(
13361342
type_type

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -760,8 +760,9 @@ impl PyType {
760760
let in_self = cmp_names.iter().any(|n| attrs.contains_key(*n));
761761
drop(attrs);
762762

763+
// mro[0] is self, so skip it since we already checked self above
763764
in_self
764-
|| self.mro.read().iter().any(|cls| {
765+
|| self.mro.read()[1..].iter().any(|cls| {
765766
let attrs = cls.attributes.read();
766767
cmp_names.iter().any(|n| {
767768
if let Some(attr) = attrs.get(*n) {
@@ -1273,8 +1274,8 @@ impl PyType {
12731274
return None;
12741275
}
12751276

1276-
// Look up in MRO
1277-
for cls in self.mro.read().iter() {
1277+
// Look up in MRO (mro[0] is self, so skip it)
1278+
for cls in self.mro.read()[1..].iter() {
12781279
if let Some(attr) = cls.attributes.read().get(name).cloned() {
12791280
if let Some(func) = try_extract(&attr) {
12801281
return Some(func);

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,8 +405,9 @@ impl SlotAccessor {
405405

406406
/// Inherit slot value from MRO
407407
pub fn inherit_from_mro(&self, typ: &crate::builtins::PyType) {
408-
// Note: typ.mro does NOT include typ itself
409-
let mro = typ.mro.read();
408+
// mro[0] is self, so skip it
409+
let mro_guard = typ.mro.read();
410+
let mro = &mro_guard[1..];
410411

411412
macro_rules! inherit_main {
412413
($slot:ident) => {{

0 commit comments

Comments
 (0)