Align is_instance (RustPython#5855)

* Align is_instance

* Upgrade test_isinstance to 3.13.5

* patch test_isinstance

Author: youknowone

Lib/test/test_isinstance.py

@@ -3,12 +3,11 @@
 # testing of error conditions uncovered when using extension types.
 
 import unittest
-import sys
 import typing
 from test import support
 
 
-
+
 class TestIsInstanceExceptions(unittest.TestCase):
     # Test to make sure that an AttributeError when accessing the instance's
     # class's bases is masked.  This was actually a bug in Python 2.2 and
@@ -97,7 +96,7 @@ def getclass(self):
         class D: pass
         self.assertRaises(RuntimeError, isinstance, c, D)
 
-
+
 # These tests are similar to above, but tickle certain code paths in
 # issubclass() instead of isinstance() -- really PyObject_IsSubclass()
 # vs. PyObject_IsInstance().
@@ -147,7 +146,7 @@ def getbases(self):
         self.assertRaises(TypeError, issubclass, B, C())
 
 
-
+
 # meta classes for creating abstract classes and instances
 class AbstractClass(object):
     def __init__(self, bases):
@@ -179,7 +178,7 @@ class Super:
 
 class Child(Super):
     pass
-
+
 class TestIsInstanceIsSubclass(unittest.TestCase):
     # Tests to ensure that isinstance and issubclass work on abstract
     # classes and instances.  Before the 2.2 release, TypeErrors were
@@ -225,7 +224,7 @@ def test_isinstance_with_or_union(self):
         with self.assertRaises(TypeError):
             isinstance(2, list[int] | int)
         with self.assertRaises(TypeError):
-            isinstance(2, int | str | list[int] | float)
+            isinstance(2, float | str | list[int] | int)
 
 
 
@@ -311,7 +310,7 @@ class X:
             @property
             def __bases__(self):
                 return self.__bases__
-        with support.infinite_recursion():
+        with support.infinite_recursion(25):
             self.assertRaises(RecursionError, issubclass, X(), int)
             self.assertRaises(RecursionError, issubclass, int, X())
             self.assertRaises(RecursionError, isinstance, 1, X())
@@ -345,18 +344,20 @@ class B:
                     pass
                 A.__getattr__ = B.__getattr__ = X.__getattr__
                 return (A(), B())
-        with support.infinite_recursion():
+        with support.infinite_recursion(25):
             self.assertRaises(RecursionError, issubclass, X(), int)
 
 
 def blowstack(fxn, arg, compare_to):
     # Make sure that calling isinstance with a deeply nested tuple for its
     # argument will raise RecursionError eventually.
     tuple_arg = (compare_to,)
+    # XXX: RUSTPYTHON; support.exceeds_recursion_limit() is not available yet.
+    import sys
     for cnt in range(sys.getrecursionlimit()+5):
         tuple_arg = (tuple_arg,)
         fxn(arg, tuple_arg)
 
-
+
 if __name__ == '__main__':
     unittest.main()

vm/src/builtins/type.rs

@@ -1207,8 +1207,9 @@ impl Py<PyType> {
     }
 
     #[pymethod]
-    fn __instancecheck__(&self, obj: PyObjectRef) -> bool {
-        obj.fast_isinstance(self)
+    fn __instancecheck__(&self, obj: PyObjectRef, vm: &VirtualMachine) -> PyResult<bool> {
+        // Use real_is_instance to avoid infinite recursion, matching CPython's behavior
+        obj.real_is_instance(self.as_object(), vm)
     }
 
     #[pymethod]

vm/src/protocol/object.rs

@@ -428,30 +428,51 @@ impl PyObject {
         if let (Ok(obj), Ok(cls)) = (self.try_to_ref::<PyType>(vm), cls.try_to_ref::<PyType>(vm)) {
             Ok(obj.fast_issubclass(cls))
         } else {
+            // Check if derived is a class
             self.check_cls(self, vm, || {
                 format!("issubclass() arg 1 must be a class, not {}", self.class())
-            })
-            .and(self.check_cls(cls, vm, || {
-                format!(
-                    "issubclass() arg 2 must be a class, a tuple of classes, or a union, not {}",
-                    cls.class()
-                )
-            }))
-            .and(self.abstract_issubclass(cls, vm))
+            })?;
+
+            // Check if cls is a class, tuple, or union
+            if !cls.class().is(vm.ctx.types.union_type) {
+                self.check_cls(cls, vm, || {
+                    format!(
+                        "issubclass() arg 2 must be a class, a tuple of classes, or a union, not {}",
+                        cls.class()
+                    )
+                })?;
+            }
+
+            self.abstract_issubclass(cls, vm)
         }
     }
 
     /// Determines if `self` is a subclass of `cls`, either directly, indirectly or virtually
     /// via the __subclasscheck__ magic method.
