Clean up edge cases in isinstance and issubclass.

Lib/test/test_isinstance.py

@@ -22,8 +22,6 @@ class TestIsInstanceExceptions(unittest.TestCase):
     # Sounds complicated, I know, but this mimics a situation where an
     # extension type raises an AttributeError when its __bases__ attribute is
     # gotten.  In that case, isinstance() should return False.
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_class_has_no_bases(self):
         class I(object):
             def getclass(self):
@@ -40,8 +38,6 @@ class TestIsInstanceExceptions(unittest.TestCase):
 
     # Like above except that inst.__class__.__bases__ raises an exception
     # other than AttributeError
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_bases_raises_other_than_attribute_error(self):
         class E(object):
             def getbases(self):
@@ -62,8 +58,6 @@ class TestIsInstanceExceptions(unittest.TestCase):
 
     # Here's a situation where getattr(cls, '__bases__') raises an exception.
     # If that exception is not AttributeError, it should not get masked
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_dont_mask_non_attribute_error(self):
         class I: pass
 
@@ -88,8 +82,6 @@ class TestIsInstanceExceptions(unittest.TestCase):
 
     # check that we don't mask non AttributeErrors
     # see: http://bugs.python.org/issue1574217
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_isinstance_dont_mask_non_attribute_error(self):
         class C(object):
             def getclass(self):
@@ -108,8 +100,6 @@ class TestIsInstanceExceptions(unittest.TestCase):
 # issubclass() instead of isinstance() -- really PyObject_IsSubclass()
 # vs. PyObject_IsInstance().
 class TestIsSubclassExceptions(unittest.TestCase):
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_dont_mask_non_attribute_error(self):
         class C(object):
             def getbases(self):
@@ -134,8 +124,6 @@ class TestIsSubclassExceptions(unittest.TestCase):
     # second arg (the cls arg) is tested.  This means the first arg must
     # return a valid __bases__, and it's okay for it to be a normal --
     # unrelated by inheritance -- class.
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_dont_mask_non_attribute_error_in_cls_arg(self):
         class B: pass
 
@@ -198,8 +186,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase):
     # 'abstract' classes and instances.  This case tries to test all
     # combinations.
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_isinstance_normal(self):
         # normal instances
         self.assertEqual(True, isinstance(Super(), Super))
@@ -210,8 +196,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase):
         self.assertEqual(True, isinstance(Child(), Super))
         self.assertEqual(False, isinstance(Child(), AbstractSuper))
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_isinstance_abstract(self):
         # abstract instances
         self.assertEqual(True, isinstance(AbstractSuper(), AbstractSuper))
@@ -234,8 +218,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase):
         self.assertEqual(True, issubclass(Child, Super))
         self.assertEqual(False, issubclass(Child, AbstractSuper))
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_subclass_abstract(self):
         # abstract classes
         self.assertEqual(True, issubclass(AbstractSuper, AbstractSuper))
@@ -259,22 +241,20 @@ class TestIsInstanceIsSubclass(unittest.TestCase):
         self.assertEqual(True, issubclass(int, (int, (float, int))))
         self.assertEqual(True, issubclass(str, (str, (Child, str))))
 
-    # TODO: RUSTPYTHON
+    # TODO: RUSTPYTHON requires issue 2680 for stack depth detection in Rust
     @unittest.expectedFailure
     def test_subclass_recursion_limit(self):
         # make sure that issubclass raises RecursionError before the C stack is
         # blown
         self.assertRaises(RecursionError, blowstack, issubclass, str, str)
 
-    # TODO: RUSTPYTHON
+    # TODO: RUSTPYTHON requires issue 2680 for stack depth detection in Rust
     @unittest.expectedFailure
     def test_isinstance_recursion_limit(self):
         # make sure that issubclass raises RecursionError before the C stack is
         # blown
         self.assertRaises(RecursionError, blowstack, isinstance, '', str)
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     def test_issubclass_refcount_handling(self):
         # bpo-39382: abstract_issubclass() didn't hold item reference while
         # peeking in the bases tuple, in the single inheritance case.
@@ -295,8 +275,6 @@ class TestIsInstanceIsSubclass(unittest.TestCase):
 
         self.assertEqual(True, issubclass(B(), int))
 
