Skip to content

Commit 2bd5212

Browse files
youknowoneyouknowone
authored
Align is_instance (#5855)
* Align is_instance * Upgrade test_isinstance to 3.13.5 * patch test_isinstance
1 parent 910077d commit 2bd5212

File tree

3 files changed

+107
-48
lines changed

3 files changed

+107
-48
lines changed

‎Lib/test/test_isinstance.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33
# testing of error conditions uncovered when using extension types.
44

55
import unittest
6-
import sys
76
import typing
87
from test import support
98

109

11-
10+
1211
class TestIsInstanceExceptions(unittest.TestCase):
1312
# Test to make sure that an AttributeError when accessing the instance's
1413
# class's bases is masked. This was actually a bug in Python 2.2 and
@@ -97,7 +96,7 @@ def getclass(self):
9796
class D: pass
9897
self.assertRaises(RuntimeError, isinstance, c, D)
9998

100-
99+
101100
# These tests are similar to above, but tickle certain code paths in
102101
# issubclass() instead of isinstance() -- really PyObject_IsSubclass()
103102
# vs. PyObject_IsInstance().
@@ -147,7 +146,7 @@ def getbases(self):
147146
self.assertRaises(TypeError, issubclass, B, C())
148147

149148

150-
149+
151150
# meta classes for creating abstract classes and instances
152151
class AbstractClass(object):
153152
def __init__(self, bases):
@@ -179,7 +178,7 @@ class Super:
179178

180179
class Child(Super):
181180
pass
182-
181+
183182
class TestIsInstanceIsSubclass(unittest.TestCase):
184183
# Tests to ensure that isinstance and issubclass work on abstract
185184
# classes and instances. Before the 2.2 release, TypeErrors were
@@ -225,7 +224,7 @@ def test_isinstance_with_or_union(self):
225224
with self.assertRaises(TypeError):
226225
isinstance(2, list[int] | int)
227226
with self.assertRaises(TypeError):
228-
isinstance(2, int | str | list[int] | float)
227+
isinstance(2, float | str | list[int] | int)
229228

230229

231230

@@ -311,7 +310,7 @@ class X:
311310
@property
312311
def __bases__(self):
313312
return self.__bases__
314-
with support.infinite_recursion():
313+
with support.infinite_recursion(25):
315314
self.assertRaises(RecursionError, issubclass, X(), int)
316315
self.assertRaises(RecursionError, issubclass, int, X())
317316
self.assertRaises(RecursionError, isinstance, 1, X())
@@ -345,18 +344,20 @@ class B:
345344
pass
346345
A.__getattr__ = B.__getattr__ = X.__getattr__
347346
return (A(), B())
348-
with support.infinite_recursion():
347+
with support.infinite_recursion(25):
349348
self.assertRaises(RecursionError, issubclass, X(), int)
350349

351350

352351
def blowstack(fxn, arg, compare_to):
353352
# Make sure that calling isinstance with a deeply nested tuple for its
354353
# argument will raise RecursionError eventually.
355354
tuple_arg = (compare_to,)
355+
# XXX: RUSTPYTHON; support.exceeds_recursion_limit() is not available yet.
356+
import sys
356357
for cnt in range(sys.getrecursionlimit()+5):
357358
tuple_arg = (tuple_arg,)
358359
fxn(arg, tuple_arg)
359360

360-
361+
361362
if __name__ == '__main__':
362363
unittest.main()

‎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: 94 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -428,30 +428,51 @@ impl PyObject {
428428
if let (Ok(obj), Ok(cls)) = (self.try_to_ref::<PyType>(vm), cls.try_to_ref::<PyType>(vm)) {
429429
Ok(obj.fast_issubclass(cls))
430430
} else {
431+
// Check if derived is a class
431432
self.check_cls(self, vm, || {
432433
format!("issubclass() arg 1 must be a class, not {}", self.class())
433-
})
434-
.and(self.check_cls(cls, vm, || {
435-
format!(
436-
"issubclass() arg 2 must be a class, a tuple of classes, or a union, not {}",
437-
cls.class()
438-
)
439-
}))
440-
.and(self.abstract_issubclass(cls, vm))
434+
})?;
435+
436+
// Check if cls is a class, tuple, or union
437+
if !cls.class().is(vm.ctx.types.union_type) {
438+
self.check_cls(cls, vm, || {
439+
format!(
440+
"issubclass() arg 2 must be a class, a tuple of classes, or a union, not {}",
441+
cls.class()
442+
)
443+
})?;
444+
}
445+
446+
self.abstract_issubclass(cls, vm)
441447
}
442448
}
443449