+    /// PyObject_IsSubclass/object_issubclass
     pub fn is_subclass(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
+        // PyType_CheckExact(cls)
         if cls.class().is(vm.ctx.types.type_type) {
             if self.is(cls) {
                 return Ok(true);
             }
             return self.recursive_issubclass(cls, vm);
         }
 
-        if let Ok(tuple) = cls.try_to_value::<&Py<PyTuple>>(vm) {
+        // Check for Union type - CPython handles this before tuple
+        let cls_to_check = if cls.class().is(vm.ctx.types.union_type) {
+            // Get the __args__ attribute which contains the union members
+            if let Ok(args) = cls.get_attr(identifier!(vm, __args__), vm) {
+                args
+            } else {
+                cls.to_owned()
+            }
+        } else {
+            cls.to_owned()
+        };
+
+        // Check if cls_to_check is a tuple
+        if let Ok(tuple) = cls_to_check.try_to_value::<&Py<PyTuple>>(vm) {
             for typ in tuple {
                 if vm.with_recursion("in __subclasscheck__", || self.is_subclass(typ, vm))? {
                     return Ok(true);
@@ -460,6 +481,7 @@ impl PyObject {
             return Ok(false);
         }
 
+        // Check for __subclasscheck__ method
         if let Some(meth) = vm.get_special_method(cls, identifier!(vm, __subclasscheck__))? {
             let ret = vm.with_recursion("in __subclasscheck__", || {
                 meth.invoke((self.to_owned(),), vm)
@@ -470,51 +492,84 @@ impl PyObject {
         self.recursive_issubclass(cls, vm)
     }
 
-    fn abstract_isinstance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
-        let r = if let Ok(typ) = cls.try_to_ref::<PyType>(vm) {
-            if self.class().fast_issubclass(typ) {
-                true
-            } else if let Ok(i_cls) =
-                PyTypeRef::try_from_object(vm, self.get_attr(identifier!(vm, __class__), vm)?)
-            {
-                if i_cls.is(self.class()) {
-                    false
-                } else {
-                    i_cls.fast_issubclass(typ)
+    /// Real isinstance check without going through __instancecheck__
+    /// This is equivalent to CPython's _PyObject_RealIsInstance/object_isinstance
+    pub fn real_is_instance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
+        if let Ok(typ) = cls.try_to_ref::<PyType>(vm) {
+            // PyType_Check(cls) - cls is a type object
+            let mut retval = self.fast_isinstance(typ);
+
+            if !retval {
+                // Check __class__ attribute, only masking AttributeError
+                if let Some(i_cls) =
+                    vm.get_attribute_opt(self.to_owned(), identifier!(vm, __class__))?
+                {
+                    if let Ok(i_cls_type) = PyTypeRef::try_from_object(vm, i_cls) {
+                        if !i_cls_type.is(self.class()) {
+                            retval = i_cls_type.fast_issubclass(typ);
+                        }
+                    }
                 }
-            } else {
-                false
             }
+            Ok(retval)
         } else {
+            // Not a type object, check if it's a valid class
             self.check_cls(cls, vm, || {
                 format!(
-                    "isinstance() arg 2 must be a type or tuple of types, not {}",
+                    "isinstance() arg 2 must be a type, a tuple of types, or a union, not {}",
                     cls.class()
                 )
             })?;
-            let i_cls: PyObjectRef = self.get_attr(identifier!(vm, __class__), vm)?;
-            if vm.is_none(&i_cls) {
-                false
+
+            // Get __class__ attribute and check, only masking AttributeError
+            if let Some(i_cls) =
+                vm.get_attribute_opt(self.to_owned(), identifier!(vm, __class__))?
+            {
+                if vm.is_none(&i_cls) {
+                    Ok(false)
+                } else {
+                    i_cls.abstract_issubclass(cls, vm)
+                }
             } else {
-                i_cls.abstract_issubclass(cls, vm)?
+                Ok(false)
             }
-        };
-        Ok(r)
+        }
     }
 
     /// Determines if `self` is an instance of `cls`, either directly, indirectly or virtually via
     /// the __instancecheck__ magic method.
+    // This is object_recursive_isinstance from CPython's Objects/abstract.c
     pub fn is_instance(&self, cls: &PyObject, vm: &VirtualMachine) -> PyResult<bool> {
-        // cpython first does an exact check on the type, although documentation doesn't state that
-        // https://github.com/python/cpython/blob/a24107b04c1277e3c1105f98aff5bfa3a98b33a0/Objects/abstract.c#L2408
+        // PyObject_TypeCheck(inst, (PyTypeObject *)cls)
+        // This is an exact check of the type
         if self.class().is(cls) {
             return Ok(true);
         }
 
+        // PyType_CheckExact(cls) optimization
         if cls.class().is(vm.ctx.types.type_type) {
-            return self.abstract_isinstance(cls, vm);
+            // When cls is exactly a type (not a subclass), use real_is_instance
+            // to avoid going through __instancecheck__ (matches CPython behavior)
+            return self.real_is_instance(cls, vm);
+        }
+
+        // Check for Union type (e.g., int | str) - CPython checks this before tuple
+        if cls.class().is(vm.ctx.types.union_type) {
+            if let Ok(args) = cls.get_attr(identifier!(vm, __args__), vm) {
+                if let Ok(tuple) = args.try_to_ref::<PyTuple>(vm) {
+                    for typ in tuple {
+                        if vm
+                            .with_recursion("in __instancecheck__", || self.is_instance(typ, vm))?
+                        {
+                            return Ok(true);
+                        }
+                    }
+                    return Ok(false);
+                }
+            }
         }
 
+        // Check if cls is a tuple
         if let Ok(tuple) = cls.try_to_ref::<PyTuple>(vm) {
             for typ in tuple {
                 if vm.with_recursion("in __instancecheck__", || self.is_instance(typ, vm))? {
@@ -524,14 +579,16 @@ impl PyObject {
             return Ok(false);
         }
 
-        if let Some(meth) = vm.get_special_method(cls, identifier!(vm, __instancecheck__))? {
-            let ret = vm.with_recursion("in __instancecheck__", || {
-                meth.invoke((self.to_owned(),), vm)
+        // Check for __instancecheck__ method
+        if let Some(checker) = vm.get_special_method(cls, identifier!(vm, __instancecheck__))? {
+            let res = vm.with_recursion("in __instancecheck__", || {
+                checker.invoke((self.to_owned(),), vm)
             })?;
-            return ret.try_to_bool(vm);
+            return res.try_to_bool(vm);
         }
 
-        self.abstract_isinstance(cls, vm)
+        // Fall back to object_isinstance (without going through __instancecheck__ again)
+        self.real_is_instance(cls, vm)
     }
 
     pub fn hash(&self, vm: &VirtualMachine) -> PyResult<PyHash> {