-    # TODO: RUSTPYTHON
-    @unittest.expectedFailure
     @unittest.skipIf(sys.platform == "win32", "thread 'main' has overflowed its stack")
     def test_infinite_recursion_in_bases(self):
         class X:

vm/src/vm.rs

@@ -944,14 +944,52 @@ impl VirtualMachine {
         }
     }
 
-    pub fn _isinstance(&self, obj: &PyObjectRef, cls: &PyObjectRef) -> PyResult<bool> {
+    // Equivalent to check_class. Masks Attribute errors (into TypeErrors) and lets everything
+    // else go through.
+    fn check_cls<F>(&self, cls: &PyObjectRef, msg: F) -> PyResult
+    where
+        F: Fn() -> String,
+    {
+        self.get_attribute(cls.clone(), "__bases__").map_err(|e| {
+            // Only mask AttributeErrors.
+            if e.class().is(&self.ctx.exceptions.attribute_error) {
+                self.new_type_error(msg())
+            } else {
+                e
+            }
+        })
+    }
+
+    fn abstract_isinstance(&self, obj: &PyObjectRef, cls: &PyObjectRef) -> PyResult<bool> {
         if let Ok(typ) = PyTypeRef::try_from_object(self, cls.clone()) {
-            Ok(obj.isinstance(&typ))
+            if obj.class().issubclass(typ.clone()) {
+                Ok(true)
+            } else if let Ok(icls) =
+                PyTypeRef::try_from_object(self, self.get_attribute(obj.clone(), "__class__")?)
+            {
+                if icls.is(&obj.class()) {
+                    Ok(false)
+                } else {
+                    Ok(icls.issubclass(typ))
+                }
+            } else {
+                Ok(false)
+            }
         } else {
-            Err(self.new_type_error(format!(
-                "isinstance() arg 2 must be a type or tuple of types, not {}",
-                cls.class()
-            )))
+            self.check_cls(cls, || {
+                format!(
+                    "isinstance() arg 2 must be a type or tuple of types, not {}",
+                    cls.class()
+                )
+            })
+            .and_then(|_| {
+                let icls = self.get_attribute(obj.clone(), "__class__")?;
+                if self.is_none(&icls) {
+                    Ok(false)
+                } else {
+                    self.abstract_issubclass(icls, cls)
+                }
+            })
         }
     }
 
@@ -965,7 +1003,7 @@ impl VirtualMachine {
         }
 
         if cls.class().is(&self.ctx.types.type_type) {
-            return self._isinstance(obj, cls);
+            return self.abstract_isinstance(obj, cls);
         }
 
         if let Ok(tuple) = PyTupleRef::try_from_object(self, cls.clone()) {
@@ -982,13 +1020,13 @@ impl VirtualMachine {
             return pybool::boolval(self, ret);
         }
 
-        self._isinstance(obj, cls)
+        self.abstract_isinstance(obj, cls)
     }
 
     fn abstract_issubclass(&self, subclass: PyObjectRef, cls: &PyObjectRef) -> PyResult<bool> {
         let mut derived = subclass;
         loop {
-            if derived.class().is(cls) {
+            if derived.is(cls) {
                 return Ok(true);
             }
 
@@ -1024,19 +1062,19 @@ impl VirtualMachine {
         ) {
             Ok(subclass.issubclass(cls))
         } else {
-            if self.get_attribute(subclass.clone(), "__bases__").is_err() {
-                return Err(self.new_type_error(format!(
+            self.check_cls(subclass, || {
+                format!(
                     "issubclass() arg 1 must be a class, not {}",
                     subclass.class()
-                )));
-            }
-            if self.get_attribute(cls.clone(), "__bases__").is_err() {
-                return Err(self.new_type_error(format!(
+                )
+            })
+            .and(self.check_cls(cls, || {
+                format!(
                     "issubclass() arg 2 must be a class or tuple of classes, not {}",
                     cls.class()
-                )));
-            }
-            self.abstract_issubclass(subclass.clone(), cls)
+                )
+            }))
+            .and(self.abstract_issubclass(subclass.clone(), cls))
         }
     }