444450
/// Determines if `self` is a subclass of `cls`, either directly, indirectly or virtually
445451
/// via the __subclasscheck__ magic method.
452+
/// PyObject_IsSubclass/object_issubclass
446453
pub fn is_subclass(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
454+
// PyType_CheckExact(cls)
447455
if cls.class().is(vm.ctx.types.type_type) {
448456
if self.is(cls) {
449457
return Ok(true);
450458
}
451459
return self.recursive_issubclass(cls, vm);
452460
}
453461

454-
if let Ok(tuple) = cls.try_to_value::<&Py<PyTuple>>(vm) {
462+
// Check for Union type - CPython handles this before tuple
463+
let cls_to_check = if cls.class().is(vm.ctx.types.union_type) {
464+
// Get the __args__ attribute which contains the union members
465+
if let Ok(args) = cls.get_attr(identifier!(vm, __args__), vm) {
466+
args
467+
} else {
468+
cls.to_owned()
469+
}
470+
} else {
471+
cls.to_owned()
472+
};
473+
474+
// Check if cls_to_check is a tuple
475+
if let Ok(tuple) = cls_to_check.try_to_value::<&Py<PyTuple>>(vm) {
455476
for typ in tuple {
456477
if vm.with_recursion("in __subclasscheck__", || self.is_subclass(typ, vm))? {
457478
return Ok(true);
@@ -460,6 +481,7 @@ impl PyObject {
460481
return Ok(false);
461482
}
462483

484+
// Check for __subclasscheck__ method
463485
if let Some(meth) = vm.get_special_method(cls, identifier!(vm, __subclasscheck__))? {
464486
let ret = vm.with_recursion("in __subclasscheck__", || {
465487
meth.invoke((self.to_owned(),), vm)
@@ -470,51 +492,84 @@ impl PyObject {
470492
self.recursive_issubclass(cls, vm)
471493
}
472494

473-
fn abstract_isinstance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
474-
let r = if let Ok(typ) = cls.try_to_ref::<PyType>(vm) {
475-
if self.class().fast_issubclass(typ) {
476-
true
477-
} else if let Ok(i_cls) =
478-
PyTypeRef::try_from_object(vm, self.get_attr(identifier!(vm, __class__), vm)?)
479-
{
480-
if i_cls.is(self.class()) {
481-
false
482-
} else {
483-
i_cls.fast_issubclass(typ)
495+
/// Real isinstance check without going through __instancecheck__
496+
/// This is equivalent to CPython's _PyObject_RealIsInstance/object_isinstance
497+
pub fn real_is_instance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
498+
if let Ok(typ) = cls.try_to_ref::<PyType>(vm) {
499+
// PyType_Check(cls) - cls is a type object
500+
let mut retval = self.fast_isinstance(typ);
501+
502+
if !retval {
503+
// Check __class__ attribute, only masking AttributeError
504+
if let Some(i_cls) =
505+
vm.get_attribute_opt(self.to_owned(), identifier!(vm, __class__))?
506+
{
507+
if let Ok(i_cls_type) = PyTypeRef::try_from_object(vm, i_cls) {
508+
if !i_cls_type.is(self.class()) {
509+
retval = i_cls_type.fast_issubclass(typ);
510+
}
511+
}
484512
}
485-
} else {
486-
false
487513
}
514+
Ok(retval)
488515
} else {
516+
// Not a type object, check if it's a valid class
489517
self.check_cls(cls, vm, || {
490518
format!(
491-
"isinstance() arg 2 must be a type or tuple of types, not {}",
519+
"isinstance() arg 2 must be a type, a tuple of types, or a union, not {}",
492520
cls.class()
493521
)
494522
})?;
495-
let i_cls: PyObjectRef = self.get_attr(identifier!(vm, __class__), vm)?;
496-
if vm.is_none(&i_cls) {
497-
false
523+
524+
// Get __class__ attribute and check, only masking AttributeError
525+
if let Some(i_cls) =
526+
vm.get_attribute_opt(self.to_owned(), identifier!(vm, __class__))?
527+
{
528+
if vm.is_none(&i_cls) {
529+
Ok(false)
530+
} else {
531+
i_cls.abstract_issubclass(cls, vm)
532+
}
498533
} else {
499-
i_cls.abstract_issubclass(cls, vm)?
534+
Ok(false)
500535
}
501-
};
502-
Ok(r)
536+
}
503537
}
504538

505539
/// Determines if `self` is an instance of `cls`, either directly, indirectly or virtually via
506540
/// the __instancecheck__ magic method.
541+
// This is object_recursive_isinstance from CPython's Objects/abstract.c
507542
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
543+
// PyObject_TypeCheck(inst, (PyTypeObject *)cls)
544+
// This is an exact check of the type
510545
if self.class().is(cls) {
511546
return Ok(true);
512547
}
513548

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

572+
// Check if cls is a tuple
518573
if let Ok(tuple) = cls.try_to_ref::<PyTuple>(vm) {
519574
for typ in tuple {
520575
if vm.with_recursion("in __instancecheck__", || self.is_instance(typ, vm))? {
@@ -524,14 +579,16 @@ impl PyObject {
524579
return Ok(false);
525580
}
526581

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)
582+
// Check for __instancecheck__ method
583+
if let Some(checker) = vm.get_special_method(cls, identifier!(vm, __instancecheck__))? {
584+
let res = vm.with_recursion("in __instancecheck__", || {
585+
checker.invoke((self.to_owned(),), vm)
530586
})?;
531-
return ret.try_to_bool(vm);
587+
return res.try_to_bool(vm);
532588
}
533589

534-
self.abstract_isinstance(cls, vm)
590+
// Fall back to object_isinstance (without going through __instancecheck__ again)
591+
self.real_is_instance(cls, vm)
535592
}
536593

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

0 commit comments

Comments
 (0)