Skip to content

Commit 1b40c45

Browse files
committed
Align is_instance
1 parent ac26be7 commit 1b40c45

File tree

2 files changed

+74
-10
lines changed

2 files changed

+74
-10
lines changed

vm/src/builtins/type.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,8 +1207,9 @@ impl Py<PyType> {
12071207
}
12081208

12091209
#[pymethod]
1210-
fn __instancecheck__(&self, obj: PyObjectRef) -> bool {
1211-
obj.fast_isinstance(self)
1210+
fn __instancecheck__(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
1211+
// Use real_is_instance to avoid infinite recursion, matching CPython's behavior
1212+
obj.real_is_instance(self.as_object(), vm)
12121213
}
12131214

12141215
#[pymethod]

vm/src/protocol/object.rs

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -470,6 +470,46 @@ impl PyObject {
470470
self.recursive_issubclass(cls, vm)
471471
}
472472

473+
/// Real isinstance check without going through __instancecheck__
474+
/// This is equivalent to CPython's _PyObject_RealIsInstance/object_isinstance
475+
pub fn real_is_instance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
476+
if let Ok(typ) = cls.try_to_ref::<PyType>(vm) {
477+
// PyType_Check(cls) - cls is a type object
478+
let mut retval = self.fast_isinstance(typ);
479+
480+
if !retval {
481+
// Check __class__ attribute
482+
if let Ok(i_cls) = self.get_attr(identifier!(vm, __class__), vm) {
483+
if let Ok(i_cls_type) = PyTypeRef::try_from_object(vm, i_cls) {
484+
if !i_cls_type.is(self.class()) {
485+
retval = i_cls_type.fast_issubclass(typ);
486+
}
487+
}
488+
}
489+
}
490+
Ok(retval)
491+
} else {
492+
// Not a type object, check if it's a valid class
493+
self.check_cls(cls, vm, || {
494+
format!(
495+
"isinstance() arg 2 must be a type, a tuple of types, or a union, not {}",
496+
cls.class()
497+
)
498+
})?;
499+
500+
// Get __class__ attribute and check
501+
if let Ok(i_cls) = self.get_attr(identifier!(vm, __class__), vm) {
502+
if vm.is_none(&i_cls) {
503+
Ok(false)
504+
} else {
505+
i_cls.abstract_issubclass(cls, vm)
506+
}
507+
} else {
508+
Ok(false)
509+
}
510+
}
511+
}
512+
473513
fn abstract_isinstance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
474514
let r = if let Ok(typ) = cls.try_to_ref::<PyType>(vm) {
475515
if self.class().fast_issubclass(typ) {
@@ -504,17 +544,38 @@ impl PyObject {
504544

505545
/// Determines if `self` is an instance of `cls`, either directly, indirectly or virtually via
506546
/// the __instancecheck__ magic method.
547+
// This is object_recursive_isinstance from CPython's Objects/abstract.c
507548
pub fn is_instance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
508-
// cpython first does an exact check on the type, although documentation doesn't state that
509-
// https://github.com/python/cpython/blob/a24107b04c1277e3c1105f98aff5bfa3a98b33a0/Objects/abstract.c#L2408
549+
// PyObject_TypeCheck(inst, (PyTypeObject *)cls)
550+
// This is an exact check of the type
510551
if self.class().is(cls) {
511552
return Ok(true);
512553
}
513554

555+
// PyType_CheckExact(cls) optimization
514556
if cls.class().is(vm.ctx.types.type_type) {
515-
return self.abstract_isinstance(cls, vm);
557+
// When cls is exactly a type (not a subclass), use real_is_instance
558+
// to avoid going through __instancecheck__ (matches CPython behavior)
559+
return self.real_is_instance(cls, vm);
560+
}
561+
562+
// Check for Union type (e.g., int | str)
563+
if cls.class().is(vm.ctx.types.union_type) {
564+
if let Ok(args) = cls.get_attr(identifier!(vm, __args__), vm) {
565+
if let Ok(tuple) = args.try_to_ref::<PyTuple>(vm) {
566+
for typ in tuple {
567+
if vm
568+
.with_recursion("in __instancecheck__", || self.is_instance(typ, vm))?
569+
{
570+
return Ok(true);
571+
}
572+
}
573+
return Ok(false);
574+
}
575+
}
516576
}
517577

578+
// Check if cls is a tuple
518579
if let Ok(tuple) = cls.try_to_ref::<PyTuple>(vm) {
519580
for typ in tuple {
520581
if vm.with_recursion("in __instancecheck__", || self.is_instance(typ, vm))? {
@@ -524,14 +585,16 @@ impl PyObject {
524585
return Ok(false);
525586
}
526587

527-
if let Some(meth) = vm.get_special_method(cls, identifier!(vm, __instancecheck__))? {
528-
let ret = vm.with_recursion("in __instancecheck__", || {
529-
meth.invoke((self.to_owned(),), vm)
588+
// Check for __instancecheck__ method
589+
if let Some(checker) = vm.get_special_method(cls, identifier!(vm, __instancecheck__))? {
590+
let res = vm.with_recursion("in __instancecheck__", || {
591+
checker.invoke((self.to_owned(),), vm)
530592
})?;
531-
return ret.try_to_bool(vm);
593+
return res.try_to_bool(vm);
532594
}
533595

534-
self.abstract_isinstance(cls, vm)
596+
// Fall back to object_isinstance (without going through __instancecheck__ again)
597+
self.real_is_instance(cls, vm)
535598
}
536599

537600
pub fn hash(&self, vm: &VirtualMachine) -> PyResult<PyHash> {

0 commit comments

Comments
 (0)